[U-Boot] [PATCH 00/15] sunxi_nand_spl: cleanups, auto config and bad block handling

[U-Boot] [PATCH 00/15] sunxi_nand_spl: cleanups, auto config and bad block handling

[Hans de Goede]

Hi All,

As promised I've been working on cleaning up the sunxi nand spl driver,
as well as adding bad block handling in the same way as the BROM does,
as the SPL + u-boot must be placed in a BROM compatible nand partition.

This series is a bit of a patch bomb, but as you will see they are all
nice and small (which is why there are quite a few).

Regards,

Hans

[U-Boot] [PATCH 01/15] sunxi_nand_spl: Fix CONFIG_SPL_NAND_SUNXI handling

[Hans de Goede]

CONFIG_SPL_NAND_SUPPORT gets used via IS_ENABLED so it must be defined
to 1, rather then just being defined.

While at remove 2 other unused NAND related defines from sunxi-common.h.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/configs/sunxi-common.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 1abf73c..3735afb 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -138,15 +138,10 @@
 #define CONFIG_SERIAL_TAG
 
 #if defined(CONFIG_SPL_NAND_SUNXI)
-#define CONFIG_SPL_NAND_DRIVERS
-#define CONFIG_SPL_NAND_SUPPORT
-
-#define CONFIG_SYS_NAND_SPL_KERNEL_OFFS 0x280000
+#define CONFIG_SPL_NAND_SUPPORT 1
 #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x008000
-
 #endif
 
-
 /* mmc config */
 #if !defined(CONFIG_UART0_PORT_F)
 #define CONFIG_MMC
-- 
2.4.3

[U-Boot] [PATCH 02/15] sunxi_nand_spl: Drop unnecessary temp buf

[Hans de Goede]

nand_spl_load_image() always gets called with either CONFIG_SYS_TEXT_BASE
or spl_image.load_addr as destination, both of which are properly aligened,
and have plenty of space for "overshooting" up to
CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE bytes, as we read in
CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE bytes chunks.

This saves CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE (typically 1k) in
SPL size, which is a lot on the total 24k we have.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 35 ++++++++---------------------------
 1 file changed, 8 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index ac5f56d..46654e4 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -85,6 +85,7 @@
 
 #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)
@@ -94,10 +95,6 @@
 
 /* minimal "boot0" style NAND support for Allwinner A20 */
 
-/* temporary buffer in internal ram */
-unsigned char temp_buf[CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE]
-	__aligned(0x10) __section(".text#");
-
 /* random seed used by linux */
 const uint16_t random_seed[128] = {
 	0x2b75, 0x0bd0, 0x5ca3, 0x62d1, 0x1c93, 0x07e9, 0x2162, 0x3a72,
@@ -167,8 +164,8 @@ void nand_init(void)
 	}
 }
 
-static void nand_read_page(unsigned int real_addr, int syndrome,
-			   uint32_t *ecc_errors)
+static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
+			   int syndrome, uint32_t *ecc_errors)
 {
 	uint32_t val;
 	int ecc_off = 0;
@@ -226,9 +223,6 @@ static void nand_read_page(unsigned int real_addr, int syndrome,
 		return;
 	}
 
-	/* clear temp_buf */
-	memset(temp_buf, 0, CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE);
-
 	/* set CMD  */
 	writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET,
 	       SUNXI_NFC_BASE + NFC_CMD);
@@ -278,8 +272,7 @@ static void nand_read_page(unsigned int real_addr, int syndrome,
 	writel(SUNXI_NFC_BASE + NFC_IO_DATA,
 	       SUNXI_DMA_BASE + SUNXI_DMA_SRC_START_ADDR_REG0);
 	/* read to RAM */
-	writel((uint32_t)temp_buf,
-	       SUNXI_DMA_BASE + SUNXI_DMA_DEST_START_ADDRR_REG0);
+	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);
@@ -287,6 +280,7 @@ static void nand_read_page(unsigned int real_addr, int syndrome,
 	       SUNXI_DMA_BASE + SUNXI_DMA_DDMA_BC_REG0); /* 1kB */
 	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,
@@ -324,27 +318,14 @@ static void nand_read_page(unsigned int real_addr, int syndrome,
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
 {
 	void *current_dest;
-	uint32_t count;
-	uint32_t current_count;
 	uint32_t ecc_errors = 0;
 
-	memset(dest, 0x0, size); /* clean destination memory */
 	for (current_dest = dest;
 			current_dest < (dest + size);
 			current_dest += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE) {
-		nand_read_page(offs, offs
-				< CONFIG_NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END,
-			       &ecc_errors);
-		count = current_dest - dest;
-
-		if (size - count > CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE)
-			current_count = CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE;
-		else
-			current_count = size - count;
-
-		memcpy(current_dest,
-		       temp_buf,
-		       current_count);
+		nand_read_page(offs, (dma_addr_t)current_dest,
+			offs < CONFIG_NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END,
+			&ecc_errors);
 		offs += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE;
 	}
 	return ecc_errors ? -1 : 0;
-- 
2.4.3

[U-Boot] [PATCH 03/15] sunxi_nand_spl: We only need to reset the nand chip once

[Hans de Goede]

There is no need to reset the nand chip for every ecc-block read.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 46654e4..56c0be0 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -162,6 +162,16 @@ void nand_init(void)
 				 NFC_CTL_RESET, MAX_RETRIES)) {
 		printf("Couldn't initialize nand\n");
 	}
+
+	/* reset NAND */
+	writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET,
+	       SUNXI_NFC_BASE + NFC_CMD);
+
+	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_CMD_INT_FLAG,
+			 MAX_RETRIES)) {
+		printf("Error timeout waiting for nand reset\n");
+		return;
+	}
 }
 
 static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
@@ -223,16 +233,6 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 		return;
 	}
 
-	/* set CMD  */
-	writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET,
-	       SUNXI_NFC_BASE + NFC_CMD);
-
-	if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_CMD_INT_FLAG,
-			 MAX_RETRIES)) {
-		printf("Error while initilizing command interrupt\n");
-		return;
-	}
-
 	page = real_addr / CONFIG_NAND_SUNXI_SPL_PAGE_SIZE;
 	column = real_addr % CONFIG_NAND_SUNXI_SPL_PAGE_SIZE;
 
-- 
2.4.3

[U-Boot] [PATCH 04/15] sunxi_nand_spl: Do not bother writing the spare-area reg in syndrome mode

[Hans de Goede]

In syndrome mode we set the NFC_SEQ bit in the command register, so the
spare-area register is not used. Also the value currently being written is
actual wrong, the ecc sits at "column + CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE"
not just CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE.

So the current code only serves to confuse the user -> remove it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 56c0be0..f6f4928 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -256,10 +256,7 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 	val = readl(SUNXI_NFC_BASE + NFC_CTL);
 	writel(val | NFC_CTL_RAM_METHOD, SUNXI_NFC_BASE + NFC_CTL);
 
