[U-Boot] [PATCH 1/6] Davinci: ea20: set console on UART0
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 */
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@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
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
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
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
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
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
/*
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 */
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 Gardinerbengardiner@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
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 Gardinerbengardiner@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
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
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 Gardinerbengardiner@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:
- no patch: VID offset <page size> required
- chip NAND_NO_SUBPAGE_WRITE patch
- 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
- 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
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:
- no patch: VID offset <page size> required
- chip NAND_NO_SUBPAGE_WRITE patch
- 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
- 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
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
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
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?
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
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
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
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
participants (4)
-
Ben Gardiner -
Jon Povey -
Scott Wood -
Stefano Babic