[U-Boot] [PATCH 1/6] Davinci: ea20: set console on UART0

[U-Boot] [PATCH 1/6] Davinci: ea20: set console on UART0

[Stefano Babic]

From: Bastian Ruppert <Bastian.Ruppert@Sewerin.de>

Signed-off-by: Bastian Ruppert <Bastian.Ruppert@Sewerin.de>
Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Sandeep Paulraj <s-paulraj@ti.com>
---
 arch/arm/include/asm/arch-davinci/hardware.h |    2 +-
 board/davinci/ea20/ea20.c                    |   16 ++++++++--------
 include/configs/ea20.h                       |    2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/arch-davinci/hardware.h b/arch/arm/include/asm/arch-davinci/hardware.h
index df3f549..1892f15 100644
--- a/arch/arm/include/asm/arch-davinci/hardware.h
+++ b/arch/arm/include/asm/arch-davinci/hardware.h
@@ -412,7 +412,7 @@ struct davinci_syscfg_regs {
 #define DAVINCI_SYSCFG_SUSPSRC_I2C		(1 << 16)
 #define DAVINCI_SYSCFG_SUSPSRC_SPI0		(1 << 21)
 #define DAVINCI_SYSCFG_SUSPSRC_SPI1		(1 << 22)
-#define DAVINCI_SYSCFG_SUSPSRC_UART2		(1 << 20)
+#define DAVINCI_SYSCFG_SUSPSRC_UART0		(1 << 18)
 #define DAVINCI_SYSCFG_SUSPSRC_TIMER0		(1 << 27)
 
 /* Interrupt controller */
diff --git a/board/davinci/ea20/ea20.c b/board/davinci/ea20/ea20.c
index 9d0f71b..4e4107a 100644
--- a/board/davinci/ea20/ea20.c
+++ b/board/davinci/ea20/ea20.c
@@ -48,12 +48,12 @@ static const struct pinmux_config spi1_pins[] = {
 	{ pinmux(5), 1, 5 }
 };
 
-/* UART pin muxer settings */
+/* UART0 pin muxer settings */
 static const struct pinmux_config uart_pins[] = {
-	{ pinmux(0), 4, 6 },
-	{ pinmux(0), 4, 7 },
-	{ pinmux(4), 2, 4 },
-	{ pinmux(4), 2, 5 }
+	{ pinmux(3), 2, 7 },
+	{ pinmux(3), 2, 6 },
+	{ pinmux(3), 2, 4 },
+	{ pinmux(3), 2, 5 }
 };
 
 #ifdef CONFIG_DRIVER_TI_EMAC
@@ -104,7 +104,7 @@ static const struct lpsc_resource lpsc[] = {
 	{ DAVINCI_LPSC_AEMIF },	/* NAND, NOR */
 	{ DAVINCI_LPSC_SPI1 },	/* Serial Flash */
 	{ DAVINCI_LPSC_EMAC },	/* image download */
-	{ DAVINCI_LPSC_UART2 },	/* console */
+	{ DAVINCI_LPSC_UART0 },	/* console */
 	{ DAVINCI_LPSC_GPIO },
 };
 
@@ -150,7 +150,7 @@ int board_init(void)
 	writel(readl(&davinci_syscfg_regs->suspsrc) &
 	       ~(DAVINCI_SYSCFG_SUSPSRC_EMAC | DAVINCI_SYSCFG_SUSPSRC_I2C |
 		 DAVINCI_SYSCFG_SUSPSRC_SPI1 | DAVINCI_SYSCFG_SUSPSRC_TIMER0 |
-		 DAVINCI_SYSCFG_SUSPSRC_UART2),
+		 DAVINCI_SYSCFG_SUSPSRC_UART0),
 	       &davinci_syscfg_regs->suspsrc);
 
 	/* configure pinmux settings */
@@ -167,7 +167,7 @@ int board_init(void)
 	/* enable the console UART */
 	writel((DAVINCI_UART_PWREMU_MGMT_FREE | DAVINCI_UART_PWREMU_MGMT_URRST |
 		DAVINCI_UART_PWREMU_MGMT_UTRST),
-	       &davinci_uart2_ctrl_regs->pwremu_mgmt);
+	       &davinci_uart0_ctrl_regs->pwremu_mgmt);
 
 	return 0;
 }
diff --git a/include/configs/ea20.h b/include/configs/ea20.h
index 48ce945..601990a 100644
--- a/include/configs/ea20.h
+++ b/include/configs/ea20.h
@@ -67,7 +67,7 @@
 #define CONFIG_SYS_NS16550
 #define CONFIG_SYS_NS16550_SERIAL
 #define CONFIG_SYS_NS16550_REG_SIZE	-4	/* NS16550 register size */
-#define CONFIG_SYS_NS16550_COM1	DAVINCI_UART2_BASE /* Base address of UART2 */
+#define CONFIG_SYS_NS16550_COM1	DAVINCI_UART0_BASE /* Base address of UART0 */
 #define CONFIG_SYS_NS16550_CLK	clk_get(DAVINCI_UART2_CLKID)
 #define CONFIG_CONS_INDEX	1		/* use UART0 for console */
 #define CONFIG_BAUDRATE		115200		/* Default baud rate */
-- 
1.7.1

[U-Boot] [PATCH 2/6] Davinci: ea20: set GPIOs to hold MII-Phy in reset and set UART0-Switch for console

[Stefano Babic]

From: Bastian Ruppert <Bastian.Ruppert@Sewerin.de>

Signed-off-by: Bastian Ruppert <Bastian.Ruppert@Sewerin.de>
Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: dzu at denx.de
CC: Sandeep Paulraj <s-paulraj@ti.com>
---
 arch/arm/include/asm/arch-davinci/hardware.h |    4 +++
 board/davinci/ea20/ea20.c                    |   30 +++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/arch/arm/include/asm/arch-davinci/hardware.h b/arch/arm/include/asm/arch-davinci/hardware.h
index 1892f15..ea61913 100644
--- a/arch/arm/include/asm/arch-davinci/hardware.h
+++ b/arch/arm/include/asm/arch-davinci/hardware.h
@@ -156,6 +156,10 @@ typedef volatile unsigned int *	dv_reg_p;
 #define GPIO_BANK2_REG_OPDATA_ADDR		(DAVINCI_GPIO_BASE + 0x3c)
 #define GPIO_BANK2_REG_SET_ADDR			(DAVINCI_GPIO_BASE + 0x40)
 #define GPIO_BANK2_REG_CLR_ADDR			(DAVINCI_GPIO_BASE + 0x44)
+#define GPIO_BANK6_REG_DIR_ADDR			(DAVINCI_GPIO_BASE + 0x88)
+#define GPIO_BANK6_REG_OPDATA_ADDR		(DAVINCI_GPIO_BASE + 0x8c)
+#define GPIO_BANK6_REG_SET_ADDR			(DAVINCI_GPIO_BASE + 0x90)
+#define GPIO_BANK6_REG_CLR_ADDR			(DAVINCI_GPIO_BASE + 0x94)
 #endif /* CONFIG_SOC_DA8XX */
 
 /* Power and Sleep Controller (PSC) Domains */
diff --git a/board/davinci/ea20/ea20.c b/board/davinci/ea20/ea20.c
index 4e4107a..d1b56bf 100644
--- a/board/davinci/ea20/ea20.c
+++ b/board/davinci/ea20/ea20.c
@@ -34,6 +34,7 @@
 #include <asm/arch/emif_defs.h>
 #include <asm/arch/emac_defs.h>
 #include <asm/io.h>
+#include <asm/arch/gpio_defs.h>
 #include <asm/arch/davinci_misc.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -90,6 +91,12 @@ const struct pinmux_config nand_pins[] = {
 };
 #endif
 