-	if (syndrome) {
-		writel(CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE,
-		       SUNXI_NFC_BASE + NFC_SPARE_AREA);
-	} else {
+	if (!syndrome) {
 		oob_offset = CONFIG_NAND_SUNXI_SPL_PAGE_SIZE
 			+ (column / CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE)
 			* ecc_off;
-- 
2.4.3

[U-Boot] [PATCH 05/15] sunxi_nand_spl: Use SYS_NAND_SELF_INIT and only do nand init when necessary

[Hans de Goede]

Use SYS_NAND_SELF_INIT and only setup the pinmux and clocks when we are
actually using the nand.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/board.c               | 12 +++++++-----
 drivers/mtd/nand/Kconfig          |  1 +
 drivers/mtd/nand/sunxi_nand_spl.c |  2 ++
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 680523a..b76bb83 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -31,6 +31,7 @@
 #include <asm/arch/usb_phy.h>
 #include <asm/gpio.h>
 #include <asm/io.h>
+#include <nand.h>
 #include <net.h>
 
 #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
@@ -127,6 +128,12 @@ static void nand_clock_setup(void)
 	setbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
 	setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
 }
+
+void board_nand_init(void)
+{
+	nand_pinmux_setup();
+	nand_clock_setup();
+}
 #endif
 
 #ifdef CONFIG_GENERIC_MMC
@@ -453,11 +460,6 @@ void sunxi_board_init(void)
 	power_failed |= axp221_set_eldo(3, CONFIG_AXP221_ELDO3_VOLT);
 #endif
 
-#ifdef CONFIG_SPL_NAND_SUNXI
-	nand_pinmux_setup();
-	nand_clock_setup();
-#endif
-
 	printf("DRAM:");
 	ramsize = sunxi_dram_init();
 	printf(" %lu MiB\n", ramsize >> 20);
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 41ebfea..18039f7 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -88,6 +88,7 @@ config SPL_NAND_DENALI
 config SPL_NAND_SUNXI
 	bool "Support for NAND on Allwinner A20 in SPL"
 	depends on MACH_SUN7I
+	select SYS_NAND_SELF_INIT
 	---help---
 	Enable support for NAND. This option allows SPL to read from
 	sunxi NAND using DMA transfers.
diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index f6f4928..9efe904 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -153,6 +153,8 @@ void nand_init(void)
 {
 	uint32_t val;
 
+	board_nand_init();
+
 	val = readl(SUNXI_NFC_BASE + NFC_CTL);
 	/* enable and reset CTL */
 	writel(val | NFC_CTL_EN | NFC_CTL_RESET,
-- 
2.4.3

[U-Boot] [PATCH 06/15] sunxi_nand_spl: Make sure the DMA controller is enabled

[Hans de Goede]

We use DMA for nand data transfers in the SPL, so make sure the DMA
controller is enabled.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/board.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index b76bb83..1ebd0a4 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -125,7 +125,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));
+#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));
+#endif
 	setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
 }
 
-- 
2.4.3

[U-Boot] [PATCH 07/15] sunxi_nand_spl: Turn off clocks when we're done with the nand

[Hans de Goede]

Turn off the nand and dma clocks when we're done with the nand, this
puts the nand and dma controllers back into a clean state for when the
kernel boots.

Without this the kernel will not boot properly when it is build with
dma-controller support.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 9efe904..147d476 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -5,9 +5,10 @@
  * SPDX-License-Identifier:     GPL-2.0+
  */
 
+#include <asm/arch/clock.h>
+#include <asm/io.h>
 #include <common.h>
 #include <config.h>
-#include <asm/io.h>
 #include <nand.h>
 
 /* registers */
@@ -330,4 +331,16 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
 	return ecc_errors ? -1 : 0;
 }
 
-void nand_deselect(void) {}
+void nand_deselect(void)
+{
+	struct sunxi_ccm_reg *const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+	clrbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
+#ifdef CONFIG_MACH_SUN9I
+	clrbits_le32(&ccm->ahb_gate1, (1 << AHB_GATE_OFFSET_DMA));
+#else
+	clrbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_DMA));
+#endif
+	clrbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
+}
-- 
2.4.3

[U-Boot] [PATCH 08/15] sunxi_nand_spl: Add proper cache flusing

[Hans de Goede]

We are using dma, so we should flush the cache before starting the dma,
and invalidate it once the dma is done.

Things are working without this by mostly luck, but lets not rely on that.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 147d476..663c03e 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -266,6 +266,10 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 		writel(oob_offset, SUNXI_NFC_BASE + NFC_SPARE_AREA);
 	}
 
+	flush_dcache_range(dst,
+			   ALIGN(dst + CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE,
+				 ARCH_DMA_MINALIGN));
+
 	/* SUNXI_DMA */
 	writel(0x0, SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0); /* clr dma cmd */
 	/* read from REG_IO_DATA */
@@ -311,6 +315,10 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 		return;
 	}
 
+	invalidate_dcache_range(dst,
+			ALIGN(dst + CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE,
+			      ARCH_DMA_MINALIGN));
+
 	if (readl(SUNXI_NFC_BASE + NFC_ECC_ST))
 		(*ecc_errors)++;
 }
-- 
2.4.3

[U-Boot] [PATCH 09/15] sunxi_nand_spl: Use kernel driver algorithm for determining ecc_mode / _off

[Hans de Goede]

