Bring NAND support to Nintendo NES Classic

When no requirement in Device Tree is given about the ECC strength and step size, the engine should fallback on the minimal working case for this engine (16b/1024B) instead of the NAND chip requirement which might be simply unreachable.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/sunxi_nand.c | 1 +
 1 file changed, 1 insertion(+)

drivers/mtd/nand/sunxi_nand.c +0 -0

index 8bc3828854..e8e7ad8ac5 100644

Hi,

Quoted 18 lines · Miquel Raynal

When no requirement in Device Tree is given about the ECC strength and step size, the engine should fallback on the minimal working case for this engine (16b/1024B) instead of the NAND chip requirement which might be simply unreachable.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/sunxi_nand.c | 1 +
 1 file changed, 1 insertion(+)

drivers/mtd/nand/sunxi_nand.c +0 -0

index 8bc3828854..e8e7ad8ac5 100644
@@ -1417,6 +1417,7 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
 		goto err;
 	}
 

drivers/mtd/nand/sunxi_nand.c static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd, · line 1420

Quoted 1 line

drivers/mtd/nand/sunxi_nand.c +1 -0

+	ecc->strength = strengths[i];  

You should have a comment here as well mentionning why you're doing this (your commit log would be a good start :))

When no requirement in Device Tree is given about the ECC strength and step size, the engine should fallback on the minimal working case for this engine (16b/1024B) instead of the NAND chip requirement which might be simply unreachable.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/sunxi_nand.c | 1 +
 1 file changed, 1 insertion(+)

drivers/mtd/nand/sunxi_nand.c +1 -0

index 8bc3828854..e8e7ad8ac5 100644
@@ -1417,6 +1417,7 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
 		goto err;
 	}
 
+	ecc->strength = strengths[i];
 	data->mode = i;
 
 	/* HW ECC always request ECC bytes for 1024 bytes blocks */

Quoted 4 lines · On Wed, 24 Jan 2018 01:44:47 +0100 Miquel Raynal…

When no requirement in Device Tree is given about the ECC strength and step size, the engine should fallback on the minimal working case for this engine (16b/1024B) instead of the NAND chip requirement which might be simply unreachable.

Actually, this is not what this patch is fixing. It fixes all cases where the requested ECC strength does not exactly match the strengths supported by the ECC engine. In this case, the driver is selecting the closest strength meeting the 'selected_strength > requested_strength' constraint. The problem was that we were not updating ecc->strength with the real strength, which is what you're fixing here.

Quoted 19 lines

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/sunxi_nand.c | 1 +
 1 file changed, 1 insertion(+)

drivers/mtd/nand/sunxi_nand.c +1 -0

index 8bc3828854..e8e7ad8ac5 100644
@@ -1417,6 +1417,7 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
 		goto err;
 	}
 
+	ecc->strength = strengths[i];
 	data->mode = i;
 
 	/* HW ECC always request ECC bytes for 1024 bytes blocks */  

In the nand_read_buffer() step, the seed is calculated by doing a modulo by conf->nseeds which is always zero when not using the randomizer (most of SLC NANDs).

This situation turns out to lead to a run time freeze.

Derive this seed only when the randomizer is enabled (and conf->nseeds logically not zero).

Quoted 4 lines · Miquel Raynal

In the nand_read_buffer() step, the seed is calculated by doing a modulo by conf->nseeds which is always zero when not using the randomizer (most of SLC NANDs).

Quoted 4 lines

This situation turns out to lead to a run time freeze.

Derive this seed only when the randomizer is enabled (and conf->nseeds logically not zero).

Mentionning the first occurence of that issue in your commit log would be great, especially since the situation depends on configuration options.

Add some clocks/PLL definitions and the dependency on MACH_SUN8I in Kconfig so the SPL could be compiled to boards using A33 SoCs.

Signed-off-by Miquel Raynal <[email protected]>

Hi,

Quoted 4 lines · Miquel Raynal

Add some clocks/PLL definitions and the dependency on MACH_SUN8I in Kconfig so the SPL could be compiled to boards using A33 SoCs.

Signed-off-by Miquel Raynal <[email protected]>

The SPL can already be compiled for these platforms. What you're doing is allowing the NAND support to be compiled in the SPL.

Maxime

Do some cleaning in sunxi NAND SPL driver like adding helpers and clearing flags at the right spot

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/sunxi_nand_spl.c | 64 +++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 23 deletions(-)

drivers/mtd/nand/sunxi_nand_spl.c +0 -0

index 06695fc15f..2488d5cb51 100644

drivers/mtd/nand/sunxi_nand_spl.c +14 -0

@@ -155,6 +155,30 @@ static inline int check_value_negated(int offset, int unexpected_bits,
 	return check_value_inner(offset, unexpected_bits, timeout_us, 1);
 }
 
+static int nand_wait_cmd_fifo_empty(void)
+{
+	if (!check_value_negated(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_FIFO_STAT,
+				 DEFAULT_TIMEOUT_US)) {
+		printf("nand: timeout waiting for empty cmd FIFO\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int nand_wait_int(void)
+{
+	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,

drivers/mtd/nand/sunxi_nand_spl.c +0 -5

 	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
 	writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET,
 	       SUNXI_NFC_BASE + NFC_CMD);
 
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error timeout waiting for nand reset\n");
-		return;
-	}

drivers/mtd/nand/sunxi_nand_spl.c +0 -4

 	       SUNXI_NFC_BASE + NFC_CMD);
 
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error while initializing dma interrupt\n");
-		return -EIO;

drivers/mtd/nand/sunxi_nand_spl.c +4 -7

 	writel(0, SUNXI_NFC_BASE + NFC_ADDR_LOW);
 	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD |
-	       (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR | NFC_CMD_RNDOUT,
+	       (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT,
 	       SUNXI_NFC_BASE + NFC_CMD);
 
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error while initializing dma interrupt\n");
-		return -1;
-	}
+	return nand_wait_int();
 
-	return 0;
 }
 