+const struct pinmux_config gpio_pins[] = {
+	{ pinmux(13), 8, 0 }, /* GPIO6[15] RESETOUTn on SOM*/
+	{ pinmux(13), 8, 5 }, /* GPIO6[10] U0_SW0 on EA20-00101_2*/
+	{ pinmux(13), 8, 3 }  /* GPIO6[12] U0_SW1 on EA20-00101_2*/
+};
+
 static const struct pinmux_resource pinmuxes[] = {
 #ifdef CONFIG_SPI_FLASH
 	PINMUX_ITEM(spi1_pins),
@@ -110,11 +117,32 @@ static const struct lpsc_resource lpsc[] = {
 
 int board_init(void)
 {
+	struct davinci_gpio *gpio6_base =
+			(struct davinci_gpio *)DAVINCI_GPIO_BANK67;
+
+	/* PinMux for GPIO */
+	if (davinci_configure_pin_mux(gpio_pins, ARRAY_SIZE(gpio_pins)) != 0)
+		return 1;
+
+	/* Set the RESETOUTn low */
+	writel((readl(&gpio6_base->set_data) & ~(1 << 15)),
+		&gpio6_base->set_data);
+	writel((readl(&gpio6_base->dir) & ~(1 << 15)), &gpio6_base->dir);
+
+	/* Set U0_SW0 low for UART0 as console*/
+	writel((readl(&gpio6_base->set_data) & ~(1 << 10)),
+		&gpio6_base->set_data);
+	writel((readl(&gpio6_base->dir) & ~(1 << 10)), &gpio6_base->dir);
+
+	/* Set U0_SW1 low for UART0 as console*/
+	writel((readl(&gpio6_base->set_data) & ~(1 << 12)),
+		&gpio6_base->set_data);
+	writel((readl(&gpio6_base->dir) & ~(1 << 12)), &gpio6_base->dir);
+
 #ifndef CONFIG_USE_IRQ
 	irq_init();
 #endif
 
-
 #ifdef CONFIG_NAND_DAVINCI
 	/*
 	 * NAND CS setup - cycle counts based on da850evm NAND timings in the
-- 
1.7.1

[U-Boot] [PATCH 3/6] Davinci: ea20: Add NAND support

[Stefano Babic]

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Sandeep Paulraj <s-paulraj@ti.com>
CC: Scott Wood <scottwood@freescale.com>
---
 board/davinci/ea20/ea20.c |   35 ++++++++++++++++++-----------------
 include/configs/ea20.h    |   20 +++++++++++++++++---
 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/board/davinci/ea20/ea20.c b/board/davinci/ea20/ea20.c
index d1b56bf..f43d644 100644
--- a/board/davinci/ea20/ea20.c
+++ b/board/davinci/ea20/ea20.c
@@ -74,20 +74,21 @@ static const struct pinmux_config emac_pins[] = {
 
 #ifdef CONFIG_NAND_DAVINCI
 const struct pinmux_config nand_pins[] = {
-	{ pinmux(7), 1, 1 },
-	{ pinmux(7), 1, 2 },
-	{ pinmux(7), 1, 4 },
-	{ pinmux(7), 1, 5 },
-	{ pinmux(9), 1, 0 },
-	{ pinmux(9), 1, 1 },
-	{ pinmux(9), 1, 2 },
-	{ pinmux(9), 1, 3 },
-	{ pinmux(9), 1, 4 },
-	{ pinmux(9), 1, 5 },
-	{ pinmux(9), 1, 6 },
-	{ pinmux(9), 1, 7 },
-	{ pinmux(12), 1, 5 },
-	{ pinmux(12), 1, 6 }
+	{ pinmux(7), 1, 0},	/* CS2 */
+	{ pinmux(7), 0, 1},	/* CS3  in three state*/
+	{ pinmux(7), 1, 4 },	/* EMA_WE */
+	{ pinmux(7), 1, 5 },	/* EMA_OE */
+	{ pinmux(9), 1, 0 },	/* EMA_D[7] */
+	{ pinmux(9), 1, 1 },	/* EMA_D[6] */
+	{ pinmux(9), 1, 2 },	/* EMA_D[5] */
+	{ pinmux(9), 1, 3 },	/* EMA_D[4] */
+	{ pinmux(9), 1, 4 },	/* EMA_D[3] */
+	{ pinmux(9), 1, 5 },	/* EMA_D[2] */
+	{ pinmux(9), 1, 6 },	/* EMA_D[1] */
+	{ pinmux(9), 1, 7 },	/* EMA_D[0] */
+	{ pinmux(12), 1, 5 },	/* EMA_A[2] */
+	{ pinmux(12), 1, 6 },	/* EMA_A[1] */
+	{ pinmux(6), 1, 0 }	/* EMA_CLK */
 };
 #endif
 
@@ -143,20 +144,20 @@ int board_init(void)
 	irq_init();
 #endif
 
-#ifdef CONFIG_NAND_DAVINCI
 	/*
 	 * NAND CS setup - cycle counts based on da850evm NAND timings in the
 	 * Linux kernel @ 25MHz EMIFA
 	 */
+#ifdef CONFIG_NAND_DAVINCI
 	writel((DAVINCI_ABCR_WSETUP(0) |
-		DAVINCI_ABCR_WSTROBE(0) |
+		DAVINCI_ABCR_WSTROBE(1) |
 		DAVINCI_ABCR_WHOLD(0) |
 		DAVINCI_ABCR_RSETUP(0) |
 		DAVINCI_ABCR_RSTROBE(1) |
 		DAVINCI_ABCR_RHOLD(0) |
 		DAVINCI_ABCR_TA(0) |
 		DAVINCI_ABCR_ASIZE_8BIT),
-	       &davinci_emif_regs->ab2cr); /* CS3 */
+	       &davinci_emif_regs->ab1cr); /* CS2 */
 #endif
 
 	/* arch number of the board */
diff --git a/include/configs/ea20.h b/include/configs/ea20.h
index 601990a..2a7f531 100644
--- a/include/configs/ea20.h
+++ b/include/configs/ea20.h
@@ -28,6 +28,7 @@
  */
 #define CONFIG_DRIVER_TI_EMAC
 #define CONFIG_USE_SPIFLASH
+#define	CONFIG_SYS_USE_NAND
 #define CONFIG_DRIVER_TI_EMAC_USE_RMII
 
 /*
@@ -101,7 +102,7 @@
 #undef CONFIG_ENV_IS_IN_NAND
 #define CONFIG_ENV_IS_IN_SPI_FLASH
 #define CONFIG_ENV_SIZE			(8 << 10)
-#define CONFIG_ENV_OFFSET		(256 << 10)
+#define CONFIG_ENV_OFFSET		0x80000
 #define CONFIG_ENV_SECT_SIZE		(64 << 10)
 #define CONFIG_SYS_NO_FLASH
 #endif
@@ -153,7 +154,8 @@
 #undef CONFIG_CMD_PING
 #endif
 
-#ifdef CONFIG_USE_NAND
+/* NAND Setup */
+#ifdef CONFIG_SYS_USE_NAND
 #undef CONFIG_CMD_FLASH
 #undef CONFIG_CMD_IMLS
 #define CONFIG_CMD_NAND
@@ -165,8 +167,20 @@
 #define CONFIG_RBTREE
 #define CONFIG_CMD_UBI
 #define CONFIG_CMD_UBIFS
+
+#define CONFIG_NAND_DAVINCI
+#define	CONFIG_SYS_NAND_PAGE_2K
+#define CONFIG_SYS_NAND_CS		2
+#define CONFIG_SYS_NAND_BASE		DAVINCI_ASYNC_EMIF_DATA_CE2_BASE
+#undef CONFIG_SYS_NAND_HW_ECC
+#define CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST
+#define	CONFIG_SYS_NAND_USE_FLASH_BBT
+#define CONFIG_SYS_MAX_NAND_DEVICE	1 /* Max number of NAND devices */
+#define NAND_MAX_CHIPS			1
+#define CONFIG_SYS_64BIT_VSPRINTF	/* needed for nand_util.c */
 #endif
 
+/* SPI Flash */
 #ifdef CONFIG_USE_SPIFLASH
 #undef CONFIG_CMD_IMLS
 #undef CONFIG_CMD_FLASH
@@ -175,7 +189,7 @@
 #define CONFIG_CMD_SAVEENV
 #endif
 
-#if !defined(CONFIG_USE_NAND) && \
+#if !defined(CONFIG_SYS_USE_NAND) && \
 	!defined(CONFIG_USE_NOR) && \
 	!defined(CONFIG_USE_SPIFLASH)
 #define CONFIG_ENV_IS_NOWHERE
-- 
1.7.1

[U-Boot] [PATCH 4/6] Davinci: ea20: Add early init to get early output from console

[Stefano Babic]

Use board_early_init_f so that the full boot log output can be displayed.

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Sandeep Paulraj <s-paulraj@ti.com>
---
 board/davinci/ea20/ea20.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/board/davinci/ea20/ea20.c b/board/davinci/ea20/ea20.c
index f43d644..5025117 100644
--- a/board/davinci/ea20/ea20.c
+++ b/board/davinci/ea20/ea20.c
@@ -116,7 +116,7 @@ static const struct lpsc_resource lpsc[] = {
 	{ DAVINCI_LPSC_GPIO },
 };
 
-int board_init(void)
+int board_early_init_f(void)
 {
 	struct davinci_gpio *gpio6_base =
 			(struct davinci_gpio *)DAVINCI_GPIO_BANK67;
@@ -160,12 +160,6 @@ int board_init(void)
 	       &davinci_emif_regs->ab1cr); /* CS2 */
 #endif
 
-	/* arch number of the board */
-	gd->bd->bi_arch_number = MACH_TYPE_EA20;
-
-	/* address of boot parameters */
-	gd->bd->bi_boot_params = LINUX_BOOT_PARAM_ADDR;
-
 	/*
 	 * Power on required peripherals
 	 * ARM does not have access by default to PSC0 and PSC1
@@ -201,6 +195,16 @@ int board_init(void)
 	return 0;
 }
 
+int board_init(void)
+{
+	/* arch number of the board */
+	gd->bd->bi_arch_number = MACH_TYPE_EA20;
+
+	/* address of boot parameters */
+	gd->bd->bi_boot_params = LINUX_BOOT_PARAM_ADDR;
+
+	return 0;
+}
 #ifdef CONFIG_DRIVER_TI_EMAC
 
 /*
-- 
1.7.1

[U-Boot] [PATCH 5/6] Davinci: ea20: Add default U-Boot environment

[Stefano Babic]

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Sandeep Paulraj <s-paulraj@ti.com>
---
 include/configs/ea20.h |   58 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/include/configs/ea20.h b/include/configs/ea20.h
index 2a7f531..1843dae 100644
--- a/include/configs/ea20.h
+++ b/include/configs/ea20.h
@@ -30,6 +30,7 @@
 #define CONFIG_USE_SPIFLASH
 #define	CONFIG_SYS_USE_NAND
 #define CONFIG_DRIVER_TI_EMAC_USE_RMII
+#define CONFIG_BOARD_EARLY_INIT_F
 
 /*
  * SoC Configuration
@@ -203,4 +204,61 @@
 #define CONFIG_SYS_SDRAM_BASE		0xc0000000
 #define CONFIG_SYS_INIT_SP_ADDR		(CONFIG_SYS_SDRAM_BASE + 0x1000 - \
 					GENERATED_GBL_DATA_SIZE)
+/*
+ * Default environment and default scripts
+ * to update uboot and load kernel
+ */
+#define xstr(s)	str(s)
+#define str(s)	#s
+
+#define CONFIG_HOSTNAME ea20
+#define	CONFIG_EXTRA_ENV_SETTINGS					\
+	"netdev=eth0\0"							\
+	"nfsargs=setenv bootargs root=/dev/nfs rw "			\
+		"nfsroot=${serverip}:${rootpath}\0"			\
+	"ramargs=setenv bootargs root=/dev/ram rw\0"			\
+	"addip_sta=setenv bootargs ${bootargs} "			\
+		"ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}"	\
+		":${hostname}:${netdev}:off panic=1\0"			\
+	"addip_dyn=setenv bootargs ${bootargs} ip=dhcp\0"		\
+	"addip=if test -n ${ipdyn};then run addip_dyn;"			\
+		"else run addip_sta;fi\0"	\
+	"addmtd=setenv bootargs ${bootargs} ${mtdparts}\0"		\
+	"addtty=setenv bootargs ${bootargs}"				\
+		" console=${consoledev},${baudrate}n8\0"		\
+	"addmisc=setenv bootargs ${bootargs} ${misc}\0"			\
+	"addmem=setenv bootargs ${bootargs} mem=${memory}\0"		\
+	"consoledev=ttyS0\0"						\
+	"loadaddr=c0000014\0"						\
+	"memory=32M\0"							\
+	"kernel_addr_r=c0700000\0"					\
+	"hostname=" xstr(CONFIG_HOSTNAME) "\0"				\
+	"bootfile=" xstr(CONFIG_HOSTNAME) "/uImage\0"			\
+	"ramdisk_file=" xstr(CONFIG_HOSTNAME) "/uRamdisk\0"		\
+	"flash_self=run ramargs addip addtty addmtd addmisc addmem;"	\
+		"bootm ${kernel_addr} ${ramdisk_addr}\0"		\
+	"flash_nfs=run nfsargs addip addtty addmtd addmisc addmem;"	\
+		"bootm ${kernel_addr}\0"				\
+	"net_nfs=tftp ${kernel_addr_r} ${bootfile}; "			\
+		"run nfsargs addip addtty addmtd addmisc addmem;"	\
+		"bootm ${kernel_addr_r}\0"				\
+	"net_self_load=tftp ${kernel_addr_r} ${bootfile};"		\
+		"tftp ${ramdisk_addr_r} ${ramdisk_file};\0"		\
+	"u-boot=" xstr(CONFIG_HOSTNAME) "/u-boot.bin\0"			\
+	"load=tftp ${loadaddr} ${u-boot}\0"				\
+	"load=if sf probe 0;then sf read c0000000 0x10000 0x60000;"	\
+		"tftp c0000014 ${u-boot};"				\
+		"mw c0000008 ${filesize};"				\
+		"fi\0"		\
+	"update=if sf probe 0;then sf erase 10000 60000;"		\
+		"sf write c0000000 10000 60000;"			\
+		"fi\0"							\
+	"upd=if run load;then echo Updating u-boot;if run update;"	\
+		"then echo U-Boot updated;"				\
+			"else echo Error updating u-boot !;"		\
+			"echo Board without bootloader !!;"		\
+		"fi;"							\
+		"else echo U-Boot not downloaded..exiting;fi\0"		\
+	"bootcmd=run net_nfs\0"
+
 #endif /* __CONFIG_H */