Sync the code for figuring out the ecc_mode and ecc_offset with the kernel
code. Keeping this in sync seems like a good idea in general, and it
fixes / adds support for ecc strengths of 56, 60 and 64 bits.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 58 ++++++++-------------------------------
 1 file changed, 12 insertions(+), 46 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 663c03e..61eb393 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -181,60 +181,26 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 			   int syndrome, uint32_t *ecc_errors)
 {
 	uint32_t val;
-	int ecc_off = 0;
+	int i, ecc_off = 0;
 	uint16_t ecc_mode = 0;
 	uint16_t rand_seed;
 	uint32_t page;
 	uint16_t column;
 	uint32_t oob_offset;
+	static const u8 strengths[] = { 16, 24, 28, 32, 40, 48, 56, 60, 64 };
 
-	switch (CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH) {
-	case 16:
-		ecc_mode = 0;
-		ecc_off = 0x20;
-		break;
-	case 24:
-		ecc_mode = 1;
-		ecc_off = 0x2e;
-		break;
-	case 28:
-		ecc_mode = 2;
-		ecc_off = 0x32;
-		break;
-	case 32:
-		ecc_mode = 3;
-		ecc_off = 0x3c;
-		break;
-	case 40:
-		ecc_mode = 4;
-		ecc_off = 0x4a;
-		break;
-	case 48:
-		ecc_mode = 4;
-		ecc_off = 0x52;
-		break;
-	case 56:
-		ecc_mode = 4;
-		ecc_off = 0x60;
-		break;
-	case 60:
-		ecc_mode = 4;
-		ecc_off = 0x0;
-		break;
-	case 64:
-		ecc_mode = 4;
-		ecc_off = 0x0;
-		break;
-	default:
-		ecc_mode = 0;
-		ecc_off = 0;
+	for (i = 0; i < ARRAY_SIZE(strengths); i++) {
+		if (CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH == strengths[i]) {
+			ecc_mode = i;
+			break;
+		}
 	}
 
-	if (ecc_off == 0) {
-		printf("Unsupported ECC strength (%d)!\n",
-		       CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH);
-		return;
-	}
+	/* HW ECC always request ECC bytes for 1024 bytes blocks */
+	ecc_off = DIV_ROUND_UP(CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH * fls(8 * 1024), 8);
+	/* HW ECC always work with even numbers of ECC bytes */
+	ecc_off += (ecc_off & 1);
+	ecc_off += 4; /* prepad */
 
 	page = real_addr / CONFIG_NAND_SUNXI_SPL_PAGE_SIZE;
 	column = real_addr % CONFIG_NAND_SUNXI_SPL_PAGE_SIZE;
-- 
2.4.3

[U-Boot] [PATCH 10/15] sunxi_nand_spl: Remove NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END

[Hans de Goede]

We currently only use the spl nand code to get u-boot itself, which
_always_ is located on a syndrome partition. Once we figure out how
we are going to store the u-boot env on nand, we may need non syndrome
support, but even then there will likely be better ways to determine
whether to use syndrome or not (e.g. look at the loadaddr passed in).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/Kconfig          | 8 --------
 drivers/mtd/nand/sunxi_nand_spl.c | 3 +--
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 18039f7..8a3fae5 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -95,14 +95,6 @@ config SPL_NAND_SUNXI
 	Depending on the NAND chip, values like ECC strength and page sizes
 	have to be configured.
 
-config NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END
-	hex "Size of syndrome partitions in sunxi NAND"
-	default 0x400000
-	depends on SPL_NAND_SUNXI
-	---help---
-	End address for boot partitions on NAND. Those partitions have a
-	different random seed that has to match the sunxi BROM setting.
-
 config NAND_SUNXI_SPL_ECC_STRENGTH
 	int "ECC Strength for sunxi NAND"
 	default 40
diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 61eb393..9c304f4 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -297,8 +297,7 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
 	for (current_dest = dest;
 			current_dest < (dest + size);
 			current_dest += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE) {
-		nand_read_page(offs, (dma_addr_t)current_dest,
-			offs < CONFIG_NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END,
+		nand_read_page(offs, (dma_addr_t)current_dest, 1,
 			&ecc_errors);
 		offs += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE;
 	}
-- 
2.4.3

[U-Boot] [PATCH 11/15] sunxi_nand_spl: Add support for sun4i and sun5i SoCs

[Hans de Goede]

Other then having a few less chip-select lines the nand controller
on sun4i, sun5i and sun7i is identical.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/board.c      | 12 +++++++++---
 drivers/mtd/nand/Kconfig |  4 ++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 1ebd0a4..d411e96 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -112,13 +112,19 @@ int dram_init(void)
 static void nand_pinmux_setup(void)
 {
 	unsigned int pin;
-	for (pin = SUNXI_GPC(0); pin <= SUNXI_GPC(6); pin++)
-		sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_NAND);
 
-	for (pin = SUNXI_GPC(8); pin <= SUNXI_GPC(22); pin++)
+	for (pin = SUNXI_GPC(0); pin <= SUNXI_GPC(19); pin++)
 		sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_NAND);
 
+#if defined CONFIG_MACH_SUN4I || defined CONFIG_MACH_SUN7I
+	for (pin = SUNXI_GPC(20); pin <= SUNXI_GPC(22); pin++)
+		sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_NAND);
+#endif
+	/* sun4i / sun7i do have a PC23, but it is not used for nand,
+	 * only sun7i has a PC24 */
+#ifdef CONFIG_MACH_SUN7I
 	sunxi_gpio_set_cfgpin(SUNXI_GPC(24), SUNXI_GPC_NAND);
+#endif
 }
 
 static void nand_clock_setup(void)
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 8a3fae5..e3cbc31 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -86,8 +86,8 @@ config SPL_NAND_DENALI
 	  for use on SPL.
 
 config SPL_NAND_SUNXI
-	bool "Support for NAND on Allwinner A20 in SPL"
-	depends on MACH_SUN7I
+	bool "Support for NAND on Allwinner SoCs in SPL"
+	depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
 	select SYS_NAND_SELF_INIT
 	---help---
 	Enable support for NAND. This option allows SPL to read from
-- 
2.4.3

[U-Boot] [PATCH 12/15] sunxi_nand_spl: Properly config page-size in the nand ctl register

[Hans de Goede]

Properly config page-size in the nand ctl register, it seems that things
work fine without doing this, but still lets play it safe and properly
set the page-size.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 9c304f4..34bb1a3 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -42,6 +42,8 @@
 #define NFC_CTL_EN                 (1 << 0)
 #define NFC_CTL_RESET              (1 << 1)
 #define NFC_CTL_RAM_METHOD         (1 << 14)
+#define NFC_CTL_PAGE_SIZE_MASK     (0xf << 8)
+#define NFC_CTL_PAGE_SIZE(a)       ((fls(a) - 11) << 8)
 
 
 #define NFC_ECC_EN                 (1 << 0)
@@ -294,6 +296,9 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
 	void *current_dest;
 	uint32_t ecc_errors = 0;
 
+	clrsetbits_le32(SUNXI_NFC_BASE + NFC_CTL, NFC_CTL_PAGE_SIZE_MASK,
+			NFC_CTL_PAGE_SIZE(CONFIG_NAND_SUNXI_SPL_PAGE_SIZE));
+
 	for (current_dest = dest;
 			current_dest < (dest + size);
 			current_dest += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE) {
-- 
2.4.3

[U-Boot] [PATCH 13/15] sunxi_nand_spl: Parametrize lowlevel read functions

[Hans de Goede]

Parametrize the lowlevel nand_read_page function, instead of directly
using the CONFIG_foo settings for page-size, etc. there and add a few
wrappers / helper functions for calling it.

This is a preparation patch for adding auto-detecting of the nand
parameters like the BROM does.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 82 +++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 34bb1a3..4cfcdb3 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -179,8 +179,8 @@ void nand_init(void)
 	}
 }
 