+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)
 {

Hi,

Quoted 4 lines · Miquel Raynal

Do some cleaning in sunxi NAND SPL driver like adding helpers and clearing flags at the right spot

Signed-off-by Miquel Raynal <[email protected]>

I'm not really fond of these kind of wildcard patches, especially since..

Overall, I think it would be great to split that patch into 4:
- the first one to fix the ADR vs ADDR typo (without changing the order)
- the second one to introduce the nand_wait_int helper
- the third one to introduce the nand_wait_cmd_fifo_empty, with a proper commit log explaining why this needs to be done and which issues is it fixing
- And maybe if justified moving the ecc_bytes array to global scope

Do some cleaning in sunxi NAND SPL driver like adding helpers and clearing flags at the right spot

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/sunxi_nand_spl.c | 64 +++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 23 deletions(-)

drivers/mtd/nand/sunxi_nand_spl.c +0 -0

index 06695fc15f..2488d5cb51 100644

drivers/mtd/nand/sunxi_nand_spl.c +10 -0

 #define NFC_DATA_TRANS             (1 << 21)
 #define NFC_SEND_CMD1              (1 << 22)
 #define NFC_WAIT_FLAG              (1 << 23)

@@ -155,6 +155,30 @@ static inline int check_value_negated(int offset, int unexpected_bits,
 	return check_value_inner(offset, unexpected_bits, timeout_us, 1);
 }
 
+static int nand_wait_cmd_fifo_empty(void)
+{
+	if (!check_value_negated(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_FIFO_STAT,
+				 DEFAULT_TIMEOUT_US)) {
+		printf("nand: timeout waiting for empty cmd FIFO\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}

drivers/mtd/nand/sunxi_nand_spl.c +6 -0

+
+	return 0;
+}
+
 void nand_init(void)
 {
 	uint32_t val;

@@ -172,22 +196,21 @@ void nand_init(void)
 	}
 
 	/* reset NAND */
+	nand_wait_cmd_fifo_empty();
+
 	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);

drivers/mtd/nand/sunxi_nand_spl.c +9 -8

 }
 
 static void nand_apply_config(const struct nfc_config *conf)
 {
 	u32 val;
 
+	nand_wait_cmd_fifo_empty();
+
 	val = readl(SUNXI_NFC_BASE + NFC_CTL);
 	val &= ~NFC_CTL_PAGE_SIZE_MASK;
 	writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size),

@@ -200,6 +223,8 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs)
 {
 	int page = offs / conf->page_size;
 
+	nand_wait_cmd_fifo_empty();
+
 	writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
 	       (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
 	       (NFC_CMD_READSTART << NFC_READ_CMD_OFFSET),

@@ -208,38 +233,32 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs)
 	writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_ADDR_HIGH);
 	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
 	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | NFC_WAIT_FLAG |
-	       ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR,
+	       ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR,
 	       SUNXI_NFC_BASE + NFC_CMD);
 
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error while initializing dma interrupt\n");
-		return -EIO;
-	}
-
-	return 0;
+	return nand_wait_int();
 }
 
 static int nand_reset_column(void)
 {
+	nand_wait_cmd_fifo_empty();
+
 	writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
 	       (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
 	       (NFC_CMD_RNDOUTSTART << NFC_READ_CMD_OFFSET),
 	       SUNXI_NFC_BASE + NFC_RCMD_SET);
+	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
 	writel(0, SUNXI_NFC_BASE + NFC_ADDR_LOW);
 	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD |

drivers/mtd/nand/sunxi_nand_spl.c +0 -1

 static int nand_read_page(const struct nfc_config *conf, u32 offs,
 			  void *dest, int len)
 {

@@ -327,7 +346,6 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
 
 static int nand_max_ecc_strength(struct nfc_config *conf)
 {
-	static const int ecc_bytes[] = { 32, 46, 54, 60, 74, 88, 102, 110, 116 };
 	int max_oobsize, max_ecc_bytes;
 	int nsectors = conf->page_size / conf->ecc_size;
 	int i;

Quoted 15 lines · On Wed, 24 Jan 2018 01:44:50 +0100 Miquel Raynal…

Do some cleaning in sunxi NAND SPL driver like adding helpers and clearing flags at the right spot

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/sunxi_nand_spl.c | 64 +++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 23 deletions(-)

drivers/mtd/nand/sunxi_nand_spl.c +0 -0

index 06695fc15f..2488d5cb51 100644
@@ -55,8 +55,8 @@
 
 

drivers/mtd/nand/sunxi_nand_spl.c line 59

Quoted 4 lines

drivers/mtd/nand/sunxi_nand_spl.c +1 -1

 #define NFC_ADDR_NUM_OFFSET        16
-#define NFC_SEND_ADR               (1 << 19)
 #define NFC_ACCESS_DIR             (1 << 20)
+#define NFC_SEND_ADDR              (1 << 19)  

This typo fix probably deserves a separate patch.

Quoted 23 lines

drivers/mtd/nand/sunxi_nand_spl.c +15 -0

 #define NFC_DATA_TRANS             (1 << 21)
 #define NFC_SEND_CMD1              (1 << 22)
 #define NFC_WAIT_FLAG              (1 << 23)

@@ -155,6 +155,30 @@ static inline int check_value_negated(int offset, int unexpected_bits,
 	return check_value_inner(offset, unexpected_bits, timeout_us, 1);
 }
 
+static int nand_wait_cmd_fifo_empty(void)
+{
+	if (!check_value_negated(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_FIFO_STAT,
+				 DEFAULT_TIMEOUT_US)) {
+		printf("nand: timeout waiting for empty cmd FIFO\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int nand_wait_int(void)
+{
+	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
+			 DEFAULT_TIMEOUT_US)) {

drivers/mtd/nand/sunxi_nand_spl.c static inline int check_value_negated(int offset, int unexpected_bits, · line 177

Quoted 5 lines

drivers/mtd/nand/sunxi_nand_spl.c +5 -0

+		printf("nand: timeout waiting for interruption\n");
+		return -ETIMEDOUT;
+	}
+
+	udelay(1);  

I'm pretty sure this udelay() is not required. Probably some leftover of your debug session ;-).

Quoted 15 lines

drivers/mtd/nand/sunxi_nand_spl.c +6 -0

+
+	return 0;
+}
+
 void nand_init(void)
 {
 	uint32_t val;

@@ -172,22 +196,21 @@ void nand_init(void)
 	}
 
 	/* reset NAND */
+	nand_wait_cmd_fifo_empty();
+
 	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);

drivers/mtd/nand/sunxi_nand_spl.c void nand_init(void) · line 205

Quoted 10 lines

drivers/mtd/nand/sunxi_nand_spl.c +1 -6

 	writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET,
 	       SUNXI_NFC_BASE + NFC_CMD);
 
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error timeout waiting for nand reset\n");
-		return;
-	}
-	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+	nand_wait_int();  

I would go even further and create an helper that takes care of the 'wait_fifo_empty+clear_int+write_cmd+wait_int' sequence:

static int nand_exec_cmd(u32 cmd)

{
	int ret;

	ret = nand_wait_cmd_fifo_empty();
	if (ret)
		return ret;

	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
	writel(cmd, SUNXI_NFC_BASE + NFC_CMD);

	return nand_wait_int();
}

Quoted 54 lines

drivers/mtd/nand/sunxi_nand_spl.c +10 -9

 }
 
 static void nand_apply_config(const struct nfc_config *conf)
 {
 	u32 val;
 
+	nand_wait_cmd_fifo_empty();
+
 	val = readl(SUNXI_NFC_BASE + NFC_CTL);
 	val &= ~NFC_CTL_PAGE_SIZE_MASK;
 	writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size),

@@ -200,6 +223,8 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs)
 {
 	int page = offs / conf->page_size;
 
+	nand_wait_cmd_fifo_empty();
+
 	writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
 	       (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
 	       (NFC_CMD_READSTART << NFC_READ_CMD_OFFSET),

@@ -208,38 +233,32 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs)
 	writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_ADDR_HIGH);
 	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
 	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | NFC_WAIT_FLAG |