-- 
1.7.1

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Stefano Babic]

The NAND controller does not support subpage accessing. This is
not used at all for MLC NAND, but it is set for SLC NAND. UBI tries
to access to subpages, and because it fails, it starts "torture tests"
on the whole page that are always successful, making an endless loop
with the UBI background task taking most CPU time.
On the console, the issue is recognized by the messages:

UBI error: ubi_io_read: error -74 (ECC error) while reading 512 bytes from
PEB 37:512, read 512 bytes
UBI: run torture test for PEB 37
UBI: PEB 37 passed torture test, do not mark it a bad

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Ben Gardiner<bengardiner@nanometrics.ca>
CC: Sandeep Paulraj <s-paulraj@ti.com>
CC: Scott Wood <scottwood@freescale.com>
---
 README                          |    4 ++++
 drivers/mtd/nand/davinci_nand.c |    3 +++
 include/configs/ea20.h          |    1 +
 include/linux/mtd/nand.h        |    3 ++-
 4 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/README b/README
index 21cd71b..8d664eb 100644
--- a/README
+++ b/README
@@ -2907,6 +2907,10 @@ Low Level (hardware related) configuration options:
 		that is executed before the actual U-Boot. E.g. when
 		compiling a NAND SPL.
 
+- CONFIG_SYS_NAND_NO_SUBPAGE
+		Some drivers (davinci) do not support access to NAND subpage.
+
+
 Building the Software:
 ======================
 
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index d41579c..f6c7d09 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -609,6 +609,9 @@ void davinci_nand_init(struct nand_chip *nand)
 #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
 	nand->options	  |= NAND_USE_FLASH_BBT;
 #endif