-static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
-			   int syndrome, uint32_t *ecc_errors)
+static int nand_read_page(int page_size, int ecc_strength, int ecc_page_size,
+	int addr_cycles, uint32_t real_addr, dma_addr_t dst, int syndrome)
 {
 	uint32_t val;
 	int i, ecc_off = 0;
@@ -188,28 +188,26 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 	uint16_t rand_seed;
 	uint32_t page;
 	uint16_t column;
-	uint32_t oob_offset;
 	static const u8 strengths[] = { 16, 24, 28, 32, 40, 48, 56, 60, 64 };
 
 	for (i = 0; i < ARRAY_SIZE(strengths); i++) {
-		if (CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH == strengths[i]) {
+		if (ecc_strength == strengths[i]) {
 			ecc_mode = i;
 			break;
 		}
 	}
 
 	/* HW ECC always request ECC bytes for 1024 bytes blocks */
-	ecc_off = DIV_ROUND_UP(CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH * fls(8 * 1024), 8);
+	ecc_off = DIV_ROUND_UP(ecc_strength * fls(8 * 1024), 8);
 	/* HW ECC always work with even numbers of ECC bytes */
 	ecc_off += (ecc_off & 1);
 	ecc_off += 4; /* prepad */
 
-	page = real_addr / CONFIG_NAND_SUNXI_SPL_PAGE_SIZE;
-	column = real_addr % CONFIG_NAND_SUNXI_SPL_PAGE_SIZE;
+	page = real_addr / page_size;
+	column = real_addr % page_size;
 
 	if (syndrome)
-		column += (column / CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE)
-			* ecc_off;
+		column += (column / ecc_page_size) * ecc_off;
 
 	/* clear ecc status */
 	writel(0, SUNXI_NFC_BASE + NFC_ECC_ST);
@@ -227,16 +225,11 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 	val = readl(SUNXI_NFC_BASE + NFC_CTL);
 	writel(val | NFC_CTL_RAM_METHOD, SUNXI_NFC_BASE + NFC_CTL);
 
-	if (!syndrome) {
-		oob_offset = CONFIG_NAND_SUNXI_SPL_PAGE_SIZE
-			+ (column / CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE)
-			* ecc_off;
-		writel(oob_offset, SUNXI_NFC_BASE + NFC_SPARE_AREA);
-	}
+	if (!syndrome)
+		writel(page_size + (column / ecc_page_size) * ecc_off,
+		       SUNXI_NFC_BASE + NFC_SPARE_AREA);
 
-	flush_dcache_range(dst,
-			   ALIGN(dst + CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE,
-				 ARCH_DMA_MINALIGN));
+	flush_dcache_range(dst, ALIGN(dst + ecc_page_size, ARCH_DMA_MINALIGN));
 
 	/* SUNXI_DMA */
 	writel(0x0, SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0); /* clr dma cmd */
@@ -248,7 +241,7 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 	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(CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE,
+	writel(ecc_page_size,
 	       SUNXI_DMA_BASE + SUNXI_DMA_DDMA_BC_REG0); /* 1kB */
 	writel(SUNXI_DMA_DDMA_CFG_REG_LOADING
 		| SUNXI_DMA_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32
@@ -267,46 +260,61 @@ static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
 	       SUNXI_NFC_BASE + NFC_ADDR_LOW);
 	writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_ADDR_HIGH);
 	writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_DATA_TRANS |
-		NFC_PAGE_CMD | NFC_WAIT_FLAG | (4 << NFC_ADDR_NUM_OFFSET) |
+		NFC_PAGE_CMD | NFC_WAIT_FLAG |
+		((addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) |
 		NFC_SEND_ADR | NFC_DATA_SWAP_METHOD | (syndrome ? NFC_SEQ : 0),
 		SUNXI_NFC_BASE + NFC_CMD);
 
 	if (!check_value(SUNXI_NFC_BASE + NFC_ST, (1 << 2),
 			 MAX_RETRIES)) {
 		printf("Error while initializing dma interrupt\n");
-		return;
+		return -1;
 	}
 
 	if (!check_value_negated(SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0,
 				 SUNXI_DMA_DDMA_CFG_REG_LOADING, MAX_RETRIES)) {
 		printf("Error while waiting for dma transfer to finish\n");
-		return;
+		return -1;
 	}
 
 	invalidate_dcache_range(dst,
-			ALIGN(dst + CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE,
-			      ARCH_DMA_MINALIGN));
+			        ALIGN(dst + ecc_page_size, ARCH_DMA_MINALIGN));
 
 	if (readl(SUNXI_NFC_BASE + NFC_ECC_ST))
-		(*ecc_errors)++;
+		return -1;
+
+	return 0;
 }
 
-int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
+static int nand_read_ecc(int page_size, int ecc_strength, int ecc_page_size,
+	int addr_cycles, uint32_t offs, uint32_t size, void *dest, int syndrome)
 {
-	void *current_dest;
-	uint32_t ecc_errors = 0;
+	void *end = dest + size;
 
 	clrsetbits_le32(SUNXI_NFC_BASE + NFC_CTL, NFC_CTL_PAGE_SIZE_MASK,
-			NFC_CTL_PAGE_SIZE(CONFIG_NAND_SUNXI_SPL_PAGE_SIZE));
-
-	for (current_dest = dest;
-			current_dest < (dest + size);
-			current_dest += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE) {
-		nand_read_page(offs, (dma_addr_t)current_dest, 1,
-			&ecc_errors);
-		offs += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE;
+			NFC_CTL_PAGE_SIZE(page_size));
+
+	for ( ;dest < end; dest += ecc_page_size, offs += ecc_page_size) {
+		if (nand_read_page(page_size, ecc_strength, ecc_page_size,
+				   addr_cycles, offs, (dma_addr_t)dest, 1))
+			return -1;
 	}
-	return ecc_errors ? -1 : 0;
+
+	return 0;
+}
+
+static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest,
+			    int syndrome)
+{
+	return nand_read_ecc(CONFIG_NAND_SUNXI_SPL_PAGE_SIZE,
+			     CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH,
+			     CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE,
+			     5, offs, size, dest, syndrome);
+}
+
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
+{
+	return nand_read_buffer(offs, size, dest, 1);
 }
 
 void nand_deselect(void)
-- 
2.4.3

[U-Boot] [PATCH 14/15] sunxi_nand_spl: Auto detect nand configuration parameters

[Hans de Goede]

Auto detect the nand configuration parameters, like the BROM does.

This allows us to get rid of various Kconfig settings, and is
necessary to support generic boards like the mk802 which have seen
many production runs with different nands.

The full blown u-boot/kernel nand driver uses the nand id to determine
this info, for the SPL we do as the BROM does and simply try a few
standard configs.

Note the table only contains configs which are known to actually be used,
rather then all the configs the BROM tries. This means that it may need
to be updated in the future as we add support for nand on more boards.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/Kconfig          | 23 ---------------------
 drivers/mtd/nand/sunxi_nand_spl.c | 42 +++++++++++++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index e3cbc31..a07cbeb 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -95,29 +95,6 @@ config SPL_NAND_SUNXI
 	Depending on the NAND chip, values like ECC strength and page sizes
 	have to be configured.
 
-config NAND_SUNXI_SPL_ECC_STRENGTH
-	int "ECC Strength for sunxi NAND"
-	default 40
-	depends on SPL_NAND_SUNXI
-	---help---
-	ECC strength used by the sunxi NAND SPL driver. This is specific to the
-	chosen NAND chip and has to match the value used by the sunxi BROM.
-
-config NAND_SUNXI_SPL_ECC_PAGE_SIZE
-	hex "ECC page size for sunxi NAND"
-	default 0x400
-	depends on SPL_NAND_SUNXI
-	---help---
-	ECC page size used by the sunxi NAND SPL driver for syndrome partitions.
-	This setting has to match the value used by the sunxi BROM.
-
-config NAND_SUNXI_SPL_PAGE_SIZE
-	hex "Page size for sunxi NAND"
-	default 0x2000
-	depends on SPL_NAND_SUNXI
-	---help---
-	Page size of the NAND flash used by the sunxi NAND SPL driver. This is
-	specific to the chosen NAND chip.
 endif
 
 endmenu
diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 4cfcdb3..5687cd8 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -306,10 +306,44 @@ static int nand_read_ecc(int page_size, int ecc_strength, int ecc_page_size,
 static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest,
 			    int syndrome)
 {
-	return nand_read_ecc(CONFIG_NAND_SUNXI_SPL_PAGE_SIZE,
-			     CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH,
-			     CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE,
-			     5, offs, size, dest, syndrome);
+	const struct {
+		int page_size;
+		int ecc_strength;
+		int ecc_page_size;
+		int addr_cycles;
+	} nand_configs[] = {
+		{ 8192, 40, 1024, 5 },
+		{ 8192, 24, 1024, 5 },
+	};
+	static int nand_config = -1;
+	int i;
+
+	if (nand_config == -1) {
+		for (i = 0; i < ARRAY_SIZE(nand_configs); i++) {
+			debug("nand: trying page %d ecc %d / %d addr %d: ",
+			      nand_configs[i].page_size,
+			      nand_configs[i].ecc_strength,
+			      nand_configs[i].ecc_page_size,
+			      nand_configs[i].addr_cycles);
+			if (nand_read_ecc(nand_configs[i].page_size,
+					  nand_configs[i].ecc_strength,
+					  nand_configs[i].ecc_page_size,
+					  nand_configs[i].addr_cycles,
+					  offs, size, dest, syndrome) == 0) {
+				debug("success\n");
+				nand_config = i;
+				return 0;
+			}
+			debug("failed\n");
+		}
+		return -1;
+	}
+
+	return nand_read_ecc(nand_configs[nand_config].page_size,
+			     nand_configs[nand_config].ecc_strength,
+			     nand_configs[nand_config].ecc_page_size,
+			     nand_configs[nand_config].addr_cycles,
+			     offs, size, dest, syndrome);
 }
 
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
-- 
2.4.3

[U-Boot] [PATCH 15/15] sunxi_nand_spl: Add support for backup boot partitions

[Hans de Goede]

The BROM does not care / use bad page markings, instead it deals with
any bad pages in the first erase-block by simply trying to load "boot0"
from the first next erase-block.

This commit implements the same strategy for the sunxi spl nand code,
allowing it to boot from the backup boot partition when the main boot
partition is bad (tested by erasing the main boot partition).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 5687cd8..a75e674 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -348,6 +348,23 @@ static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest,
 
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
 {
+	const uint32_t boot_offsets[] = {
+		0 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
+		1 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
+		2 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
+		4 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
+	};
+	int i;
+
+	if (offs == CONFIG_SYS_NAND_U_BOOT_OFFS) {
+		for (i = 0; i < ARRAY_SIZE(boot_offsets); i++) {
+			if (nand_read_buffer(boot_offsets[i], size,
+					     dest, 1) == 0)
+				return 0;
+		}
+		return -1;
+	}
+
 	return nand_read_buffer(offs, size, dest, 1);
 }
 
-- 
2.4.3

[U-Boot] [PATCH 02/15] sunxi_nand_spl: Drop unnecessary temp buf

[Chen-Yu Tsai]

On Sun, Aug 16, 2015 at 4:02 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> nand_spl_load_image() always gets called with either CONFIG_SYS_TEXT_BASE
> or spl_image.load_addr as destination, both of which are properly aligened,
> and have plenty of space for "overshooting" up to
> CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE bytes, as we read in
> CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE bytes chunks.
>
> This saves CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE (typically 1k) in
> SPL size, which is a lot on the total 24k we have.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mtd/nand/sunxi_nand_spl.c | 35 ++++++++---------------------------
>  1 file changed, 8 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
> index ac5f56d..46654e4 100644
> --- a/drivers/mtd/nand/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/sunxi_nand_spl.c
> @@ -85,6 +85,7 @@
>
>  #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)

Might want to mention this change (correct DMA type?) in the commit log.

>  #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)
> @@ -94,10 +95,6 @@
>
>  /* minimal "boot0" style NAND support for Allwinner A20 */
>
> -/* temporary buffer in internal ram */
> -unsigned char temp_buf[CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE]
> -       __aligned(0x10) __section(".text#");
> -
>  /* random seed used by linux */
>  const uint16_t random_seed[128] = {
>         0x2b75, 0x0bd0, 0x5ca3, 0x62d1, 0x1c93, 0x07e9, 0x2162, 0x3a72,
> @@ -167,8 +164,8 @@ void nand_init(void)
>         }
>  }
>
> -static void nand_read_page(unsigned int real_addr, int syndrome,
> -                          uint32_t *ecc_errors)
> +static void nand_read_page(unsigned int real_addr, dma_addr_t dst,
> +                          int syndrome, uint32_t *ecc_errors)
>  {
>         uint32_t val;
>         int ecc_off = 0;
> @@ -226,9 +223,6 @@ static void nand_read_page(unsigned int real_addr, int syndrome,
>                 return;
>         }
>
> -       /* clear temp_buf */
> -       memset(temp_buf, 0, CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE);
> -
>         /* set CMD  */
>         writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET,
>                SUNXI_NFC_BASE + NFC_CMD);
> @@ -278,8 +272,7 @@ static void nand_read_page(unsigned int real_addr, int syndrome,
>         writel(SUNXI_NFC_BASE + NFC_IO_DATA,
>                SUNXI_DMA_BASE + SUNXI_DMA_SRC_START_ADDR_REG0);
>         /* read to RAM */
> -       writel((uint32_t)temp_buf,
> -              SUNXI_DMA_BASE + SUNXI_DMA_DEST_START_ADDRR_REG0);
> +       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);
> @@ -287,6 +280,7 @@ static void nand_read_page(unsigned int real_addr, int syndrome,
>                SUNXI_DMA_BASE + SUNXI_DMA_DDMA_BC_REG0); /* 1kB */
>         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,
> @@ -324,27 +318,14 @@ static void nand_read_page(unsigned int real_addr, int syndrome,
>  int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
>  {
>         void *current_dest;
> -       uint32_t count;
> -       uint32_t current_count;
>         uint32_t ecc_errors = 0;
>
> -       memset(dest, 0x0, size); /* clean destination memory */
>         for (current_dest = dest;
>                         current_dest < (dest + size);
>                         current_dest += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE) {
> -               nand_read_page(offs, offs
> -                               < CONFIG_NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END,
> -                              &ecc_errors);
> -               count = current_dest - dest;
> -
> -               if (size - count > CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE)
> -                       current_count = CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE;
> -               else
> -                       current_count = size - count;
> -
> -               memcpy(current_dest,
> -                      temp_buf,
> -                      current_count);
> +               nand_read_page(offs, (dma_addr_t)current_dest,
> +                       offs < CONFIG_NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END,
> +                       &ecc_errors);
>                 offs += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE;
>         }
>         return ecc_errors ? -1 : 0;
> --
> 2.4.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 01/15] sunxi_nand_spl: Fix CONFIG_SPL_NAND_SUNXI handling

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> CONFIG_SPL_NAND_SUPPORT gets used via IS_ENABLED so it must be 
> defined
> to 1, rather then just being defined.
> 
> While at remove 2 other unused NAND related defines from sunxi
> -common.h.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 02/15] sunxi_nand_spl: Drop unnecessary temp buf

[Ian Campbell]

On Mon, 2015-08-17 at 15:22 +0800, Chen-Yu Tsai wrote:
> 
> > +#define SUNXI_DMA_DDMA_CFG_REG_DDMA_DST_DRQ_TYPE_DRAM (1 << 16)
> 
> Might want to mention this change (correct DMA type?) in the commit 
> log.

Yes, would be good.