-	       ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR,
+	       ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR,
 	       SUNXI_NFC_BASE + NFC_CMD);
 
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error while initializing dma interrupt\n");
-		return -EIO;
-	}
-
-	return 0;
+	return nand_wait_int();
 }
 
 static int nand_reset_column(void)
 {
+	nand_wait_cmd_fifo_empty();
+
 	writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
 	       (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
 	       (NFC_CMD_RNDOUTSTART << NFC_READ_CMD_OFFSET),
 	       SUNXI_NFC_BASE + NFC_RCMD_SET);
+	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
 	writel(0, SUNXI_NFC_BASE + NFC_ADDR_LOW);
 	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD |
-	       (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR | NFC_CMD_RNDOUT,
+	       (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT,
 	       SUNXI_NFC_BASE + NFC_CMD);
 

drivers/mtd/nand/sunxi_nand_spl.c static int nand_load_page(const struct nfc_config *conf, u32 offs) · line 256

Quoted 6 lines

drivers/mtd/nand/sunxi_nand_spl.c +1 -5

-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error while initializing dma interrupt\n");
-		return -1;
-	}
+	return nand_wait_int();  

Here, I think you should wait, just to make sure we don't read data before tCCS. udelay(1) should be enough.

drivers/mtd/nand/sunxi_nand_spl.c static int nand_load_page(const struct nfc_config *conf, u32 offs) · line 261

Quoted 7 lines

drivers/mtd/nand/sunxi_nand_spl.c +2 -1

-	return 0;
 }
 
+static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116};
+  

Putting ecc_bytes out of nand_max_ecc_strength() is not really related to this patch, and should probably go in patch 5.

Quoted 6 lines

drivers/mtd/nand/sunxi_nand_spl.c +0 -0

 static int nand_read_page(const struct nfc_config *conf, u32 offs,
 			  void *dest, int len)
 {

@@ -327,7 +346,6 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
 

drivers/mtd/nand/sunxi_nand_spl.c static int nand_read_page(const struct nfc_config *conf, u32 offs, · line 351

Quoted 6 lines

drivers/mtd/nand/sunxi_nand_spl.c +0 -1

 static int nand_max_ecc_strength(struct nfc_config *conf)
 {
-	static const int ecc_bytes[] = { 32, 46, 54, 60, 74, 88, 102, 110, 116 };
 	int max_oobsize, max_ecc_bytes;
 	int nsectors = conf->page_size / conf->ecc_size;
 	int i;  

Because using DMA implementation is not generic and was not developped to work on SoCs like A33, migrate the SPL to use PIO.

Signed-off-by Miquel Raynal <[email protected]>

---
 board/sunxi/board.c               |   4 +-
 drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++---------------------
 2 files changed, 79 insertions(+), 92 deletions(-)

board/sunxi/board.c +0 -0

index 70e01437c4..512e2c17a6 100644

board/sunxi/board.c +0 -0

 #endif
 	setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
 }

drivers/mtd/nand/sunxi_nand_spl.c +3 -18

index 2488d5cb51..5de6825994 100644
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <config.h>
 #include <nand.h>
+#include <linux/ctype.h>
 
 /* registers */
 #define NFC_CTL                    0x00000000

@@ -45,32 +46,22 @@
 #define NFC_CTL_PAGE_SIZE_MASK     (0xf << 8)
 #define NFC_CTL_PAGE_SIZE(a)       ((fls(a) - 11) << 8)
 
-
 #define NFC_ECC_EN                 (1 << 0)
-#define NFC_ECC_PIPELINE           (1 << 3)
 #define NFC_ECC_EXCEPTION          (1 << 4)
 #define NFC_ECC_BLOCK_SIZE         (1 << 5)
 #define NFC_ECC_RANDOM_EN          (1 << 9)
-#define NFC_ECC_RANDOM_DIRECTION   (1 << 10)
-
 
 #define NFC_ADDR_NUM_OFFSET        16
-#define NFC_ACCESS_DIR             (1 << 20)
 #define NFC_SEND_ADDR              (1 << 19)
 #define NFC_DATA_TRANS             (1 << 21)
 #define NFC_SEND_CMD1              (1 << 22)
 #define NFC_WAIT_FLAG              (1 << 23)
 #define NFC_SEND_CMD2              (1 << 24)
-#define NFC_SEQ                    (1 << 25)
-#define NFC_DATA_SWAP_METHOD       (1 << 26)
-#define NFC_ROW_AUTO_INC           (1 << 27)
-#define NFC_SEND_CMD3              (1 << 28)
-#define NFC_SEND_CMD4              (1 << 29)
 #define NFC_RAW_CMD                (0 << 30)
-#define NFC_PAGE_CMD               (2 << 30)
+#define NFC_ECC_CMD                (1 << 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 +71,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_REG0        0x318
-

drivers/mtd/nand/sunxi_nand_spl.c +38 -39

 struct nfc_config {
 	int page_size;
 	int ecc_strength;

@@ -254,7 +229,23 @@ static int nand_reset_column(void)
 	       SUNXI_NFC_BASE + NFC_CMD);
 
 	return nand_wait_int();
+}
 
+static int nand_change_column(u16 column)
+{
+	nand_wait_cmd_fifo_empty();
+
+	writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
+	       (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
+	       (NFC_CMD_RNDOUTSTART << NFC_READ_CMD_OFFSET),
+	       SUNXI_NFC_BASE + NFC_RCMD_SET);
+	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+	writel(column, SUNXI_NFC_BASE + NFC_ADDR_LOW);
+	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD |
+	       (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT,
+	       SUNXI_NFC_BASE + NFC_CMD);
+
+	return nand_wait_int();
 }
 
 static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116};