+#ifdef CONFIG_SYS_NAND_NO_SUBPAGE
+	nand->options	  |= NAND_NO_SUBPAGE_WRITE;
+#endif
 #ifdef CONFIG_SYS_NAND_HW_ECC
 	nand->ecc.mode = NAND_ECC_HW;
 	nand->ecc.size = 512;
diff --git a/include/configs/ea20.h b/include/configs/ea20.h
index 1843dae..9e5dda4 100644
--- a/include/configs/ea20.h
+++ b/include/configs/ea20.h
@@ -171,6 +171,7 @@
 
 #define CONFIG_NAND_DAVINCI
 #define	CONFIG_SYS_NAND_PAGE_2K
+#define CONFIG_SYS_NAND_NO_SUBPAGE
 #define CONFIG_SYS_NAND_CS		2
 #define CONFIG_SYS_NAND_BASE		DAVINCI_ASYNC_EMIF_DATA_CE2_BASE
 #undef CONFIG_SYS_NAND_HW_ECC
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 987a2ec..215e781 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -193,7 +193,8 @@ typedef enum {
 					&& (chip->page_shift > 9))
 
 /* Mask to zero out the chip options, which come from the id table */
-#define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR)
+#define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR & \
+				~NAND_NO_SUBPAGE_WRITE)
 
 /* Non chip related options */
 /* Use a flash based bad block table. This option is passed to the
-- 
1.7.1

[U-Boot] [PATCH 3/6] Davinci: ea20: Add NAND support

[Ben Gardiner]

On Sat, Apr 9, 2011 at 2:05 PM, Stefano Babic <sbabic@denx.de> wrote:
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Sandeep Paulraj <s-paulraj@ti.com>
> CC: Scott Wood <scottwood@freescale.com>

> [...]
> @@ -143,20 +144,20 @@ int board_init(void)
> ? ? ? ?irq_init();
> ?#endif
>
> -#ifdef CONFIG_NAND_DAVINCI
> ? ? ? ?/*
> ? ? ? ? * NAND CS setup - cycle counts based on da850evm NAND timings in the
> ? ? ? ? * Linux kernel @ 25MHz EMIFA
> ? ? ? ? */
> +#ifdef CONFIG_NAND_DAVINCI
> ? ? ? ?writel((DAVINCI_ABCR_WSETUP(0) |
> - ? ? ? ? ? ? ? DAVINCI_ABCR_WSTROBE(0) |
> + ? ? ? ? ? ? ? DAVINCI_ABCR_WSTROBE(1) |

If WSTROBE is modified then the timings are no longer solely based on
the NAND timings in arch/arm/mach-davinci/board-da850-evm.c in the
Linux kernel. Can you add an amendment to the comment describing the
motivation for the extra WSTROBE cycle?

> [...]

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Ben Gardiner]

Hi Stefano,

Thanks for sharing this patch -- I have been using the "-O 2048" (VID
header offset) option to prevent subpages here.

On Sat, Apr 9, 2011 at 2:05 PM, Stefano Babic <sbabic@denx.de> wrote:
> The NAND controller does not support subpage accessing. This is
> not used at all for MLC NAND, but it is set for SLC NAND. UBI tries
> to access to subpages, and because it fails, it starts "torture tests"

"tries to access subpages" is maybe a little too vague; I think Jon
Povey described the problem quite succinctly:

"the problem is that the ECC generated for an all-FFs page is not
all-FFs, and subpage writes are handled by
nand_do_write_ops() by writing a full page with FFs in the unset data
areas." [1]

> on the whole page that are always successful, making an endless loop
> with the UBI background task taking most CPU time.
> On the console, the issue is recognized by the messages:
>
> UBI error: ubi_io_read: error -74 (ECC error) while reading 512 bytes from
> PEB 37:512, read 512 bytes
> UBI: run torture test for PEB 37
> UBI: PEB 37 passed torture test, do not mark it a bad
>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Ben Gardiner<bengardiner@nanometrics.ca>
> CC: Sandeep Paulraj <s-paulraj@ti.com>
> CC: Scott Wood <scottwood@freescale.com>

Is it worth mentioning that this will create UBI partitions that are
not usable in Linux with a similar patch or a VID header offset
workaround?

> ---
> ?README ? ? ? ? ? ? ? ? ? ? ? ? ?| ? ?4 ++++
> ?drivers/mtd/nand/davinci_nand.c | ? ?3 +++
> ?include/configs/ea20.h ? ? ? ? ?| ? ?1 +
> ?include/linux/mtd/nand.h ? ? ? ?| ? ?3 ++-
> ?4 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/README b/README
> index 21cd71b..8d664eb 100644
> --- a/README
> +++ b/README
> @@ -2907,6 +2907,10 @@ Low Level (hardware related) configuration options:
> ? ? ? ? ? ? ? ?that is executed before the actual U-Boot. E.g. when
> ? ? ? ? ? ? ? ?compiling a NAND SPL.
>
> +- CONFIG_SYS_NAND_NO_SUBPAGE
> + ? ? ? ? ? ? ? Some drivers (davinci) do not support access to NAND subpage.
> +
> +
> ?Building the Software:
> ?======================
>
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index d41579c..f6c7d09 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -609,6 +609,9 @@ void davinci_nand_init(struct nand_chip *nand)
> ?#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
> ? ? ? ?nand->options ? ? |= NAND_USE_FLASH_BBT;
> ?#endif
> +#ifdef CONFIG_SYS_NAND_NO_SUBPAGE
> + ? ? ? nand->options ? ? |= NAND_NO_SUBPAGE_WRITE;
> +#endif
> ?#ifdef CONFIG_SYS_NAND_HW_ECC
> ? ? ? ?nand->ecc.mode = NAND_ECC_HW;
> ? ? ? ?nand->ecc.size = 512;
> diff --git a/include/configs/ea20.h b/include/configs/ea20.h
> index 1843dae..9e5dda4 100644
> --- a/include/configs/ea20.h
> +++ b/include/configs/ea20.h
> @@ -171,6 +171,7 @@
>
> ?#define CONFIG_NAND_DAVINCI
> ?#define ? ? ? ?CONFIG_SYS_NAND_PAGE_2K
> +#define CONFIG_SYS_NAND_NO_SUBPAGE
> ?#define CONFIG_SYS_NAND_CS ? ? ? ? ? ? 2
> ?#define CONFIG_SYS_NAND_BASE ? ? ? ? ? DAVINCI_ASYNC_EMIF_DATA_CE2_BASE
> ?#undef CONFIG_SYS_NAND_HW_ECC
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 987a2ec..215e781 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -193,7 +193,8 @@ typedef enum {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& (chip->page_shift > 9))
>
> ?/* Mask to zero out the chip options, which come from the id table */
> -#define NAND_CHIPOPTIONS_MSK ? (0x0000ffff & ~NAND_NO_AUTOINCR)
> +#define NAND_CHIPOPTIONS_MSK ? (0x0000ffff & ~NAND_NO_AUTOINCR & \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ~NAND_NO_SUBPAGE_WRITE)

First let me say that I would be happy to have a fix for this merged
-- so I think the change is "good enough" since it makes UBI work
without specifying a VID header offset equal to the NAND page size.
What follows is more topical musings that any particular criticisms
that should be considered blockers of your patch.

I'm wondering about the long-term path for davinci NAND and subpage writes...

But the sentiment I've heard about adding an exception to
NAND_CHIPOPTIONS_MSK as above is that we are mixing features of the
NAND chip and features of the NAND controller (If you have a modern
SLC NAND then your chip probably supports subpage writes whereas it is
the controller that needs to be limited).

What's more is that the davinci nand controller could do subpage
writing if the writing operation were informed of the extents of the
subpage being written instead of being handed a buffer with 0xFF in
the non-target page areas. Which, I believe is Artem's primary
motivation for the introduction of nand_write_subpage() to the davinci
NAND controller driver [2].

So if the nand_write_subpage family of functions was introduced to the
Linux kernel instead of adding another exception to
NAND_CHIPOPTIONS_MSK then we would have 3 ways to use UBI on davinci
NAND:
1) no patch: VID offset <page size> required
2) chip NAND_NO_SUBPAGE_WRITE patch
3) subpage writes support with nand_write_subpage()

