Re: [U-Boot] [PATCH v3 13/20] spl: nand: sunxi: use PIO instead of DMA

[Maxime Ripard]

Hi,

On Tue, Jul 06, 2021 at 06:00:33PM +0200, Miquel Raynal wrote:
> Hi Maxime,
> 
> Maxime Ripard  wrote on Thu, 24 Jun 2021 14:05:40
> +0200:
> 
> > Hi Miquel,
> > 
> > On Wed, Feb 28, 2018 at 08:51:55PM +0100, Miquel Raynal wrote:
> > > SPL support was first written to support only the earlier generations of
> > > Allwinner SoCs, and was only really enabled on the A13 / GR8. However,
> > > those old SoCs had a DMA engine that has been replaced since the A31 by
> > > another DMA controller that is no longer compatible.
> > > 
> > > Since the code directly uses that DMA controller, it cannot operate
> > > properly on the later SoCs, while the NAND controller has not changed.
> > > 
> > > There's two paths forward, the first one would have been to add support
> > > for that DMA controller too, the second to just remove the DMA usage
> > > entirely and rely on PIO.
> > > 
> > > The later has been chosen because CPU overload at this stage is not an
> > > issue and it makes the driver more generic, and easier to understand.
> > > 
> > > Signed-off-by: Miquel Raynal 
> > > Acked-by: Boris Brezillon   
> > 
> > I'm a bit late to the party, but this bricks the CHIP Pro too. While
> > U-Boot proper seems to be flashed properly (re-reading it from the NAND
> > after flashing brings up the same CRC than the original image), the SPL
> > will only read 0s.
> > 
> > The transfer does complete though, so maybe it's just the copy from the
> > SRAM to the main memory that doesn't work?
> > 
> > The offset looks correct though, so I'm not sure.
> 
> Strange... I really have no idea what's going on here and especially I
> don't have any suitable board with me these days to troubleshoot this.
> 
> Sorry for not being helpful at all on this one :-)

Paul has two CHIP Pro, and there might be more at the Bootlin office

Maxime


signature.asc
Description: PGP signature

Re: [U-Boot] [PATCH v3 13/20] spl: nand: sunxi: use PIO instead of DMA

[Miquel Raynal]

Hi Maxime,

Maxime Ripard  wrote on Thu, 24 Jun 2021 14:05:40
+0200:

> Hi Miquel,
> 
> On Wed, Feb 28, 2018 at 08:51:55PM +0100, Miquel Raynal wrote:
> > SPL support was first written to support only the earlier generations of
> > Allwinner SoCs, and was only really enabled on the A13 / GR8. However,
> > those old SoCs had a DMA engine that has been replaced since the A31 by
> > another DMA controller that is no longer compatible.
> > 
> > Since the code directly uses that DMA controller, it cannot operate
> > properly on the later SoCs, while the NAND controller has not changed.
> > 
> > There's two paths forward, the first one would have been to add support
> > for that DMA controller too, the second to just remove the DMA usage
> > entirely and rely on PIO.
> > 
> > The later has been chosen because CPU overload at this stage is not an
> > issue and it makes the driver more generic, and easier to understand.
> > 
> > Signed-off-by: Miquel Raynal 
> > Acked-by: Boris Brezillon   
> 
> I'm a bit late to the party, but this bricks the CHIP Pro too. While
> U-Boot proper seems to be flashed properly (re-reading it from the NAND
> after flashing brings up the same CRC than the original image), the SPL
> will only read 0s.
> 
> The transfer does complete though, so maybe it's just the copy from the
> SRAM to the main memory that doesn't work?
> 
> The offset looks correct though, so I'm not sure.

Strange... I really have no idea what's going on here and especially I
don't have any suitable board with me these days to troubleshoot this.

Sorry for not being helpful at all on this one :-)

Thanks,
Miquèl

Re: [U-Boot] [PATCH v3 13/20] spl: nand: sunxi: use PIO instead of DMA

[Maxime Ripard]

Hi Miquel,

On Wed, Feb 28, 2018 at 08:51:55PM +0100, Miquel Raynal wrote:
> SPL support was first written to support only the earlier generations of
> Allwinner SoCs, and was only really enabled on the A13 / GR8. However,
> those old SoCs had a DMA engine that has been replaced since the A31 by
> another DMA controller that is no longer compatible.
> 
> Since the code directly uses that DMA controller, it cannot operate
> properly on the later SoCs, while the NAND controller has not changed.
> 
> There's two paths forward, the first one would have been to add support
> for that DMA controller too, the second to just remove the DMA usage
> entirely and rely on PIO.
> 
> The later has been chosen because CPU overload at this stage is not an
> issue and it makes the driver more generic, and easier to understand.
> 
> Signed-off-by: Miquel Raynal 
> Acked-by: Boris Brezillon 

I'm a bit late to the party, but this bricks the CHIP Pro too. While
U-Boot proper seems to be flashed properly (re-reading it from the NAND
after flashing brings up the same CRC than the original image), the SPL
will only read 0s.

The transfer does complete though, so maybe it's just the copy from the
SRAM to the main memory that doesn't work?

The offset looks correct though, so I'm not sure.

Maxime

[U-Boot] [PATCH v3 13/20] spl: nand: sunxi: use PIO instead of DMA

[Miquel Raynal]

SPL support was first written to support only the earlier generations of
Allwinner SoCs, and was only really enabled on the A13 / GR8. However,
those old SoCs had a DMA engine that has been replaced since the A31 by
another DMA controller that is no longer compatible.