@@ -262,86 +253,80 @@ 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 + NFC_SECTOR_NUM);
-	writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
-	writel(NFC_DATA_TRANS |	NFC_PAGE_CMD | NFC_DATA_SWAP_METHOD,
-	       SUNXI_NFC_BASE + NFC_CMD);
+	/* Retrieve data from SRAM (PIO) */
+	for (i = 0; i < nsectors; i++) {
+		int data_off = i * conf->ecc_size;
+		int oob_off = conf->page_size + (i * oob_chunk_sz);
+		u8 *data = dest + data_off;
+
+		/* Clear ECC status and restart ECC engine */
+		writel(0, SUNXI_NFC_BASE + NFC_ECC_ST);
+		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_EXCEPTION,
+		       SUNXI_NFC_BASE + NFC_ECC_CTL);
+
+		/* Move the data in SRAM */
+		nand_change_column(data_off);
+		nand_wait_cmd_fifo_empty();
+		writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);

drivers/mtd/nand/sunxi_nand_spl.c +6 -0

+		/*
+		 * Let the ECC engine consume the ECC bytes and possibly correct
+		 * the data.
+		 */
+		nand_change_column(oob_off);
+		nand_wait_cmd_fifo_empty();

Hi,

Quoted 4 lines · Miquel Raynal

Because using DMA implementation is not generic and was not developped to work on SoCs like A33, migrate the SPL to use PIO.

Signed-off-by Miquel Raynal <[email protected]>

I guess the issue is slightly different than what you're saying, or at least should be expressed differently.

The issue is that the SPL support has be done to support only the earlier generations of Allwinner SoCs, and 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 hasn't changed.

There's two pathes forward, the first one would be to add support for that DMA controller too, or you can just remove the DMA usage entirely and rely on PIO that will make the driver simpler.

Explaining why you chose PIO would be great too.

Quoted 10 lines

---
 board/sunxi/board.c               |   4 +-
 drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++---------------------
 2 files changed, 79 insertions(+), 92 deletions(-)

board/sunxi/board.c +0 -0

index 70e01437c4..512e2c17a6 100644

board/sunxi/board.c static void nand_clock_setup(void) · line 270

Quoted 6 lines

board/sunxi/board.c +2 -1

@@ -266,11 +266,13 @@ static void nand_clock_setup(void)
 	struct sunxi_ccm_reg *const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
 
-	setbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
+	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_NAND0));
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_NAND0));  

That has nothing to do with PIO vs DMA. It should be in another patch.

board/sunxi/board.c static void nand_clock_setup(void) · line 275

Quoted 5 lines

board/sunxi/board.c +1 -0

 #ifdef CONFIG_MACH_SUN9I
 	setbits_le32(&ccm->ahb_gate1, (1 << AHB_GATE_OFFSET_DMA));
 #else
 	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_DMA));
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_DMA));  

So you're de-asserting the reset line for the DMA controller, while the commit is about removing the need for that DMA controller ? :)

That whole block can actually be removed (in a separate patch) now that you're not using DMA anymore.

Quoted 44 lines

board/sunxi/board.c +0 -0

 #endif
 	setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
 }

drivers/mtd/nand/sunxi_nand_spl.c +2 -11

index 2488d5cb51..5de6825994 100644
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <config.h>
 #include <nand.h>
+#include <linux/ctype.h>
 
 /* registers */
 #define NFC_CTL                    0x00000000

@@ -45,32 +46,22 @@
 #define NFC_CTL_PAGE_SIZE_MASK     (0xf << 8)
 #define NFC_CTL_PAGE_SIZE(a)       ((fls(a) - 11) << 8)
 
-
 #define NFC_ECC_EN                 (1 << 0)
-#define NFC_ECC_PIPELINE           (1 << 3)
 #define NFC_ECC_EXCEPTION          (1 << 4)
 #define NFC_ECC_BLOCK_SIZE         (1 << 5)
 #define NFC_ECC_RANDOM_EN          (1 << 9)
-#define NFC_ECC_RANDOM_DIRECTION   (1 << 10)
-
 
 #define NFC_ADDR_NUM_OFFSET        16
-#define NFC_ACCESS_DIR             (1 << 20)
 #define NFC_SEND_ADDR              (1 << 19)
 #define NFC_DATA_TRANS             (1 << 21)
 #define NFC_SEND_CMD1              (1 << 22)
 #define NFC_WAIT_FLAG              (1 << 23)
 #define NFC_SEND_CMD2              (1 << 24)
-#define NFC_SEQ                    (1 << 25)
-#define NFC_DATA_SWAP_METHOD       (1 << 26)
-#define NFC_ROW_AUTO_INC           (1 << 27)
-#define NFC_SEND_CMD3              (1 << 28)
-#define NFC_SEND_CMD4              (1 << 29)
 #define NFC_RAW_CMD                (0 << 30)
-#define NFC_PAGE_CMD               (2 << 30)
+#define NFC_ECC_CMD                (1 << 30)
 

drivers/mtd/nand/sunxi_nand_spl.c line 64

Quoted 3 lines

drivers/mtd/nand/sunxi_nand_spl.c +1 -1

 #define NFC_ST_CMD_INT_FLAG        (1 << 1)
-#define NFC_ST_DMA_INT_FLAG        (1 << 2)
+#define NFC_ST_CMD_FIFO_STAT       (1 << 3)

Quoted 19 lines

drivers/mtd/nand/sunxi_nand_spl.c +0 -12

 #define NFC_READ_CMD_OFFSET         0
 #define NFC_RANDOM_READ_CMD0_OFFSET 8