systems with 2) or 3) could always use UBI as in 1) and a UBI volume
made with 2) would need to be attached as in 1) on systems with 2) or
3) ; but a UBI volume made with 3) could not be used on systems with
1) or 2) which means that we could not make use of the efficiency
benefits of nand_write_subpage() if/when it is available on systems
which need access to UBI from U-boot.

I've cc'd Artem and Jon in the hopes that they can correct any of my
leaps above -- what I'd really like is to understand a migration
strategy to actually using subpage writes with davinci NAND in the
future.

Best Regards,
Ben Gardiner

[1] http://article.gmane.org/gmane.linux.davinci/19853
[2] http://article.gmane.org/gmane.linux.drivers.mtd/31581

---
Nanometrics Inc.
http://www.nanometrics.ca

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Scott Wood]

On Sat, Apr 09, 2011 at 08:05:35PM +0200, Stefano Babic wrote:
> diff --git a/README b/README
> index 21cd71b..8d664eb 100644
> --- a/README
> +++ b/README
> @@ -2907,6 +2907,10 @@ Low Level (hardware related) configuration options:
>  		that is executed before the actual U-Boot. E.g. when
>  		compiling a NAND SPL.
>  
> +- CONFIG_SYS_NAND_NO_SUBPAGE
> +		Some drivers (davinci) do not support access to NAND subpage.

This only controls the davinci driver, so it should be
CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE.

Is this really board-specific?  Does the davinci driver ever support
subpage?

> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 987a2ec..215e781 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -193,7 +193,8 @@ typedef enum {
>  					&& (chip->page_shift > 9))
>  
>  /* Mask to zero out the chip options, which come from the id table */
> -#define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR)
> +#define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR & \
> +				~NAND_NO_SUBPAGE_WRITE)

I wonder what we really need CHIPOPTIONS_MSK for?  Silently ignoring what
the driver asked for doesn't seem right.  Can't we just OR the two
sources?  And it would be a driver error to specify a flag that doesn't
make sense to be set this way (i.e. only do it with the "doesn't support
X" flags).

-Scott

RE: [PATCH 6/6] EA20: do not use subpage write for NAND

[Jon Povey]

Ben Gardiner wrote:
> "tries to access subpages" is maybe a little too vague; I think Jon
> Povey described the problem quite succinctly:

That's me, I better wake up!

> What's more is that the davinci nand controller could do subpage
> writing if the writing operation were informed of the extents of the
> subpage being written instead of being handed a buffer with 0xFF in
> the non-target page areas. Which, I believe is Artem's primary
> motivation for the introduction of nand_write_subpage() to the
> davinci NAND controller driver [2].

I can't remember if I posted about this at the time, but what I ended up
doing was propagating the subpage start offset and length through to all
the write_page() functions. At the appropriate place the ECC for any
"unwritten" subpages is FF'd out. It's a fairly ugly platform-specific
hack but it works, if anyone wants to see it I can dig out the patch.

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Jon Povey]

Ben Gardiner wrote:
> "tries to access subpages" is maybe a little too vague; I think Jon
> Povey described the problem quite succinctly:

That's me, I better wake up!

> What's more is that the davinci nand controller could do subpage
> writing if the writing operation were informed of the extents of the
> subpage being written instead of being handed a buffer with 0xFF in
> the non-target page areas. Which, I believe is Artem's primary
> motivation for the introduction of nand_write_subpage() to the
> davinci NAND controller driver [2].

I can't remember if I posted about this at the time, but what I ended up
doing was propagating the subpage start offset and length through to all
the write_page() functions. At the appropriate place the ECC for any
"unwritten" subpages is FF'd out. It's a fairly ugly platform-specific
hack but it works, if anyone wants to see it I can dig out the patch.

--
Jon Povey
jon.povey at racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

[U-Boot] [PATCH 3/6] Davinci: ea20: Add NAND support

[Stefano Babic]

On 04/11/2011 03:05 PM, Ben Gardiner wrote:

Hi Ben,

> 
>> [...]
>> @@ -143,20 +144,20 @@ int board_init(void)
>>        irq_init();
>>  #endif
>>
>> -#ifdef CONFIG_NAND_DAVINCI
>>        /*
>>         * NAND CS setup - cycle counts based on da850evm NAND timings in the
>>         * Linux kernel @ 25MHz EMIFA
>>         */
>> +#ifdef CONFIG_NAND_DAVINCI
>>        writel((DAVINCI_ABCR_WSETUP(0) |
>> -               DAVINCI_ABCR_WSTROBE(0) |
>> +               DAVINCI_ABCR_WSTROBE(1) |
> 
> If WSTROBE is modified then the timings are no longer solely based on
> the NAND timings in arch/arm/mach-davinci/board-da850-evm.c in the
> Linux kernel. Can you add an amendment to the comment describing the
> motivation for the extra WSTROBE cycle?

Really this board will not use at all the board-da850-evm.c, but (no
patches are yet pushed) it will have its own board configuration file
(something like arch/arm/mach-davinci/ea20.c). I understand that if no
timing structure is set into the davinci_nand_pdata in Linux, values set
by the bootloader are not overwritten (this is the case for this board
at the moment). Regarding setting the WSTROBE, I understood from manual
(EMIFA) that WSTROBE and RSTROBE cannot be set to zero if the EMA_WAIT
pin is used, as on this board.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Stefano Babic]

On 04/11/2011 04:04 PM, Ben Gardiner wrote:
> Hi Stefano,

Hi Ben,

> 
> Thanks for sharing this patch -- I have been using the "-O 2048" (VID
> header offset) option to prevent subpages here.

Yes, this works too, at least with Linux.

> 
> On Sat, Apr 9, 2011 at 2:05 PM, Stefano Babic <sbabic@denx.de> wrote:
>> The NAND controller does not support subpage accessing. This is
>> not used at all for MLC NAND, but it is set for SLC NAND. UBI tries
>> to access to subpages, and because it fails, it starts "torture tests"
> 
> "tries to access subpages" is maybe a little too vague; I think Jon
> Povey described the problem quite succinctly:
> 
> "the problem is that the ECC generated for an all-FFs page is not
> all-FFs, and subpage writes are handled by
> nand_do_write_ops() by writing a full page with FFs in the unset data
> areas." [1]

Thanks - I have not found this link. I will merge Jon's comment.

>> on the whole page that are always successful, making an endless loop
>> with the UBI background task taking most CPU time.
>> On the console, the issue is recognized by the messages:
>>
>> UBI error: ubi_io_read: error -74 (ECC error) while reading 512 bytes from
>> PEB 37:512, read 512 bytes
>> UBI: run torture test for PEB 37
>> UBI: PEB 37 passed torture test, do not mark it a bad
>>
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> CC: Ben Gardiner<bengardiner@nanometrics.ca>
>> CC: Sandeep Paulraj <s-paulraj@ti.com>
>> CC: Scott Wood <scottwood@freescale.com>
> 
> Is it worth mentioning that this will create UBI partitions that are
> not usable in Linux with a similar patch or a VID header offset
> workaround?

Yes, better to explain ;-)