With that: Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 03/15] sunxi_nand_spl: We only need to reset the nand chip once

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> There is no need to reset the nand chip for every ecc-block read.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 04/15] sunxi_nand_spl: Do not bother writing the spare-area reg in syndrome mode

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> In syndrome mode we set the NFC_SEQ bit in the command register, so the
> spare-area register is not used. Also the value currently being written is
> actual wrong, the ecc sits at "column + CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE"
> not just CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE.
> 
> So the current code only serves to confuse the user -> remove it.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

There's a bunch of other uses of the syndrome parameter in this
function. Does syndrome=true work even without this particular bit of
code?

I suppose I'm asking, should the paramter and the other uses be
removed? Or should an ASSERT(!syndrome) be added, or am I worrying
about nothing and everything is just fine as it is after this patch?

I suspect the latter, so if that is indeed the case:
    Acked-by: Ian Campbell <    ijc@hellion.org.uk    >

[U-Boot] [PATCH 05/15] sunxi_nand_spl: Use SYS_NAND_SELF_INIT and only do nand init when necessary

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> Use SYS_NAND_SELF_INIT and only setup the pinmux and clocks when we are
> actually using the nand.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 06/15] sunxi_nand_spl: Make sure the DMA controller is enabled

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> We use DMA for nand data transfers in the SPL, so make sure the DMA
> controller is enabled.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 07/15] sunxi_nand_spl: Turn off clocks when we're done with the nand

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> Turn off the nand and dma clocks when we're done with the nand, this
> puts the nand and dma controllers back into a clean state for when the
> kernel boots.
> 
> Without this the kernel will not boot properly when it is build with

                                                            built

> dma-controller support.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 08/15] sunxi_nand_spl: Add proper cache flusing

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> We are using dma, so we should flush the cache before starting the dma,
> and invalidate it once the dma is done.
> 
> Things are working without this by mostly luck, but lets not rely on that.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 09/15] sunxi_nand_spl: Use kernel driver algorithm for determining ecc_mode / _off

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> Sync the code for figuring out the ecc_mode and ecc_offset with the kernel
> code. Keeping this in sync seems like a good idea in general,

I agree. When doing this IME it is useful to include the last version
sync'd in, either as a code comment or in the commit message, so next
time the person doing the sync has a clue where in the upstream history
to start looking.

>  and it
> fixes / adds support for ecc strengths of 56, 60 and 64 bits.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

With a reference to the specific Linux version:
    Acked-by: Ian Campbell <    ijc@hellion.org.uk    >

[U-Boot] [PATCH 10/15] sunxi_nand_spl: Remove NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> We currently only use the spl nand code to get u-boot itself, which
> _always_ is located on a syndrome partition. Once we figure out how
> we are going to store the u-boot env on nand, we may need non syndrome
> support, but even then there will likely be better ways to determine
> whether to use syndrome or not (e.g. look at the loadaddr passed in).
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I suppose I should back reference my question as to whether we should
remove this option (even though it seems to have ended up the other way
round to what I was anticipating).

Anyway, I'm fine with this anyway:
    Acked-by: Ian Campbell <    ijc@hellion.org.uk    >

[U-Boot] [PATCH 11/15] sunxi_nand_spl: Add support for sun4i and sun5i SoCs

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> Other then having a few less chip-select lines the nand controller
> on sun4i, sun5i and sun7i is identical.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  board/sunxi/board.c      | 12 +++++++++---
>  drivers/mtd/nand/Kconfig |  4 ++--
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 1ebd0a4..d411e96 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -112,13 +112,19 @@ int dram_init(void)
>  static void nand_pinmux_setup(void)
>  {
>  	unsigned int pin;
> -	for (pin = SUNXI_GPC(0); pin <= SUNXI_GPC(6); pin++)
> -		sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_NAND);
>  
> -	for (pin = SUNXI_GPC(8); pin <= SUNXI_GPC(22); pin++)
> +	for (pin = SUNXI_GPC(0); pin <= SUNXI_GPC(19); pin++)

This looks to have added GPC(7) to all platforms, was that intentional?

Ian.

[U-Boot] [PATCH 12/15] sunxi_nand_spl: Properly config page-size in the nand ctl register

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> Properly config page-size in the nand ctl register, it seems that things
> work fine without doing this, but still lets play it safe and properly
> set the page-size.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 13/15] sunxi_nand_spl: Parametrize lowlevel read functions

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> Parametrize the lowlevel nand_read_page function, instead of directly
> using the CONFIG_foo settings for page-size, etc. there and add a few
> wrappers / helper functions for calling it.
> 
> This is a preparation patch for adding auto-detecting of the nand
> parameters like the BROM does.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 11/15] sunxi_nand_spl: Add support for sun4i and sun5i SoCs

[Hans de Goede]

Hi,

On 17-08-15 10:29, Ian Campbell wrote:
> On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
>> Other then having a few less chip-select lines the nand controller
>> on sun4i, sun5i and sun7i is identical.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   board/sunxi/board.c      | 12 +++++++++---
>>   drivers/mtd/nand/Kconfig |  4 ++--
>>   2 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> index 1ebd0a4..d411e96 100644
>> --- a/board/sunxi/board.c
>> +++ b/board/sunxi/board.c
>> @@ -112,13 +112,19 @@ int dram_init(void)
>>   static void nand_pinmux_setup(void)
>>   {
>>   	unsigned int pin;
>> -	for (pin = SUNXI_GPC(0); pin <= SUNXI_GPC(6); pin++)
>> -		sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_NAND);
>>
>> -	for (pin = SUNXI_GPC(8); pin <= SUNXI_GPC(22); pin++)
>> +	for (pin = SUNXI_GPC(0); pin <= SUNXI_GPC(19); pin++)
>
> This looks to have added GPC(7) to all platforms, was that intentional?

Yes, as GPC7 is a nand pin on all platforms, no idea why the original
code was skipping it. I should probably mention this in the commit msg though.

Regards,

Hans

[U-Boot] [PATCH 14/15] sunxi_nand_spl: Auto detect nand configuration parameters

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> Auto detect the nand configuration parameters, like the BROM does.
> 
> This allows us to get rid of various Kconfig settings, and is
> necessary to support generic boards like the mk802 which have seen
> many production runs with different nands.
> 
> The full blown u-boot/kernel nand driver uses the nand id to determine
> this info, for the SPL we do as the BROM does and simply try a few
> standard configs.
> 
> Note the table only contains configs which are known to actually be used,
> rather then all the configs the BROM tries. This means that it may need
> to be updated in the future as we add support for nand on more boards.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

(assuming table vs. probing by nand id is OK by Scott)

[U-Boot] [PATCH 15/15] sunxi_nand_spl: Add support for backup boot partitions

[Ian Campbell]

On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> The BROM does not care / use bad page markings, instead it deals with
> any bad pages in the first erase-block by simply trying to load "boot0"
> from the first next erase-block.


           "first/next"? Maybe, or maybe s/first //?

> This commit implements the same strategy for the sunxi spl nand code,
> allowing it to boot from the backup boot partition when the main boot
> partition is bad (tested by erasing the main boot partition).
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