@@ -80,22 +71,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_REG0        0x318
-
-#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)

drivers/mtd/nand/sunxi_nand_spl.c line 98

Quoted 4 lines

drivers/mtd/nand/sunxi_nand_spl.c +0 -4

-
-#define SUNXI_DMA_DDMA_PARA_REG_SRC_WAIT_CYC (0x0F << 0)
-#define SUNXI_DMA_DDMA_PARA_REG_SRC_BLK_SIZE (0x7F << 8)
-  

We should probably keep those values. They're not used anymore, but the NAND controller has been under-documented on most of the datasheet we've had. It's good to keep it at least for reference.

Quoted 19 lines

Quoted 19 lines

#define NFC_READ_CMD_OFFSET 0
#define NFC_RANDOM_READ_CMD0_OFFSET 8

drivers/mtd/nand/sunxi_nand_spl.c +0 -12

@@ -80,22 +71,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_REG0        0x318
-
-#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)

Quoted 9 lines

drivers/mtd/nand/sunxi_nand_spl.c line 98

Quoted 4 lines

drivers/mtd/nand/sunxi_nand_spl.c +0 -4

-
-#define SUNXI_DMA_DDMA_PARA_REG_SRC_WAIT_CYC (0x0F << 0)
-#define SUNXI_DMA_DDMA_PARA_REG_SRC_BLK_SIZE (0x7F << 8)
-  

We should probably keep those values. They're not used anymore, but the NAND controller has been under-documented on most of the datasheet we've had. It's good to keep it at least for reference.

Hm, your statement is valid for the NFC_ defs, but these are purely DMA engine related definitions, so I think we can get rid of them.

Hi,

Quoted 4 lines · Miquel Raynal

Because using DMA implementation is not generic and was not developped to work on SoCs like A33, migrate the SPL to use PIO.

Signed-off-by Miquel Raynal <[email protected]>

I guess the issue is slightly different than what you're saying, or at least should be expressed differently.

The issue is that the SPL support has be done to support only the earlier generations of Allwinner SoCs, and 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 hasn't changed.

There's two pathes forward, the first one would be to add support for that DMA controller too, or you can just remove the DMA usage entirely and rely on PIO that will make the driver simpler.

Explaining why you chose PIO would be great too.

Quoted 13 lines

---
 board/sunxi/board.c               |   4 +-
 drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++---------------------
 2 files changed, 79 insertions(+), 92 deletions(-)

board/sunxi/board.c +0 -0

index 70e01437c4..512e2c17a6 100644
@@ -266,11 +266,13 @@ static void nand_clock_setup(void)
 	struct sunxi_ccm_reg *const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
 

board/sunxi/board.c static void nand_clock_setup(void) · line 270

Quoted 3 lines

board/sunxi/board.c +2 -1

-	setbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
+	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_NAND0));
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_NAND0));  

That has nothing to do with PIO vs DMA. It should be in another patch.

board/sunxi/board.c static void nand_clock_setup(void) · line 275

Quoted 6 lines

board/sunxi/board.c +1 -0

 #ifdef CONFIG_MACH_SUN9I
 	setbits_le32(&ccm->ahb_gate1, (1 << AHB_GATE_OFFSET_DMA));
 #else
 	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_DMA));
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_DMA));  

So you're de-asserting the reset line for the DMA controller, while the commit is about removing the need for that DMA controller ? :)

That whole block can actually be removed (in a separate patch) now that you're not using DMA anymore.

Quoted 67 lines

board/sunxi/board.c +0 -0

 #endif
 	setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
 }

drivers/mtd/nand/sunxi_nand_spl.c +3 -24

index 2488d5cb51..5de6825994 100644
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <config.h>
 #include <nand.h>
+#include <linux/ctype.h>
 
 /* registers */
 #define NFC_CTL                    0x00000000

@@ -45,32 +46,22 @@
 #define NFC_CTL_PAGE_SIZE_MASK     (0xf << 8)
 #define NFC_CTL_PAGE_SIZE(a)       ((fls(a) - 11) << 8)
 
-
 #define NFC_ECC_EN                 (1 << 0)
-#define NFC_ECC_PIPELINE           (1 << 3)
 #define NFC_ECC_EXCEPTION          (1 << 4)
 #define NFC_ECC_BLOCK_SIZE         (1 << 5)
 #define NFC_ECC_RANDOM_EN          (1 << 9)
-#define NFC_ECC_RANDOM_DIRECTION   (1 << 10)
-
 
 #define NFC_ADDR_NUM_OFFSET        16
-#define NFC_ACCESS_DIR             (1 << 20)
 #define NFC_SEND_ADDR              (1 << 19)
 #define NFC_DATA_TRANS             (1 << 21)
 #define NFC_SEND_CMD1              (1 << 22)
 #define NFC_WAIT_FLAG              (1 << 23)
 #define NFC_SEND_CMD2              (1 << 24)
-#define NFC_SEQ                    (1 << 25)
-#define NFC_DATA_SWAP_METHOD       (1 << 26)
-#define NFC_ROW_AUTO_INC           (1 << 27)
-#define NFC_SEND_CMD3              (1 << 28)
-#define NFC_SEND_CMD4              (1 << 29)
 #define NFC_RAW_CMD                (0 << 30)
-#define NFC_PAGE_CMD               (2 << 30)
+#define NFC_ECC_CMD                (1 << 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 +71,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_REG0        0x318
-
-#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)

drivers/mtd/nand/sunxi_nand_spl.c line 98

Quoted 4 lines

drivers/mtd/nand/sunxi_nand_spl.c +0 -4

-
-#define SUNXI_DMA_DDMA_PARA_REG_SRC_WAIT_CYC (0x0F << 0)
-#define SUNXI_DMA_DDMA_PARA_REG_SRC_BLK_SIZE (0x7F << 8)
-  

We should probably keep those values. They're not used anymore, but the NAND controller has been under-documented on most of the datasheet we've had. It's good to keep it at least for reference.

Quoted 108 lines

drivers/mtd/nand/sunxi_nand_spl.c +41 -39

 struct nfc_config {
 	int page_size;
 	int ecc_strength;

@@ -254,7 +229,23 @@ static int nand_reset_column(void)
 	       SUNXI_NFC_BASE + NFC_CMD);
 
 	return nand_wait_int();
+}
 