> First let me say that I would be happy to have a fix for this merged
> -- so I think the change is "good enough" since it makes UBI work
> without specifying a VID header offset equal to the NAND page size.
> What follows is more topical musings that any particular criticisms
> that should be considered blockers of your patch.
> 
> I'm wondering about the long-term path for davinci NAND and subpage writes...
> 
> But the sentiment I've heard about adding an exception to
> NAND_CHIPOPTIONS_MSK as above is that we are mixing features of the
> NAND chip and features of the NAND controller (If you have a modern
> SLC NAND then your chip probably supports subpage writes whereas it is
> the controller that needs to be limited).

That is true. With this patch I constrained the SLC NAND on my board to
be considered as MLC, as a MLC NAND cannot be accessed in subpage mode.

However, I set also a CONFIG_SYS_NAND_NO_SUBPAGE, and instead of
dropping the option in the mask we could protect the code where the
option is checked. In other words, we could change nand_base.c in this way:

#ifndef CONFIG_SYS_NAND_NO_SUBPAGE
        if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
            !(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
                switch(chip->ecc.steps) {
                case 2:
                        mtd->subpage_sft = 1;
                        break;
                case 4:
                case 8:
                case 16:
                        mtd->subpage_sft = 2;
                        break;
                }
        }
#endif

Then it could be clearer that the restriction is due to the NAND
controller, and not to the chip itself.

> 
> What's more is that the davinci nand controller could do subpage
> writing if the writing operation were informed of the extents of the
> subpage being written instead of being handed a buffer with 0xFF in
> the non-target page areas. Which, I believe is Artem's primary
> motivation for the introduction of nand_write_subpage() to the davinci
> NAND controller driver [2].

Well, of course, if the davinci NAND can handle subpages, we can remove
the limitation. What do you think to add this patch in the way I suggest
now (without touching NAND_CHIPOPTIONS_MSK, that makes the MTD
inconsistent with Linux) until a subpage_write is added to davinci ?

> 
> So if the nand_write_subpage family of functions was introduced to the
> Linux kernel instead of adding another exception to
> NAND_CHIPOPTIONS_MSK then we would have 3 ways to use UBI on davinci
> NAND:
> 1) no patch: VID offset <page size> required
> 2) chip NAND_NO_SUBPAGE_WRITE patch
> 3) subpage writes support with nand_write_subpage()
> 
> systems with 2) or 3) could always use UBI as in 1) and a UBI volume
> made with 2) would need to be attached as in 1) on systems with 2) or
> 3) ; but a UBI volume made with 3) could not be used on systems with
> 1) or 2) which means that we could not make use of the efficiency
> benefits of nand_write_subpage() if/when it is available on systems
> which need access to UBI from U-boot.

I agree with your analyses. As personal feeling, solution one has the
drawback that it cannot be used in u-boot, and it seems to me very easy
to have a wrong UBI on the target. We can consider 2) as temporary
solution, until 3) is available.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Stefano Babic]

On 04/11/2011 09:16 PM, Scott Wood wrote:
> This only controls the davinci driver, so it should be
> CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE.
> 
> Is this really board-specific?

No, really not.

>  Does the davinci driver ever support
> subpage?

No, it does not. This problem affects all boards using the davinci NAND
controller. At least with 850 or OMAP-l1 processors, that I have tested.

>> @@ -193,7 +193,8 @@ typedef enum {
>>  					&& (chip->page_shift > 9))
>>  
>>  /* Mask to zero out the chip options, which come from the id table */
>> -#define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR)
>> +#define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR & \
>> +				~NAND_NO_SUBPAGE_WRITE)
> 
> I wonder what we really need CHIPOPTIONS_MSK for?  Silently ignoring what
> the driver asked for doesn't seem right.  Can't we just OR the two
> sources?  And it would be a driver error to specify a flag that doesn't
> make sense to be set this way (i.e. only do it with the "doesn't support
> X" flags).

Ben reports quite the same issue, as this patch mixes chip options with
NAND controller capabilities.

Probably it is better as I answered to Ben to use
CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE in nand_base.c to switch off subpage
access instead of setting the chip as not able to handle subpages,
changing the  NAND_CHIPOPTIONS_MSK.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Re: [PATCH 6/6] EA20: do not use subpage write for NAND

[Ben Gardiner]

On Tue, Apr 12, 2011 at 5:08 AM, Stefano Babic <sbabic@denx.de> wrote:
> Ben Gardiner wrote:
>> Thanks for sharing this patch -- I have been using the "-O 2048" (VID
>> header offset) option to prevent subpages here.
>
> Yes, this works too, at least with Linux.

(being picky / for archival purposes) In u-boot, specifying the VID
header offset as an argument to the ubi attach command -- i.e. 'ubi
part <mtd_partition> 2048' -- also works (on da850evm, at least).


> [...]
>> First let me say that I would be happy to have a fix for this merged
>> -- so I think the change is "good enough" since it makes UBI work
>> without specifying a VID header offset equal to the NAND page size.
>> What follows is more topical musings that any particular criticisms
>> that should be considered blockers of your patch.
>>
>> I'm wondering about the long-term path for davinci NAND and subpage writes...
>>
>> But the sentiment I've heard about adding an exception to
>> NAND_CHIPOPTIONS_MSK as above is that we are mixing features of the
>> NAND chip and features of the NAND controller (If you have a modern
>> SLC NAND then your chip probably supports subpage writes whereas it is
>> the controller that needs to be limited).
>
> That is true. With this patch I constrained the SLC NAND on my board to
> be considered as MLC, as a MLC NAND cannot be accessed in subpage mode.
>
> However, I set also a CONFIG_SYS_NAND_NO_SUBPAGE, and instead of
> dropping the option in the mask we could protect the code where the
> option is checked. In other words, we could change nand_base.c in this way:
>
> #ifndef CONFIG_SYS_NAND_NO_SUBPAGE
>        if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
>            !(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
>                switch(chip->ecc.steps) {
>                case 2:
>                        mtd->subpage_sft = 1;
>                        break;
>                case 4:
>                case 8:
>                case 16:
>                        mtd->subpage_sft = 2;
>                        break;
>                }
>        }
> #endif
>
> Then it could be clearer that the restriction is due to the NAND
> controller, and not to the chip itself.

I agree -- it keeps the chip and controller options separated.

>> What's more is that the davinci nand controller could do subpage
>> writing if the writing operation were informed of the extents of the
>> subpage being written instead of being handed a buffer with 0xFF in
>> the non-target page areas. Which, I believe is Artem's primary
>> motivation for the introduction of nand_write_subpage() to the davinci
>> NAND controller driver [2].
>
> Well, of course, if the davinci NAND can handle subpages, we can remove
> the limitation. What do you think to add this patch in the way I suggest
> now (without touching NAND_CHIPOPTIONS_MSK, that makes the MTD
> inconsistent with Linux) until a subpage_write is added to davinci ?

Yes, I like the way your proposed changes disabled subpage
initialization instead of adding an exception to the chip options
mask. I'm still OK with merging a patch to disable subpage writes on
da850 -- as an interim solution. i.e. until a subpage_write is added
to the mtd subsystem, and davinci-nand has an override and u-boot gets
a similar change.

>> So if the nand_write_subpage family of functions was introduced to the
>> Linux kernel instead of adding another exception to
>> NAND_CHIPOPTIONS_MSK then we would have 3 ways to use UBI on davinci
>> NAND:
>> 1) no patch: VID offset <page size> required
>> 2) chip NAND_NO_SUBPAGE_WRITE patch
>> 3) subpage writes support with nand_write_subpage()
>>
>> systems with 2) or 3) could always use UBI as in 1) and a UBI volume
>> made with 2) would need to be attached as in 1) on systems with 2) or
>> 3) ; but a UBI volume made with 3) could not be used on systems with
>> 1) or 2) which means that we could not make use of the efficiency
>> benefits of nand_write_subpage() if/when it is available on systems
>> which need access to UBI from U-boot.
>
> I agree with your analyses. As personal feeling, solution one has the
> drawback that it cannot be used in u-boot, and it seems to me very easy
> to have a wrong UBI on the target. We can consider 2) as temporary
> solution, until 3) is available.