> ---
>  drivers/mtd/nand/sunxi_nand_spl.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/mtd/nand/sunxi_nand_spl.c 
> b/drivers/mtd/nand/sunxi_nand_spl.c
> index 5687cd8..a75e674 100644
> --- a/drivers/mtd/nand/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/sunxi_nand_spl.c
> @@ -348,6 +348,23 @@ static int nand_read_buffer(uint32_t offs, 
> unsigned int size, void *dest,
>  
>  int nand_spl_load_image(uint32_t offs, unsigned int size, void 
> *dest)
>  {
> +	const uint32_t boot_offsets[] = {
> +		0 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
> +		1 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
> +		2 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
> +		4 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
> +	};
> +	int i;
> +
> +	if (offs == CONFIG_SYS_NAND_U_BOOT_OFFS) {
> +		for (i = 0; i < ARRAY_SIZE(boot_offsets); i++) {
> +			if (nand_read_buffer(boot_offsets[i], size,
> +					     dest, 1) == 0)
> +				return 0;
> +		}
> +		return -1;
> +	}
> +
>  	return nand_read_buffer(offs, size, dest, 1);
>  }
>  

[U-Boot] [PATCH 11/15] sunxi_nand_spl: Add support for sun4i and sun5i SoCs

[Ian Campbell]

On Mon, 2015-08-17 at 10:31 +0200, Hans de Goede wrote:
> Hi,
> 
> On 17-08-15 10:29, Ian Campbell wrote:
> > On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> > > Other then having a few less chip-select lines the nand 
> > > controller
> > > on sun4i, sun5i and sun7i is identical.
> > > 
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > >   board/sunxi/board.c      | 12 +++++++++---
> > >   drivers/mtd/nand/Kconfig |  4 ++--
> > >   2 files changed, 11 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index 1ebd0a4..d411e96 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -112,13 +112,19 @@ int dram_init(void)
> > >   static void nand_pinmux_setup(void)
> > >   {
> > >   	unsigned int pin;
> > > -	for (pin = SUNXI_GPC(0); pin <= SUNXI_GPC(6); pin++)
> > > -		sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_NAND);
> > > 
> > > -	for (pin = SUNXI_GPC(8); pin <= SUNXI_GPC(22); pin++)
> > > +	for (pin = SUNXI_GPC(0); pin <= SUNXI_GPC(19); pin++)
> > 
> > This looks to have added GPC(7) to all platforms, was that 
> > intentional?
> 
> Yes, as GPC7 is a nand pin on all platforms, no idea why the original
> code was skipping it. I should probably mention this in the commit 
> msg though.

Please, then: Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 04/15] sunxi_nand_spl: Do not bother writing the spare-area reg in syndrome mode

[Hans de Goede]

Hi,

On 17-08-15 10:19, Ian Campbell wrote:
> On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
>> In syndrome mode we set the NFC_SEQ bit in the command register, so the
>> spare-area register is not used. Also the value currently being written is
>> actual wrong, the ecc sits at "column + CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE"
>> not just CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE.
>>
>> So the current code only serves to confuse the user -> remove it.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> There's a bunch of other uses of the syndrome parameter in this
> function. Does syndrome=true work even without this particular bit of
> code?

Yes, since in syndrome we are passing in the NFC_SEQ bit in the command
register, which tells the controller that the ecc data is directly behind
the actual data, the spare-area address register is not used.

I stumbled over this because the code we had was writing the wrong address
for the ecc data to the spare-area address register, leading me to wonder
why syndrome mode was working at all.

> I suppose I'm asking, should the paramter and the other uses be
> removed? Or should an ASSERT(!syndrome) be added, or am I worrying
> about nothing and everything is just fine as it is after this patch?

Everything is just fine after this patch, actually it was fine before
this patch except that it did an unnecessary write, with the wrong
ecc data address which I found confusing, hence this patch to remove
that write.

>
> I suspect the latter, so if that is indeed the case:
>      Acked-by: Ian Campbell <    ijc@hellion.org.uk    >
>

Thanks for all the reviews.

Regards,

Hans

[U-Boot] [PATCH 10/15] sunxi_nand_spl: Remove NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END

[Hans de Goede]

Hi,

On 17-08-15 10:27, Ian Campbell wrote:
> On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
>> We currently only use the spl nand code to get u-boot itself, which
>> _always_ is located on a syndrome partition. Once we figure out how
>> we are going to store the u-boot env on nand, we may need non syndrome
>> support, but even then there will likely be better ways to determine
>> whether to use syndrome or not (e.g. look at the loadaddr passed in).
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> I suppose I should back reference my question as to whether we should
> remove this option (even though it seems to have ended up the other way
> round to what I was anticipating).

Actually I've been discussing nand partitioning with Boris Brezillon and
it looks like that we need may need to use a non syndrome partition for
u-boot on some nands, because the ecc options supported by the BROM are
too weak for this nand, so we will only use a BROM compatible ecc setting
for the spl and use a separate nand partition for u-boot. We're still
figuring this out, but I think I'll drop this patch for now, as we may
need this after all.

Regards,

Hans

[U-Boot] [PATCH 04/15] sunxi_nand_spl: Do not bother writing the spare-area reg in syndrome mode

[Ian Campbell]

On Mon, 2015-08-17 at 10:59 +0200, Hans de Goede wrote:
> Hi,
> 
> On 17-08-15 10:19, Ian Campbell wrote:
> > On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
> > > In syndrome mode we set the NFC_SEQ bit in the command register, 
> > > so the
> > > spare-area register is not used. Also the value currently being 
> > > written is
> > > actual wrong, the ecc sits at "column + 
> > > CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE"
> > > not just CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE.
> > > 
> > > So the current code only serves to confuse the user -> remove it.
> > > 
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > There's a bunch of other uses of the syndrome parameter in this
> > function. Does syndrome=true work even without this particular bit 
> > of
> > code?
> 
> Yes, since in syndrome we are passing in the NFC_SEQ bit in the 
> command
> register, which tells the controller that the ecc data is directly 
> behind
> the actual data, the spare-area address register is not used.
> 
> I stumbled over this because the code we had was writing the wrong address
> for the ecc data to the spare-area address register, leading me to wonder
> why syndrome mode was working at all.

Right, that was the line of thinking which took me here too.

> > I suppose I'm asking, should the paramter and the other uses be
> > removed? Or should an ASSERT(!syndrome) be added, or am I worrying
> > about nothing and everything is just fine as it is after this 
> > patch?
> 
> Everything is just fine after this patch, actually it was fine before
> this patch except that it did an unnecessary write, with the wrong
> ecc data address which I found confusing, hence this patch to remove
> that write.

Super, thanks for the explanation. The Ack stands then.

> 
> > 
> > I suspect the latter, so if that is indeed the case:
> >      Acked-by: Ian Campbell <    ijc@hellion.org.uk    >
> > 
> 
> Thanks for all the reviews.
> 
> Regards,
> 
> Hans
> 

[U-Boot] [PATCH 01/15] sunxi_nand_spl: Fix CONFIG_SPL_NAND_SUNXI handling

[Maxime Ripard]

On Sat, Aug 15, 2015 at 10:02:34PM +0200, Hans de Goede wrote:
> CONFIG_SPL_NAND_SUPPORT gets used via IS_ENABLED so it must be defined
> to 1, rather then just being defined.
> 
> While at remove 2 other unused NAND related defines from sunxi-common.h.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  include/configs/sunxi-common.h | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 1abf73c..3735afb 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -138,15 +138,10 @@
>  #define CONFIG_SERIAL_TAG
>  
>  #if defined(CONFIG_SPL_NAND_SUNXI)
> -#define CONFIG_SPL_NAND_DRIVERS
> -#define CONFIG_SPL_NAND_SUPPORT
> -
> -#define CONFIG_SYS_NAND_SPL_KERNEL_OFFS 0x280000
> +#define CONFIG_SPL_NAND_SUPPORT 1