+static int nand_change_column(u16 column)
+{
+	nand_wait_cmd_fifo_empty();
+
+	writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
+	       (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
+	       (NFC_CMD_RNDOUTSTART << NFC_READ_CMD_OFFSET),
+	       SUNXI_NFC_BASE + NFC_RCMD_SET);
+	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+	writel(column, SUNXI_NFC_BASE + NFC_ADDR_LOW);
+	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD |
+	       (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT,
+	       SUNXI_NFC_BASE + NFC_CMD);
+
+	return nand_wait_int();
 }
 
 static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116};

@@ -262,86 +253,80 @@ 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 + NFC_SECTOR_NUM);
-	writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
-	writel(NFC_DATA_TRANS |	NFC_PAGE_CMD | NFC_DATA_SWAP_METHOD,
-	       SUNXI_NFC_BASE + NFC_CMD);
+	/* Retrieve data from SRAM (PIO) */
+	for (i = 0; i < nsectors; i++) {
+		int data_off = i * conf->ecc_size;
+		int oob_off = conf->page_size + (i * oob_chunk_sz);
+		u8 *data = dest + data_off;
+
+		/* Clear ECC status and restart ECC engine */
+		writel(0, SUNXI_NFC_BASE + NFC_ECC_ST);
+		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_EXCEPTION,
+		       SUNXI_NFC_BASE + NFC_ECC_CTL);
+
+		/* Move the data in SRAM */
+		nand_change_column(data_off);
+		nand_wait_cmd_fifo_empty();
+		writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+		writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT);
+		writel(NFC_DATA_TRANS, SUNXI_NFC_BASE + NFC_CMD);
+		nand_wait_int();
 

drivers/mtd/nand/sunxi_nand_spl.c static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; · line 321

Quoted 6 lines

drivers/mtd/nand/sunxi_nand_spl.c +0 -6

-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_DMA_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error while initializing dma interrupt\n");
-		return -EIO;
-	}
-	writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);  

It seems that you're also converting this code to some of your new helpers. This should be done in the patch introducing them.

Quoted 11 lines

drivers/mtd/nand/sunxi_nand_spl.c +9 -0

+		/*
+		 * Let the ECC engine consume the ECC bytes and possibly correct
+		 * the data.
+		 */
+		nand_change_column(oob_off);
+		nand_wait_cmd_fifo_empty();
+		writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+		writel(NFC_DATA_TRANS | NFC_ECC_CMD, SUNXI_NFC_BASE + NFC_CMD);
+		nand_wait_int();
 

drivers/mtd/nand/sunxi_nand_spl.c static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; · line 328

Quoted 6 lines

drivers/mtd/nand/sunxi_nand_spl.c +0 -6

-	if (!check_value_negated(SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0,
-				 SUNXI_DMA_DDMA_CFG_REG_LOADING,
-				 DEFAULT_TIMEOUT_US)) {
-		printf("Error while waiting for dma transfer to finish\n");
-		return -EIO;
-	}  

Ditto.

Because using DMA implementation is not generic and was not developped to work on SoCs like A33, migrate the SPL to use PIO.

Signed-off-by Miquel Raynal <[email protected]>

---
 board/sunxi/board.c               |   4 +-
 drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++---------------------
 2 files changed, 79 insertions(+), 92 deletions(-)

board/sunxi/board.c +3 -1

index 70e01437c4..512e2c17a6 100644
@@ -266,11 +266,13 @@ static void nand_clock_setup(void)
 	struct sunxi_ccm_reg *const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
 
-	setbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
+	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_NAND0));
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_NAND0));
 #ifdef CONFIG_MACH_SUN9I
 	setbits_le32(&ccm->ahb_gate1, (1 << AHB_GATE_OFFSET_DMA));
 #else
 	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_DMA));
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_DMA));
 #endif
 	setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
 }

drivers/mtd/nand/sunxi_nand_spl.c +10 -28

index 2488d5cb51..5de6825994 100644
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <config.h>
 #include <nand.h>
+#include <linux/ctype.h>
 
 /* registers */
 #define NFC_CTL                    0x00000000

@@ -45,32 +46,22 @@
 #define NFC_CTL_PAGE_SIZE_MASK     (0xf << 8)
 #define NFC_CTL_PAGE_SIZE(a)       ((fls(a) - 11) << 8)
 
-
 #define NFC_ECC_EN                 (1 << 0)
-#define NFC_ECC_PIPELINE           (1 << 3)
 #define NFC_ECC_EXCEPTION          (1 << 4)
 #define NFC_ECC_BLOCK_SIZE         (1 << 5)
 #define NFC_ECC_RANDOM_EN          (1 << 9)
-#define NFC_ECC_RANDOM_DIRECTION   (1 << 10)
-
 
 #define NFC_ADDR_NUM_OFFSET        16
-#define NFC_ACCESS_DIR             (1 << 20)
 #define NFC_SEND_ADDR              (1 << 19)
 #define NFC_DATA_TRANS             (1 << 21)
 #define NFC_SEND_CMD1              (1 << 22)
 #define NFC_WAIT_FLAG              (1 << 23)
 #define NFC_SEND_CMD2              (1 << 24)
-#define NFC_SEQ                    (1 << 25)
-#define NFC_DATA_SWAP_METHOD       (1 << 26)
-#define NFC_ROW_AUTO_INC           (1 << 27)
-#define NFC_SEND_CMD3              (1 << 28)
-#define NFC_SEND_CMD4              (1 << 29)
 #define NFC_RAW_CMD                (0 << 30)
-#define NFC_PAGE_CMD               (2 << 30)
+#define NFC_ECC_CMD                (1 << 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 +71,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_REG0        0x318
-
-#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;

@@ -254,7 +229,23 @@ static int nand_reset_column(void)
 	       SUNXI_NFC_BASE + NFC_CMD);
 
 	return nand_wait_int();
+}
 