Thanks for confirming, Stefano.

I'm not intimately familiar with the ea20 so there could be additional
quirks in the way -- but I can say that option 1) works in u-boot on
the da850evm (+UI board). I use 'ubi part ubi_part 2048' -- ubi_part
is the mtdpart name of my ubi part :)

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Ben Gardiner]

On Tue, Apr 12, 2011 at 5:08 AM, Stefano Babic <sbabic@denx.de> wrote:
> Ben Gardiner wrote:
>> Thanks for sharing this patch -- I have been using the "-O 2048" (VID
>> header offset) option to prevent subpages here.
>
> Yes, this works too, at least with Linux.

(being picky / for archival purposes) In u-boot, specifying the VID
header offset as an argument to the ubi attach command -- i.e. 'ubi
part <mtd_partition> 2048' -- also works (on da850evm, at least).


> [...]
>> First let me say that I would be happy to have a fix for this merged
>> -- so I think the change is "good enough" since it makes UBI work
>> without specifying a VID header offset equal to the NAND page size.
>> What follows is more topical musings that any particular criticisms
>> that should be considered blockers of your patch.
>>
>> I'm wondering about the long-term path for davinci NAND and subpage writes...
>>
>> But the sentiment I've heard about adding an exception to
>> NAND_CHIPOPTIONS_MSK as above is that we are mixing features of the
>> NAND chip and features of the NAND controller (If you have a modern
>> SLC NAND then your chip probably supports subpage writes whereas it is
>> the controller that needs to be limited).
>
> That is true. With this patch I constrained the SLC NAND on my board to
> be considered as MLC, as a MLC NAND cannot be accessed in subpage mode.
>
> However, I set also a CONFIG_SYS_NAND_NO_SUBPAGE, and instead of
> dropping the option in the mask we could protect the code where the
> option is checked. In other words, we could change nand_base.c in this way:
>
> #ifndef CONFIG_SYS_NAND_NO_SUBPAGE
> ? ? ? ?if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
> ? ? ? ? ? ?!(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
> ? ? ? ? ? ? ? ?switch(chip->ecc.steps) {
> ? ? ? ? ? ? ? ?case 2:
> ? ? ? ? ? ? ? ? ? ? ? ?mtd->subpage_sft = 1;
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?case 4:
> ? ? ? ? ? ? ? ?case 8:
> ? ? ? ? ? ? ? ?case 16:
> ? ? ? ? ? ? ? ? ? ? ? ?mtd->subpage_sft = 2;
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
> #endif
>
> Then it could be clearer that the restriction is due to the NAND
> controller, and not to the chip itself.

I agree -- it keeps the chip and controller options separated.

>> What's more is that the davinci nand controller could do subpage
>> writing if the writing operation were informed of the extents of the
>> subpage being written instead of being handed a buffer with 0xFF in
>> the non-target page areas. Which, I believe is Artem's primary
>> motivation for the introduction of nand_write_subpage() to the davinci
>> NAND controller driver [2].
>
> Well, of course, if the davinci NAND can handle subpages, we can remove
> the limitation. What do you think to add this patch in the way I suggest
> now (without touching NAND_CHIPOPTIONS_MSK, that makes the MTD
> inconsistent with Linux) until a subpage_write is added to davinci ?

Yes, I like the way your proposed changes disabled subpage
initialization instead of adding an exception to the chip options
mask. I'm still OK with merging a patch to disable subpage writes on
da850 -- as an interim solution. i.e. until a subpage_write is added
to the mtd subsystem, and davinci-nand has an override and u-boot gets
a similar change.

>> So if the nand_write_subpage family of functions was introduced to the
>> Linux kernel instead of adding another exception to
>> NAND_CHIPOPTIONS_MSK then we would have 3 ways to use UBI on davinci
>> NAND:
>> 1) no patch: VID offset <page size> required
>> 2) chip NAND_NO_SUBPAGE_WRITE patch
>> 3) subpage writes support with nand_write_subpage()
>>
>> systems with 2) or 3) could always use UBI as in 1) and a UBI volume
>> made with 2) would need to be attached as in 1) on systems with 2) or
>> 3) ; but a UBI volume made with 3) could not be used on systems with
>> 1) or 2) which means that we could not make use of the efficiency
>> benefits of nand_write_subpage() if/when it is available on systems
>> which need access to UBI from U-boot.
>
> I agree with your analyses. As personal feeling, solution one has the
> drawback that it cannot be used in u-boot, and it seems to me very easy
> to have a wrong UBI on the target. We can consider 2) as temporary
> solution, until 3) is available.

Thanks for confirming, Stefano.

I'm not intimately familiar with the ea20 so there could be additional
quirks in the way -- but I can say that option 1) works in u-boot on
the da850evm (+UI board). I use 'ubi part ubi_part 2048' -- ubi_part
is the mtdpart name of my ubi part :)

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[U-Boot] [PATCH 3/6] Davinci: ea20: Add NAND support

[Ben Gardiner]