Since the code directly uses that DMA controller, it cannot operate
properly on the later SoCs, while the NAND controller has not changed.

There's two paths forward, the first one would have been to add support
for that DMA controller too, the second to just remove the DMA usage
entirely and rely on PIO.

The later has been chosen because CPU overload at this stage is not an
issue and it makes the driver more generic, and easier to understand.

Signed-off-by: Miquel Raynal 
Acked-by: Boris Brezillon 
---
 drivers/mtd/nand/sunxi_nand_spl.c | 141 --
 1 file changed, 58 insertions(+), 83 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c 
b/drivers/mtd/nand/sunxi_nand_spl.c
index af1e964eff..7241e9a374 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* registers */
 #define NFC_CTL0x
@@ -67,10 +68,12 @@
 #define NFC_SEND_CMD3  (1 << 28)
 #define NFC_SEND_CMD4  (1 << 29)
 #define NFC_RAW_CMD(0 << 30)
+#define NFC_ECC_CMD(1 << 30)
 #define NFC_PAGE_CMD   (2 << 30)
 
 #define NFC_ST_CMD_INT_FLAG(1 << 1)
 #define NFC_ST_DMA_INT_FLAG(1 << 2)
+#define NFC_ST_CMD_FIFO_STAT   (1 << 3)
 
 #define NFC_READ_CMD_OFFSET 0
 #define NFC_RANDOM_READ_CMD0_OFFSET 8
@@ -80,22 +83,6 @@
 #define NFC_CMD_RNDOUT 0x05
 #define NFC_CMD_READSTART  0x30
 
-#define SUNXI_DMA_CFG_REG0  0x300
-#define SUNXI_DMA_SRC_START_ADDR_REG0   0x304
-#define SUNXI_DMA_DEST_START_ADDRR_REG0 0x308
-#define SUNXI_DMA_DDMA_BC_REG0  0x30C
-#define SUNXI_DMA_DDMA_PARA_REG00x318
-
-#define SUNXI_DMA_DDMA_CFG_REG_LOADING  (1 << 31)
-#define SUNXI_DMA_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 (2 << 25)
-#define SUNXI_DMA_DDMA_CFG_REG_DDMA_DST_DRQ_TYPE_DRAM (1 << 16)
-#define SUNXI_DMA_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 (2 << 9)
-#define SUNXI_DMA_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO (1 << 5)
-#define SUNXI_DMA_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC (3 << 0)
-
-#define SUNXI_DMA_DDMA_PARA_REG_SRC_WAIT_CYC (0x0F << 0)
-#define SUNXI_DMA_DDMA_PARA_REG_SRC_BLK_SIZE (0x7F << 8)
-
 struct nfc_config {
int page_size;
int ecc_strength;
@@ -268,86 +255,74 @@ static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 
102, 110, 116};
 static int nand_read_page(const struct nfc_config *conf, u32 offs,
  void *dest, int len)
 {
-   dma_addr_t dst = (dma_addr_t)dest;
int nsectors = len / conf->ecc_size;
u16 rand_seed = 0;
-   u32 val;
-   int page;
-
-   page = offs / conf->page_size;
+   int oob_chunk_sz = ecc_bytes[conf->ecc_strength];
+   int page = offs / conf->page_size;
+   u32 ecc_st;
+   int i;
 
if (offs % conf->page_size || len % conf->ecc_size ||
len > conf->page_size || len < 0)
return -EINVAL;
 
-   /* clear ecc status */
-   writel(0, SUNXI_NFC_BASE + NFC_ECC_ST);
-
/* Choose correct seed if randomized */
if (conf->randomize)
rand_seed = random_seed[page % conf->nseeds];
 
-   writel((rand_seed << 16) | (conf->ecc_strength << 12) |
-   (conf->randomize ? NFC_ECC_RANDOM_EN : 0) |
-   (conf->ecc_size == 512 ? NFC_ECC_BLOCK_SIZE : 0) |
-   NFC_ECC_EN | NFC_ECC_PIPELINE | NFC_ECC_EXCEPTION,
-   SUNXI_NFC_BASE + NFC_ECC_CTL);
-
-   flush_dcache_range(dst, ALIGN(dst + conf->ecc_size, ARCH_DMA_MINALIGN));
-
-   /* SUNXI_DMA */
-   writel(0x0, SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0); /* clr dma cmd */
-   /* read from REG_IO_DATA */
-   writel(SUNXI_NFC_BASE + NFC_IO_DATA,
-  SUNXI_DMA_BASE + SUNXI_DMA_SRC_START_ADDR_REG0);
-   /* read to RAM */
-   writel(dst, SUNXI_DMA_BASE + SUNXI_DMA_DEST_START_ADDRR_REG0);
-   writel(SUNXI_DMA_DDMA_PARA_REG_SRC_WAIT_CYC |
-  SUNXI_DMA_DDMA_PARA_REG_SRC_BLK_SIZE,
-  SUNXI_DMA_BASE + SUNXI_DMA_DDMA_PARA_REG0);
-   writel(len, SUNXI_DMA_BASE + SUNXI_DMA_DDMA_BC_REG0);
-   writel(SUNXI_DMA_DDMA_CFG_REG_LOADING |
-  SUNXI_DMA_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 |
-  SUNXI_DMA_DDMA_CFG_REG_DDMA_DST_DRQ_TYPE_DRAM |
-  SUNXI_DMA_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 |
-  SUNXI_DMA_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO |
-  SUNXI_DMA_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC,
-  SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0);
-
-   writel(nsectors, SUNXI_NFC_BASE +