+static int nand_change_column(u16 column)
+{
+	nand_wait_cmd_fifo_empty();
+
+	writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
+	       (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |

drivers/mtd/nand/sunxi_nand_spl.c +34 -49

 static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116};

@@ -262,86 +253,80 @@ 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 + NFC_SECTOR_NUM);
-	writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
-	writel(NFC_DATA_TRANS |	NFC_PAGE_CMD | NFC_DATA_SWAP_METHOD,
-	       SUNXI_NFC_BASE + NFC_CMD);
+	/* Retrieve data from SRAM (PIO) */
+	for (i = 0; i < nsectors; i++) {
+		int data_off = i * conf->ecc_size;
+		int oob_off = conf->page_size + (i * oob_chunk_sz);
+		u8 *data = dest + data_off;
+
+		/* Clear ECC status and restart ECC engine */
+		writel(0, SUNXI_NFC_BASE + NFC_ECC_ST);
+		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_EXCEPTION,
+		       SUNXI_NFC_BASE + NFC_ECC_CTL);
+
+		/* Move the data in SRAM */
+		nand_change_column(data_off);
+		nand_wait_cmd_fifo_empty();
+		writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+		writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT);
+		writel(NFC_DATA_TRANS, SUNXI_NFC_BASE + NFC_CMD);
+		nand_wait_int();
 
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_DMA_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error while initializing dma interrupt\n");
-		return -EIO;
-	}
-	writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+		/*
+		 * Let the ECC engine consume the ECC bytes and possibly correct
+		 * the data.
+		 */
+		nand_change_column(oob_off);
+		nand_wait_cmd_fifo_empty();
+		writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+		writel(NFC_DATA_TRANS | NFC_ECC_CMD, SUNXI_NFC_BASE + NFC_CMD);
+		nand_wait_int();
 
-	if (!check_value_negated(SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0,
-				 SUNXI_DMA_DDMA_CFG_REG_LOADING,
-				 DEFAULT_TIMEOUT_US)) {
-		printf("Error while waiting for dma transfer to finish\n");

drivers/mtd/nand/sunxi_nand_spl.c +6 -6

-	val = readl(SUNXI_NFC_BASE + NFC_ECC_ST);
+		/* Stop ECC engine */
+		writel(readl(SUNXI_NFC_BASE + NFC_ECC_CTL) & ~NFC_ECC_EN,
+		       SUNXI_NFC_BASE + NFC_ECC_CTL);
 
-	/* ECC error detected. */
-	if (val & 0xffff)
-		return -EIO;
+		/* ECC error detected. */
+		if (ecc_st & 0xffff)
+			return -EIO;
 
-	/*
-	 * Return 1 if the page is empty.

drivers/mtd/nand/sunxi_nand_spl.c +6 -0

+
+		if (data_off + conf->ecc_size >= len)
+			break;
+	}
+
+	return 0;
 }
 
 static int nand_max_ecc_strength(struct nfc_config *conf)

Quoted 117 lines · On Wed, 24 Jan 2018 01:44:51 +0100 Miquel Raynal…

Because using DMA implementation is not generic and was not developped to work on SoCs like A33, migrate the SPL to use PIO.

Signed-off-by Miquel Raynal <[email protected]>

---
 board/sunxi/board.c               |   4 +-
 drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++---------------------
 2 files changed, 79 insertions(+), 92 deletions(-)

board/sunxi/board.c +3 -1

index 70e01437c4..512e2c17a6 100644
@@ -266,11 +266,13 @@ static void nand_clock_setup(void)
 	struct sunxi_ccm_reg *const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
 
-	setbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
+	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_NAND0));
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_NAND0));
 #ifdef CONFIG_MACH_SUN9I
 	setbits_le32(&ccm->ahb_gate1, (1 << AHB_GATE_OFFSET_DMA));
 #else
 	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_DMA));
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_DMA));
 #endif
 	setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
 }

drivers/mtd/nand/sunxi_nand_spl.c +17 -28

index 2488d5cb51..5de6825994 100644
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <config.h>
 #include <nand.h>
+#include <linux/ctype.h>
 
 /* registers */
 #define NFC_CTL                    0x00000000

@@ -45,32 +46,22 @@
 #define NFC_CTL_PAGE_SIZE_MASK     (0xf << 8)
 #define NFC_CTL_PAGE_SIZE(a)       ((fls(a) - 11) << 8)
 
-
 #define NFC_ECC_EN                 (1 << 0)
-#define NFC_ECC_PIPELINE           (1 << 3)
 #define NFC_ECC_EXCEPTION          (1 << 4)
 #define NFC_ECC_BLOCK_SIZE         (1 << 5)
 #define NFC_ECC_RANDOM_EN          (1 << 9)
-#define NFC_ECC_RANDOM_DIRECTION   (1 << 10)
-
 
 #define NFC_ADDR_NUM_OFFSET        16
-#define NFC_ACCESS_DIR             (1 << 20)
 #define NFC_SEND_ADDR              (1 << 19)
 #define NFC_DATA_TRANS             (1 << 21)
 #define NFC_SEND_CMD1              (1 << 22)
 #define NFC_WAIT_FLAG              (1 << 23)
 #define NFC_SEND_CMD2              (1 << 24)
-#define NFC_SEQ                    (1 << 25)
-#define NFC_DATA_SWAP_METHOD       (1 << 26)
-#define NFC_ROW_AUTO_INC           (1 << 27)
-#define NFC_SEND_CMD3              (1 << 28)
-#define NFC_SEND_CMD4              (1 << 29)
 #define NFC_RAW_CMD                (0 << 30)
-#define NFC_PAGE_CMD               (2 << 30)
+#define NFC_ECC_CMD                (1 << 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 +71,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_REG0        0x318
-
-#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;

@@ -254,7 +229,23 @@ static int nand_reset_column(void)
 	       SUNXI_NFC_BASE + NFC_CMD);
 
 	return nand_wait_int();
+}
 
+static int nand_change_column(u16 column)
+{
+	nand_wait_cmd_fifo_empty();
+
+	writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
+	       (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
+	       (NFC_CMD_RNDOUTSTART << NFC_READ_CMD_OFFSET),
+	       SUNXI_NFC_BASE + NFC_RCMD_SET);
+	writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+	writel(column, SUNXI_NFC_BASE + NFC_ADDR_LOW);
+	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD |
+	       (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT,
+	       SUNXI_NFC_BASE + NFC_CMD);

drivers/mtd/nand/sunxi_nand_spl.c static int nand_reset_column(void) · line 249

Quoted 3 lines

drivers/mtd/nand/sunxi_nand_spl.c +2 -0

+
+	return nand_wait_int();
 }  

nand_reset_column() and nand_change_column() are almost the same, except one is taking an argument to specify where you want to place the pointer. I'd recommend getting rid of nand_reset_column() entirely and patching all call sites to call nand_change_column(0) instead.