On Tue, Apr 12, 2011 at 3:29 AM, Stefano Babic <sbabic@denx.de> wrote:
> On 04/11/2011 03:05 PM, Ben Gardiner wrote:
>>
>>> [...]
>>> @@ -143,20 +144,20 @@ int board_init(void)
>>> ? ? ? ?irq_init();
>>> ?#endif
>>>
>>> -#ifdef CONFIG_NAND_DAVINCI
>>> ? ? ? ?/*
>>> ? ? ? ? * NAND CS setup - cycle counts based on da850evm NAND timings in the
>>> ? ? ? ? * Linux kernel @ 25MHz EMIFA
>>> ? ? ? ? */
>>> +#ifdef CONFIG_NAND_DAVINCI
>>> ? ? ? ?writel((DAVINCI_ABCR_WSETUP(0) |
>>> - ? ? ? ? ? ? ? DAVINCI_ABCR_WSTROBE(0) |
>>> + ? ? ? ? ? ? ? DAVINCI_ABCR_WSTROBE(1) |
>>
>> If WSTROBE is modified then the timings are no longer solely based on
>> the NAND timings in arch/arm/mach-davinci/board-da850-evm.c in the
>> Linux kernel. Can you add an amendment to the comment describing the
>> motivation for the extra WSTROBE cycle?
>
> Really this board will not use at all the board-da850-evm.c, but (no
> patches are yet pushed) it will have its own board configuration file
> (something like arch/arm/mach-davinci/ea20.c). I understand that if no

Ok. I understand that the ea20 is not the da850evm. I think that if
the comment says "based on da850evm NAND timings" then any change from
those timings should also be noted.

> [...]
> Regarding setting the WSTROBE, I understood from manual
> (EMIFA) that WSTROBE and RSTROBE cannot be set to zero if the EMA_WAIT
> pin is used, as on this board.

Yes, you're absolutely right:

"
Finally, a restriction is placed on the strobe period timing
parameters when operating in Extended Wait
mode. Specifically, the W_STROBE and R_STROBE fields must not be set
to 0 for proper operation.
" [1]

Thanks, Stefano! -- I got it wrong in commit
a3f88293ddd13facd734769c1664d35ab4ed681f da850evm: setup the NAND
flash timings

I just re-inspected the settings assigned by
davinci_aemif_setup_timing() in arch/arm/mach-davinci/aemif.c of the
Linux kernel and they are:

wsetup=0
wstrobe=1	
whold=0
rsetup=0
rstrobe=1
rhold=0
ta=1

So 'WSTROBE'(0) and 'TA(0)' need to be fixed in
board/davinci/da8xxevm/da850evm.c. Patch coming.

I think this means you should also update to TA(1) in your patch --
although it really depends on what the ea20 NAND chip timings are.
	
Best Regards,
Ben Gardiner

[1] http://www.ti.com/litv/pdf/sprufl6f

---
Nanometrics Inc.
http://www.nanometrics.ca

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Scott Wood]

On Tue, 12 Apr 2011 11:44:26 +0200
Stefano Babic <sbabic@denx.de> wrote:

> On 04/11/2011 09:16 PM, Scott Wood wrote:
> > This only controls the davinci driver, so it should be
> > CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE.
> > 
> > Is this really board-specific?
> 
> No, really not.
> 
> >  Does the davinci driver ever support
> > subpage?
> 
> No, it does not. This problem affects all boards using the davinci NAND
> controller. At least with 850 or OMAP-l1 processors, that I have tested.
> 
> >> @@ -193,7 +193,8 @@ typedef enum {
> >>  					&& (chip->page_shift > 9))
> >>  
> >>  /* Mask to zero out the chip options, which come from the id table */
> >> -#define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR)
> >> +#define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR & \
> >> +				~NAND_NO_SUBPAGE_WRITE)
> > 
> > I wonder what we really need CHIPOPTIONS_MSK for?  Silently ignoring what
> > the driver asked for doesn't seem right.  Can't we just OR the two
> > sources?  And it would be a driver error to specify a flag that doesn't
> > make sense to be set this way (i.e. only do it with the "doesn't support
> > X" flags).
> 
> Ben reports quite the same issue, as this patch mixes chip options with
> NAND controller capabilities.
> 
> Probably it is better as I answered to Ben to use
> CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE in nand_base.c to switch off subpage
> access instead of setting the chip as not able to handle subpages,
> changing the  NAND_CHIPOPTIONS_MSK.

Davinci-specific #defines do not belong in nand_base.c[1].  The controller
driver should be able to set "this isn't supported" options just as well as
the chip data -- I just don't think it should be limited to this specific
one.

For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR.
Before this thread, I didn't realize it they were getting ignored.  Things
work anyway because the former is an optimization, and the latter is getting
forced on after the masking, for some reason -- does autoincr simply not
work?  Can we remove the code? :-)

-Scott

[1] Nor should it be turned back into a non-davinci define -- what if there
are multiple NAND controllers supported, and only one requires this?  It's
not so bad in U-Boot (I'd still rather avoid it though), but this approach
is not going to go over well in Linux.

How is Linux handling this?

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Stefano Babic]

On 04/13/2011 06:24 PM, Scott Wood wrote:

Hi Scott,

> Davinci-specific #defines do not belong in nand_base.c[1].  The controller
> driver should be able to set "this isn't supported" options just as well as
> the chip data -- I just don't think it should be limited to this specific
> one.

surely, but it is not clear to me how. There is no entry for a
write_subpage function, as this issue does not happen with other
controllers, and I do not see a callback for the driver after the
nand_scan() function, where I thought the driver could change the
options according to its capabilities.

> 
> For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR.
> Before this thread, I didn't realize it they were getting ignored.
>  Things
> work anyway because the former is an optimization, and the latter is getting
> forced on after the masking, for some reason -- does autoincr simply not
> work?  Can we remove the code? :-)

Well, the options are simply ignored, I agree about removing them.

> 
> -Scott
> 
> [1] Nor should it be turned back into a non-davinci define -- what if there
> are multiple NAND controllers supported, and only one requires this?  It's
> not so bad in U-Boot (I'd still rather avoid it though), but this approach
> is not going to go over well in Linux.
> 
> How is Linux handling this?

What I have seen (kernel 2.6.38), the options are ignored in Linux as
well, and this issue is open for the davinci processors.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Scott Wood]

On Fri, 15 Apr 2011 19:34:48 +0200
Stefano Babic <sbabic@denx.de> wrote:

> On 04/13/2011 06:24 PM, Scott Wood wrote:
> 
> Hi Scott,
> 
> > Davinci-specific #defines do not belong in nand_base.c[1].  The controller
> > driver should be able to set "this isn't supported" options just as well as
> > the chip data -- I just don't think it should be limited to this specific
> > one.
> 
> surely, but it is not clear to me how. There is no entry for a
> write_subpage function, as this issue does not happen with other
> controllers,

I don't see where subpage writes are done at all, actually.

> and I do not see a callback for the driver after the
> nand_scan() function, where I thought the driver could change the
> options according to its capabilities.

nand_scan() is broken into head and tail functions.  In Linux, the driver
calls these, and can look at the chip info before tail is called.  In
U-boot, common code drives this, and the controller driver is not involved
-- but it would be good to change this.

> > For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR.
> > Before this thread, I didn't realize it they were getting ignored.
> >  Things
> > work anyway because the former is an optimization, and the latter is getting
> > forced on after the masking, for some reason -- does autoincr simply not
> > work?  Can we remove the code? :-)
> 
> Well, the options are simply ignored, I agree about removing them.

I think it can be enabled by the controller driver between head and tail
(at least on Linux), though I don't see any drivers that do this as far as
a quick grep shows.

-Scott

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Stefano Babic]

On 04/15/2011 10:29 PM, Scott Wood wrote:

> 
> nand_scan() is broken into head and tail functions.  In Linux, the driver
> calls these, and can look at the chip info before tail is called.  In
> U-boot, common code drives this, and the controller driver is not involved
> -- but it would be good to change this.

Agree, we can do in this way.

> 
>>> For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR.
>>> Before this thread, I didn't realize it they were getting ignored.
>>>  Things
>>> work anyway because the former is an optimization, and the latter is getting
>>> forced on after the masking, for some reason -- does autoincr simply not
>>> work?  Can we remove the code? :-)
>>
>> Well, the options are simply ignored, I agree about removing them.
> 
> I think it can be enabled by the controller driver between head and tail
> (at least on Linux), though I don't see any drivers that do this as far as
> a quick grep shows.

The only point is we need that the controller checks the parameters
after the tail part, as the tail part scans the chip and sets its
options, as the SUBPAGE flag. We could add a pre_adjust()  and
post_adjust() functions to be called after the head and tail part of the
nand_scan().

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Scott Wood]

On Fri, 22 Apr 2011 09:13:34 +0200
Stefano Babic <sbabic@denx.de> wrote:

> The only point is we need that the controller checks the parameters
> after the tail part, as the tail part scans the chip and sets its
> options, as the SUBPAGE flag. We could add a pre_adjust()  and
> post_adjust() functions to be called after the head and tail part of the
> nand_scan().

This is what the tail function is supposed to be for.  I don't see
where NAND_NO_AUTOINCR or NAND_NO_SUBPAGE_WRITE are called in the head
(nand_scan_ident/nand_get_flash_type) functions.  I only see
NAND_BUSWIDTH_16 and NAND_4PAGE_ARRAY checked there.

-Scott