Every other platform just use an empty define for this one, and the
only user of IS_ENABLED together with this option is the sunxi
board. Why not simply replace the IS_ENABLED by an ifdef to maintain
consistency?

AFAIK, IS_ENABLED doesn't bring anythnig to the table here.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150819/6329d7aa/attachment.sig>

[U-Boot] [PATCH 07/15] sunxi_nand_spl: Turn off clocks when we're done with the nand

[Maxime Ripard]

On Sat, Aug 15, 2015 at 10:02:40PM +0200, Hans de Goede wrote:
> Turn off the nand and dma clocks when we're done with the nand, this
> puts the nand and dma controllers back into a clean state for when the
> kernel boots.
> 
> Without this the kernel will not boot properly when it is build with
> dma-controller support.

The kernel shouldn't behave like that. What issue are you seeing?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150819/d3d18c5e/attachment.sig>

[U-Boot] [PATCH 01/15] sunxi_nand_spl: Fix CONFIG_SPL_NAND_SUNXI handling

[Hans de Goede]

Hi,

On 08/19/2015 03:45 PM, Maxime Ripard wrote:
> On Sat, Aug 15, 2015 at 10:02:34PM +0200, Hans de Goede wrote:
>> CONFIG_SPL_NAND_SUPPORT gets used via IS_ENABLED so it must be defined
>> to 1, rather then just being defined.
>>
>> While at remove 2 other unused NAND related defines from sunxi-common.h.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   include/configs/sunxi-common.h | 7 +------
>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
>> index 1abf73c..3735afb 100644
>> --- a/include/configs/sunxi-common.h
>> +++ b/include/configs/sunxi-common.h
>> @@ -138,15 +138,10 @@
>>   #define CONFIG_SERIAL_TAG
>>
>>   #if defined(CONFIG_SPL_NAND_SUNXI)
>> -#define CONFIG_SPL_NAND_DRIVERS
>> -#define CONFIG_SPL_NAND_SUPPORT
>> -
>> -#define CONFIG_SYS_NAND_SPL_KERNEL_OFFS 0x280000
>> +#define CONFIG_SPL_NAND_SUPPORT 1
>
> Every other platform just use an empty define for this one, and the
> only user of IS_ENABLED together with this option is the sunxi
> board. Why not simply replace the IS_ENABLED by an ifdef to maintain
> consistency?

Because u-boot is moving to Kconfig which will define it to 1,
and ...

> AFAIK, IS_ENABLED doesn't bring anythnig to the table here.

IS_ENABLED leads too cleaner / easier to read code, so it does
bring something to the table.

REgards,

Hans

[U-Boot] [PATCH 07/15] sunxi_nand_spl: Turn off clocks when we're done with the nand

[Hans de Goede]

Hi,

On 08/19/2015 03:48 PM, Maxime Ripard wrote:
> On Sat, Aug 15, 2015 at 10:02:40PM +0200, Hans de Goede wrote:
>> Turn off the nand and dma clocks when we're done with the nand, this
>> puts the nand and dma controllers back into a clean state for when the
>> kernel boots.
>>
>> Without this the kernel will not boot properly when it is build with
>> dma-controller support.
>
> The kernel shouldn't behave like that. What issue are you seeing?

The kernel does not output anything, not even on the serial console,
so it seems to crash quite early on. I guess that the dma controller
has an irq asserted and that that fires as soon as the dma driver
unmasks it.

I did not investigate this further since u-boot really should put
the hardware in a clean state before booting the os, so we want this
change even if the kernel were fixed to deal with the unclean state.

Regards,

Hans

[U-Boot] [PATCH 07/15] sunxi_nand_spl: Turn off clocks when we're done with the nand

[Maxime Ripard]

On Thu, Aug 20, 2015 at 08:35:20AM +0200, Hans de Goede wrote:
> On 08/19/2015 03:48 PM, Maxime Ripard wrote:
> >On Sat, Aug 15, 2015 at 10:02:40PM +0200, Hans de Goede wrote:
> >>Turn off the nand and dma clocks when we're done with the nand, this
> >>puts the nand and dma controllers back into a clean state for when the
> >>kernel boots.
> >>
> >>Without this the kernel will not boot properly when it is build with
> >>dma-controller support.
> >
> >The kernel shouldn't behave like that. What issue are you seeing?
> 
> The kernel does not output anything, not even on the serial console,
> so it seems to crash quite early on. I guess that the dma controller
> has an irq asserted and that that fires as soon as the dma driver
> unmasks it.

Hmmm, fishy.

> I did not investigate this further since u-boot really should put
> the hardware in a clean state before booting the os, so we want this
> change even if the kernel were fixed to deal with the unclean state.

I guess we should aim at "fixing" both. The kernel should not really
care about the state the hardware has been left in, and U-boot should
not really leave the dma controller running either.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150824/4cc77938/attachment.sig>

[U-Boot] [PATCH 01/15] sunxi_nand_spl: Fix CONFIG_SPL_NAND_SUNXI handling

[Maxime Ripard]

On Thu, Aug 20, 2015 at 08:33:01AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 08/19/2015 03:45 PM, Maxime Ripard wrote:
> >On Sat, Aug 15, 2015 at 10:02:34PM +0200, Hans de Goede wrote:
> >>CONFIG_SPL_NAND_SUPPORT gets used via IS_ENABLED so it must be defined
> >>to 1, rather then just being defined.
> >>
> >>While at remove 2 other unused NAND related defines from sunxi-common.h.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>  include/configs/sunxi-common.h | 7 +------
> >>  1 file changed, 1 insertion(+), 6 deletions(-)
> >>
> >>diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> >>index 1abf73c..3735afb 100644
> >>--- a/include/configs/sunxi-common.h
> >>+++ b/include/configs/sunxi-common.h
> >>@@ -138,15 +138,10 @@
> >>  #define CONFIG_SERIAL_TAG
> >>
> >>  #if defined(CONFIG_SPL_NAND_SUNXI)
> >>-#define CONFIG_SPL_NAND_DRIVERS
> >>-#define CONFIG_SPL_NAND_SUPPORT
> >>-
> >>-#define CONFIG_SYS_NAND_SPL_KERNEL_OFFS 0x280000
> >>+#define CONFIG_SPL_NAND_SUPPORT 1
> >
> >Every other platform just use an empty define for this one, and the
> >only user of IS_ENABLED together with this option is the sunxi
> >board. Why not simply replace the IS_ENABLED by an ifdef to maintain
> >consistency?
> 
> Because u-boot is moving to Kconfig which will define it to 1,
> and ...
> 
> >AFAIK, IS_ENABLED doesn't bring anythnig to the table here.
> 
> IS_ENABLED leads too cleaner / easier to read code, so it does
> bring something to the table.

Ok, if the plan is to move to Kconfig eventually, I guess it makes
sense.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150827/7ba8f77a/attachment.sig>