Quoted 108 lines

drivers/mtd/nand/sunxi_nand_spl.c +36 -51

 static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116};

@@ -262,86 +253,80 @@ 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 + NFC_SECTOR_NUM);
-	writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
-	writel(NFC_DATA_TRANS |	NFC_PAGE_CMD | NFC_DATA_SWAP_METHOD,
-	       SUNXI_NFC_BASE + NFC_CMD);
+	/* Retrieve data from SRAM (PIO) */
+	for (i = 0; i < nsectors; i++) {
+		int data_off = i * conf->ecc_size;
+		int oob_off = conf->page_size + (i * oob_chunk_sz);
+		u8 *data = dest + data_off;
+
+		/* Clear ECC status and restart ECC engine */
+		writel(0, SUNXI_NFC_BASE + NFC_ECC_ST);
+		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_EXCEPTION,
+		       SUNXI_NFC_BASE + NFC_ECC_CTL);
+
+		/* Move the data in SRAM */
+		nand_change_column(data_off);
+		nand_wait_cmd_fifo_empty();
+		writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+		writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT);
+		writel(NFC_DATA_TRANS, SUNXI_NFC_BASE + NFC_CMD);
+		nand_wait_int();
 
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_DMA_INT_FLAG,
-			 DEFAULT_TIMEOUT_US)) {
-		printf("Error while initializing dma interrupt\n");
-		return -EIO;
-	}
-	writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+		/*
+		 * Let the ECC engine consume the ECC bytes and possibly correct
+		 * the data.
+		 */
+		nand_change_column(oob_off);
+		nand_wait_cmd_fifo_empty();
+		writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
+		writel(NFC_DATA_TRANS | NFC_ECC_CMD, SUNXI_NFC_BASE + NFC_CMD);
+		nand_wait_int();
 
-	if (!check_value_negated(SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0,
-				 SUNXI_DMA_DDMA_CFG_REG_LOADING,
-				 DEFAULT_TIMEOUT_US)) {
-		printf("Error while waiting for dma transfer to finish\n");
-		return -EIO;
-	}
+		/* Get the ECC status */
+		ecc_st = readl(SUNXI_NFC_BASE + NFC_ECC_ST);
 

drivers/mtd/nand/sunxi_nand_spl.c static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; · line 308

Quoted 5 lines

drivers/mtd/nand/sunxi_nand_spl.c +3 -2

-	invalidate_dcache_range(dst,
-				ALIGN(dst + conf->ecc_size, ARCH_DMA_MINALIGN));
+		/* Retrieve the data from SRAM */
+		memcpy_fromio(data, SUNXI_NFC_BASE + NFC_RAM0_BASE,
+			      conf->ecc_size);  

Not a big deal, but should we really copy the data if the ECC engine reports an error? Maybe you can move this memcpy_fromio() after checking ecc_st.

Quoted 16 lines

drivers/mtd/nand/sunxi_nand_spl.c +6 -6

-	val = readl(SUNXI_NFC_BASE + NFC_ECC_ST);
+		/* Stop ECC engine */
+		writel(readl(SUNXI_NFC_BASE + NFC_ECC_CTL) & ~NFC_ECC_EN,
+		       SUNXI_NFC_BASE + NFC_ECC_CTL);
 
-	/* ECC error detected. */
-	if (val & 0xffff)
-		return -EIO;
+		/* ECC error detected. */
+		if (ecc_st & 0xffff)
+			return -EIO;
 
-	/*
-	 * Return 1 if the page is empty.

drivers/mtd/nand/sunxi_nand_spl.c static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; · line 323

Quoted 10 lines

drivers/mtd/nand/sunxi_nand_spl.c +6 -4

-	 * We consider the page as empty if the first ECC block is marked
-	 * empty.
-	 */
-	return (val & 0x10000) ? 1 : 0;
+		/*
+		 * Return 1 if the page is empty. We consider the page as empty
+		 * if the first ECC block is marked empty.
+		 */
+		if (ecc_st & 0x10000)
+			return 1;  

This is no longer valid, because now you iterate over all ECC chunks manually. The test should be:

		if (!i && (ecc_st & 0x10000))

drivers/mtd/nand/sunxi_nand_spl.c static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; · line 332

Quoted 9 lines

drivers/mtd/nand/sunxi_nand_spl.c +6 -0

+
+		if (data_off + conf->ecc_size >= len)
+			break;
+	}
+
+	return 0;
 }
 
 static int nand_max_ecc_strength(struct nfc_config *conf)  

Add NAND parameters to the Nintendo NES Classic configuration file.

Signed-off-by Miquel Raynal <[email protected]>

---
 configs/Nintendo_NES_Classic_Edition_defconfig | 4 ++++
 1 file changed, 4 insertions(+)

configs/Nintendo_NES_Classic_Edition_defconfig +0 -0

index d05375d0db..467055279c 100644

Add the NAND controller node, as well as the definition of the missing pins and clock.

Signed-off-by Miquel Raynal <[email protected]>

---
 arch/arm/dts/sun8i-a23-a33.dtsi | 31 +++++++++++++++++++++++++++++++
 arch/arm/dts/sun8i-a33.dtsi     |  8 ++++++++
 2 files changed, 39 insertions(+)

arch/arm/dts/sun8i-a23-a33.dtsi +3 -0

index f97c38f097..fe6ea82cb3 100644
@@ -325,6 +325,19 @@
 			#size-cells = <0>;
 		};
 
+		nfc: nand at 01c03000 {
+			compatible = "allwinner,sun4i-a10-nand";
+			reg = <0x01c03000 0x1000>;

Let the Nintendo NES Classic use the Macronix NAND chip on it.

Signed-off-by Miquel Raynal <[email protected]>

---
 arch/arm/dts/sun8i-r16-nintendo-nes-classic-edition.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/arm/dts/sun8i-r16-nintendo-nes-classic-edition.dts +0 -0

index dce688ec8e..b8535d3dac 100644
@@ -61,3 +61,17 @@
 	pinctrl-0 = <&uart0_pins_a>;
 	status = "okay";

arch/arm/dts/sun8i-r16-nintendo-nes-classic-edition.dts +5 -0

+		reg = <0>;
+		allwinner,rb = <0>;
+		nand-ecc-mode = "hw";
+	};
+};