[U-Boot] [PATCH] nand_spl_simple: store ecc data on the stack
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de CC: Heiko Schocher hs@denx.de ---
Note: Ilya has already submitted a patch to fix this issue:
http://patchwork.ozlabs.org/patch/128018/
after discussing on ML about booting linux from SPL, we agree about putting the data on the stack.
drivers/mtd/nand/nand_spl_simple.c | 27 ++++++--------------------- 1 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c index ed821f2..a3d1af0 100644 --- a/drivers/mtd/nand/nand_spl_simple.c +++ b/drivers/mtd/nand/nand_spl_simple.c @@ -145,9 +145,9 @@ static int nand_is_bad_block(int block) static int nand_read_page(int block, int page, uchar *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES]; + u_char ecc_code[CONFIG_SYS_NAND_ECCTOTAL]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; @@ -155,14 +155,6 @@ static int nand_read_page(int block, int page, uchar *dst) uint8_t *p = dst; int stat;
- /* - * No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - nand_command(block, page, 0, NAND_CMD_READOOB); this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE); nand_command(block, page, 0, NAND_CMD_READ0); @@ -185,9 +177,9 @@ static int nand_read_page(int block, int page, uchar *dst) static int nand_read_page(int block, int page, void *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES]; + u_char ecc_code[CONFIG_SYS_NAND_ECCTOTAL]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; @@ -197,13 +189,6 @@ static int nand_read_page(int block, int page, void *dst)
nand_command(block, page, 0, NAND_CMD_READ0);
- /* No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { if (this->ecc.mode != NAND_ECC_SOFT) this->ecc.hwctl(&mtd, NAND_ECC_READ);
Hi Stefano,
thanks for posting this. There is a couple of comments below.
On 11.12.2011 21:22, Stefano Babic wrote:
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c index ed821f2..a3d1af0 100644 --- a/drivers/mtd/nand/nand_spl_simple.c +++ b/drivers/mtd/nand/nand_spl_simple.c @@ -145,9 +145,9 @@ static int nand_is_bad_block(int block) static int nand_read_page(int block, int page, uchar *dst) { struct nand_chip *this = mtd.priv;
- u_char *ecc_calc;
- u_char *ecc_code;
- u_char *oob_data;
- u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
- u_char ecc_code[CONFIG_SYS_NAND_ECCTOTAL];
Doesn't ECCTOTAL always equal to ECCSTEPS * ECCBYTES?
- u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
I think we can save a few bytes by placing ecc_code and oob_data into a union. We only need oob_data to get ecc_code...
Regards, Ilya.
On 12/12/2011 01:08, Ilya Yanok wrote:
Hi Stefano,
thanks for posting this. There is a couple of comments below.
On 11.12.2011 21:22, Stefano Babic wrote:
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c index ed821f2..a3d1af0 100644 --- a/drivers/mtd/nand/nand_spl_simple.c +++ b/drivers/mtd/nand/nand_spl_simple.c @@ -145,9 +145,9 @@ static int nand_is_bad_block(int block) static int nand_read_page(int block, int page, uchar *dst) { struct nand_chip *this = mtd.priv;
- u_char *ecc_calc;
- u_char *ecc_code;
- u_char *oob_data;
- u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
- u_char ecc_code[CONFIG_SYS_NAND_ECCTOTAL];
Doesn't ECCTOTAL always equal to ECCSTEPS * ECCBYTES?
I see. Then we can also remove CONFIG_SYS_NAND_ECCTOTAL from the configuration files, because it is not useful. I will do in V2.
By grepping the code, the same issue is present in nand_spl/nand_boot.c, too. This is part of the old spl code, I do not know if we have also to fix this one. IMHO we can only fix nand_spl_simple.c, because the other one will become obsolete and there is not probelms with current boards.
- u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
I think we can save a few bytes by placing ecc_code and oob_data into a union. We only need oob_data to get ecc_code...
Checking the real values I see that ECCTOTAL is small - 8 bytes on the TI we are currently testing, and it has not a big value on other architectures. So I think we can avoid the union, also because this can generate overlapping when the oob data is copied: I mean in case nand_ecc_pos[i] is smaller as CONFIG_SYS_NAND_ECCTOTAL at:
for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
Regards, Stefano
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de ---
V2: - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok) - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
drivers/mtd/nand/nand_spl_simple.c | 33 +++++++++---------------------- include/configs/am3517_crane.h | 2 - include/configs/am3517_evm.h | 2 - include/configs/devkit8000.h | 2 - include/configs/hawkboard.h | 3 +- include/configs/omap3_beagle.h | 2 - include/configs/omap3_evm.h | 2 - include/configs/omap3_evm_quick_nand.h | 2 - 8 files changed, 11 insertions(+), 37 deletions(-)
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c index ed821f2..fbc0fce 100644 --- a/drivers/mtd/nand/nand_spl_simple.c +++ b/drivers/mtd/nand/nand_spl_simple.c @@ -145,9 +145,9 @@ static int nand_is_bad_block(int block) static int nand_read_page(int block, int page, uchar *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES]; + u_char ecc_code[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; @@ -155,20 +155,13 @@ static int nand_read_page(int block, int page, uchar *dst) uint8_t *p = dst; int stat;
- /* - * No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - nand_command(block, page, 0, NAND_CMD_READOOB); this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE); nand_command(block, page, 0, NAND_CMD_READ0);
/* Pick the ECC bytes out of the oob data */ - for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) + for (i = 0; + i < (CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES); i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
@@ -185,9 +178,9 @@ static int nand_read_page(int block, int page, uchar *dst) static int nand_read_page(int block, int page, void *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES]; + u_char ecc_code[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; @@ -197,13 +190,6 @@ static int nand_read_page(int block, int page, void *dst)
nand_command(block, page, 0, NAND_CMD_READ0);
- /* No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { if (this->ecc.mode != NAND_ECC_SOFT) this->ecc.hwctl(&mtd, NAND_ECC_READ); @@ -213,7 +199,8 @@ static int nand_read_page(int block, int page, void *dst) this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
/* Pick the ECC bytes out of the oob data */ - for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) + for (i = 0; + i < (CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES); i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
eccsteps = CONFIG_SYS_NAND_ECCSTEPS; diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h index 0a0c261..3558dfb 100644 --- a/include/configs/am3517_crane.h +++ b/include/configs/am3517_crane.h @@ -361,8 +361,6 @@ #define CONFIG_SYS_NAND_ECCBYTES 3 #define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h index d44eeec..8ddfaae 100644 --- a/include/configs/am3517_evm.h +++ b/include/configs/am3517_evm.h @@ -362,8 +362,6 @@ #define CONFIG_SYS_NAND_ECCBYTES 3 #define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h index fc7f0cd..58140df 100644 --- a/include/configs/devkit8000.h +++ b/include/configs/devkit8000.h @@ -346,8 +346,6 @@
#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS)
#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE
diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h index 12acb27..0084fc3 100644 --- a/include/configs/hawkboard.h +++ b/include/configs/hawkboard.h @@ -141,8 +141,7 @@ #define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE) #define CONFIG_SYS_NAND_OOBSIZE 64 -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) + #endif /* CONFIG_SYS_USE_NAND */
/* diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 970810c..2736f2a 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -428,8 +428,6 @@ #define CONFIG_SYS_NAND_ECCBYTES 3 #define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index 2ce3959..a82c71e 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -123,8 +123,6 @@ #define CONFIG_SYS_NAND_ECCBYTES 3 #define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h index 2f879c0..caa55a4 100644 --- a/include/configs/omap3_evm_quick_nand.h +++ b/include/configs/omap3_evm_quick_nand.h @@ -93,8 +93,6 @@ #define CONFIG_SYS_NAND_ECCBYTES 3 #define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
On Tue, Dec 13, 2011 at 4:30 AM, Stefano Babic sbabic@denx.de wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
OK, I think we need some convenience defines in nand_spl_simple.c:
- u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
This is long and used in a lot of places. Furthermore...
#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE)
Is what everyone does. So if we replace ECCSTEPS with (PAGE_SIZE/ECCSIZE) it gets eve longer. We should probably #define SYS_NAND_ECCSTEPS, SYS_NAND_ECCTOTAL up top and go with it.
On 13/12/2011 16:58, Tom Rini wrote:
On Tue, Dec 13, 2011 at 4:30 AM, Stefano Babic sbabic@denx.de wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
OK, I think we need some convenience defines in nand_spl_simple.c:
u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];This is long and used in a lot of places. Furthermore...
#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ CONFIG_SYS_NAND_ECCSIZE)
Is what everyone does.
Right, I've missed this one - we can then drop CONFIG_SYS_NAND_ECCSTEPS also from configuration files.
So if we replace ECCSTEPS with (PAGE_SIZE/ECCSIZE) it gets eve longer. We should probably #define SYS_NAND_ECCSTEPS, SYS_NAND_ECCTOTAL up top and go with it.
I'll do in V3.
Regards, Stefano
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de --- V3: - use local defines for CONFIG_SYS_NAND_ECCSTEPS and CONFIG_SYS_NAND_ECCTOTAL (Tom Rini) - drop CONFIG_SYS_NAND_ECCSTEPS from board config files
V2: - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok) - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
drivers/mtd/nand/nand_spl_simple.c | 42 ++++++++++++------------------- include/configs/am3517_crane.h | 4 --- include/configs/am3517_evm.h | 4 --- include/configs/devkit8000.h | 5 ---- include/configs/hawkboard.h | 5 +--- include/configs/omap3_beagle.h | 4 --- include/configs/omap3_evm.h | 4 --- include/configs/omap3_evm_quick_nand.h | 4 --- 8 files changed, 17 insertions(+), 55 deletions(-)
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c index ed821f2..ef34991 100644 --- a/drivers/mtd/nand/nand_spl_simple.c +++ b/drivers/mtd/nand/nand_spl_simple.c @@ -27,6 +27,11 @@ static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS; static nand_info_t mtd; static struct nand_chip nand_chip;
+#define SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ + CONFIG_SYS_NAND_ECCSIZE) +#define SYS_NAND_ECCTOTAL (SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES) + + #if (CONFIG_SYS_NAND_PAGE_SIZE <= 512) /* * NAND command for small page NAND devices (512) @@ -145,30 +150,22 @@ static int nand_is_bad_block(int block) static int nand_read_page(int block, int page, uchar *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[SYS_NAND_ECCTOTAL]; + u_char ecc_code[SYS_NAND_ECCTOTAL]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; - int eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + int eccsteps = SYS_NAND_ECCSTEPS; uint8_t *p = dst; int stat;
- /* - * No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - nand_command(block, page, 0, NAND_CMD_READOOB); this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE); nand_command(block, page, 0, NAND_CMD_READ0);
/* Pick the ECC bytes out of the oob data */ - for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) + for (i = 0; i < (SYS_NAND_ECCTOTAL); i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
@@ -185,25 +182,18 @@ static int nand_read_page(int block, int page, uchar *dst) static int nand_read_page(int block, int page, void *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[SYS_NAND_ECCTOTAL]; + u_char ecc_code[SYS_NAND_ECCTOTAL]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; - int eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + int eccsteps = SYS_NAND_ECCSTEPS; uint8_t *p = dst; int stat;
nand_command(block, page, 0, NAND_CMD_READ0);
- /* No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { if (this->ecc.mode != NAND_ECC_SOFT) this->ecc.hwctl(&mtd, NAND_ECC_READ); @@ -213,10 +203,10 @@ static int nand_read_page(int block, int page, void *dst) this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
/* Pick the ECC bytes out of the oob data */ - for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) + for (i = 0; i < (SYS_NAND_ECCTOTAL); i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
- eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + eccsteps = SYS_NAND_ECCSTEPS; p = dst;
for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h index 0a0c261..b0dd2f0 100644 --- a/include/configs/am3517_crane.h +++ b/include/configs/am3517_crane.h @@ -359,10 +359,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h index d44eeec..f797f3f 100644 --- a/include/configs/am3517_evm.h +++ b/include/configs/am3517_evm.h @@ -360,10 +360,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h index fc7f0cd..6ef6a87 100644 --- a/include/configs/devkit8000.h +++ b/include/configs/devkit8000.h @@ -344,11 +344,6 @@ #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3
-#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) - #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h index 12acb27..65b3b78 100644 --- a/include/configs/hawkboard.h +++ b/include/configs/hawkboard.h @@ -138,11 +138,8 @@ #define CONFIG_SYS_NAND_BAD_BLOCK_POS 0 #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 10 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) #define CONFIG_SYS_NAND_OOBSIZE 64 -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) + #endif /* CONFIG_SYS_USE_NAND */
/* diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 970810c..7a240bb 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -426,10 +426,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index 2ce3959..1fcb7af 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -121,10 +121,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h index 2f879c0..362fa1d 100644 --- a/include/configs/omap3_evm_quick_nand.h +++ b/include/configs/omap3_evm_quick_nand.h @@ -91,10 +91,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
On Tue, Dec 13, 2011 at 10:50 AM, Stefano Babic sbabic@denx.de wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de
Acked-by: Tom Rini trini@ti.com
Which covers the omap config file changes. Assuming Wolfgang doesn't see a problem with using SYS_... in defines, Scott, do you want this via your tree (in /next) since it's NAND or in mine since it's TI boards? Assuming you don't see any problems of course. Thanks!
Dear Tom,
In message CA+M6bX=j+=UBkZ-VGoAJgFtfap+sLp4i2JypDSjJ_z-bCbw6sQ@mail.gmail.com you wrote:
Which covers the omap config file changes. Assuming Wolfgang doesn't see a problem with using SYS_... in defines, Scott, do you want this via your tree (in /next) since it's NAND or in mine since it's TI boards? Assuming you don't see any problems of course. Thanks!
Indeed.
Stefano, what is this SYS_NAND_* supposed to mean? Either it is a config option, then you should name it CONFIG_SYS_NAND_* (or CONFIG_NAND_), or it is something else, in which case the "SYS_" is difficult to swallow for me (unless you have a really good explanation?)
Best regards,
Wolfgang Denk
On Tue, Dec 13, 2011 at 11:18 AM, Wolfgang Denk wd@denx.de wrote:
Dear Tom,
In message CA+M6bX=j+=UBkZ-VGoAJgFtfap+sLp4i2JypDSjJ_z-bCbw6sQ@mail.gmail.com you wrote:
Which covers the omap config file changes. Assuming Wolfgang doesn't see a problem with using SYS_... in defines, Scott, do you want this via your tree (in /next) since it's NAND or in mine since it's TI boards? Assuming you don't see any problems of course. Thanks!
Indeed.
Stefano, what is this SYS_NAND_* supposed to mean? Either it is a config option, then you should name it CONFIG_SYS_NAND_* (or CONFIG_NAND_), or it is something else, in which case the "SYS_" is difficult to swallow for me (unless you have a really good explanation?)
So, we have various characteristics of the NAND chip that need to be described for SPL. Today we do CONFIG_SYS_NAND... but they're unchanged in every implementation and are calculated from other per-board values. So to avoid duplication in each config file, we drop them from there and use them only locally #define calculate them in the file that needs them. Since they're still system specific values, I had suggested calling them SYS_... but I wasn't sure what the right namespace is. I had a recollection that using CONFIG_ outside of config files was a bad idea.
On 12/13/2011 12:33 PM, Tom Rini wrote:
So, we have various characteristics of the NAND chip that need to be described for SPL. Today we do CONFIG_SYS_NAND... but they're unchanged in every implementation and are calculated from other per-board values. So to avoid duplication in each config file, we drop them from there and use them only locally #define calculate them in the file that needs them. Since they're still system specific values, I had suggested calling them SYS_... but I wasn't sure what the right namespace is. I had a recollection that using CONFIG_ outside of config files was a bad idea.
These symbols are not in a header file -- how about just plain ECCSTEPS and ECCTOTAL?
-Scott
On Tue, Dec 13, 2011 at 11:45 AM, Scott Wood scottwood@freescale.com wrote:
On 12/13/2011 12:33 PM, Tom Rini wrote:
So, we have various characteristics of the NAND chip that need to be described for SPL. Today we do CONFIG_SYS_NAND... but they're unchanged in every implementation and are calculated from other per-board values. So to avoid duplication in each config file, we drop them from there and use them only locally #define calculate them in the file that needs them. Since they're still system specific values, I had suggested calling them SYS_... but I wasn't sure what the right namespace is. I had a recollection that using CONFIG_ outside of config files was a bad idea.
These symbols are not in a header file -- how about just plain ECCSTEPS and ECCTOTAL?
Works for me and Wolfgang (on IRC) so lets go there for a hopefully last v4 (along with what Scott said in the other email)
On 13/12/2011 19:45, Scott Wood wrote:
On 12/13/2011 12:33 PM, Tom Rini wrote:
So, we have various characteristics of the NAND chip that need to be described for SPL. Today we do CONFIG_SYS_NAND... but they're unchanged in every implementation and are calculated from other per-board values. So to avoid duplication in each config file, we drop them from there and use them only locally #define calculate them in the file that needs them. Since they're still system specific values, I had suggested calling them SYS_... but I wasn't sure what the right namespace is. I had a recollection that using CONFIG_ outside of config files was a bad idea.
These symbols are not in a header file -- how about just plain ECCSTEPS and ECCTOTAL?
Ok, agree - done in V4.
Regards, Stefano
Am 13/12/2011 19:18, schrieb Wolfgang Denk:
Dear Tom,
In message CA+M6bX=j+=UBkZ-VGoAJgFtfap+sLp4i2JypDSjJ_z-bCbw6sQ@mail.gmail.com you wrote:
Which covers the omap config file changes. Assuming Wolfgang doesn't see a problem with using SYS_... in defines, Scott, do you want this via your tree (in /next) since it's NAND or in mine since it's TI boards? Assuming you don't see any problems of course. Thanks!
Indeed.
Stefano, what is this SYS_NAND_* supposed to mean? Either it is a config option, then you should name it CONFIG_SYS_NAND_* (or CONFIG_NAND_), or it is something else, in which case the "SYS_" is difficult to swallow for me (unless you have a really good explanation?)
As already explained by Tom: we switch to local defines instead of CONFIG_SYS, and we get rid of CONFIG_SYS_ in board configuration files. Inside the single file, we have some freedom to use names, and the names suggested by Tom tells us that they are system specific, but because the CONFIG_SYS_ is missing, they cannot be confused with U-Boot general configuration settings. I find the names (for me) quite self explaining, but if there is some possibility to misunderstand their meaning we can drop the SYS_ prefix.
Best regards, Stefano Babic
On 12/13/2011 12:04 PM, Tom Rini wrote:
On Tue, Dec 13, 2011 at 10:50 AM, Stefano Babic sbabic@denx.de wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de
Acked-by: Tom Rini trini@ti.com
Which covers the omap config file changes. Assuming Wolfgang doesn't see a problem with using SYS_... in defines, Scott, do you want this via your tree (in /next) since it's NAND or in mine since it's TI boards? Assuming you don't see any problems of course. Thanks!
The board-specific stuff is just removing unneeded NAND config items, and isn't depending on other non-NAND patches, so I can take it through my tree -- but if you want me to ACK it (once the minor issues are fixed) to go via your tree so you can build on top of it sooner, that's fine...
-Scott
On Tue, Dec 13, 2011 at 11:52 AM, Scott Wood scottwood@freescale.com wrote:
On 12/13/2011 12:04 PM, Tom Rini wrote:
On Tue, Dec 13, 2011 at 10:50 AM, Stefano Babic sbabic@denx.de wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de
Acked-by: Tom Rini trini@ti.com
Which covers the omap config file changes. Assuming Wolfgang doesn't see a problem with using SYS_... in defines, Scott, do you want this via your tree (in /next) since it's NAND or in mine since it's TI boards? Assuming you don't see any problems of course. Thanks!
The board-specific stuff is just removing unneeded NAND config items, and isn't depending on other non-NAND patches, so I can take it through my tree -- but if you want me to ACK it (once the minor issues are fixed) to go via your tree so you can build on top of it sooner, that's fine...
Just building up my /next tree and don't want this to get lost, so if you can take v4 once it's out and you're happy, I'd appreciate it. Thanks!
On 12/13/2011 11:50 AM, Stefano Babic wrote:
/* Pick the ECC bytes out of the oob data */
- for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
- for (i = 0; i < (SYS_NAND_ECCTOTAL); i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
[snip]
@@ -213,10 +203,10 @@ static int nand_read_page(int block, int page, void *dst) this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
/* Pick the ECC bytes out of the oob data */
- for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
- for (i = 0; i < (SYS_NAND_ECCTOTAL); i++)
No need for parens around SYS_NAND_ECCTOTAL.
-Scott
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de --- V4: - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood) - drop parenthesis around defines (Scott Wood)
V3: - use local defines for CONFIG_SYS_NAND_ECCSTEPS and CONFIG_SYS_NAND_ECCTOTAL (Tom Rini) - drop CONFIG_SYS_NAND_ECCSTEPS from board config files
V2: - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok) - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
drivers/mtd/nand/nand_spl_simple.c | 42 ++++++++++++------------------- include/configs/am3517_crane.h | 4 --- include/configs/am3517_evm.h | 4 --- include/configs/devkit8000.h | 5 ---- include/configs/hawkboard.h | 5 +--- include/configs/omap3_beagle.h | 4 --- include/configs/omap3_evm.h | 4 --- include/configs/omap3_evm_quick_nand.h | 4 --- 8 files changed, 17 insertions(+), 55 deletions(-)
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c index ed821f2..5a1e1a9 100644 --- a/drivers/mtd/nand/nand_spl_simple.c +++ b/drivers/mtd/nand/nand_spl_simple.c @@ -27,6 +27,11 @@ static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS; static nand_info_t mtd; static struct nand_chip nand_chip;
+#define ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ + CONFIG_SYS_NAND_ECCSIZE) +#define ECCTOTAL (ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES) + + #if (CONFIG_SYS_NAND_PAGE_SIZE <= 512) /* * NAND command for small page NAND devices (512) @@ -145,30 +150,22 @@ static int nand_is_bad_block(int block) static int nand_read_page(int block, int page, uchar *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[ECCTOTAL]; + u_char ecc_code[ECCTOTAL]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; - int eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + int eccsteps = ECCSTEPS; uint8_t *p = dst; int stat;
- /* - * No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - nand_command(block, page, 0, NAND_CMD_READOOB); this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE); nand_command(block, page, 0, NAND_CMD_READ0);
/* Pick the ECC bytes out of the oob data */ - for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) + for (i = 0; i < ECCTOTAL; i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
@@ -185,25 +182,18 @@ static int nand_read_page(int block, int page, uchar *dst) static int nand_read_page(int block, int page, void *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[ECCTOTAL]; + u_char ecc_code[ECCTOTAL]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; - int eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + int eccsteps = ECCSTEPS; uint8_t *p = dst; int stat;
nand_command(block, page, 0, NAND_CMD_READ0);
- /* No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { if (this->ecc.mode != NAND_ECC_SOFT) this->ecc.hwctl(&mtd, NAND_ECC_READ); @@ -213,10 +203,10 @@ static int nand_read_page(int block, int page, void *dst) this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
/* Pick the ECC bytes out of the oob data */ - for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) + for (i = 0; i < ECCTOTAL; i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
- eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + eccsteps = ECCSTEPS; p = dst;
for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h index 0a0c261..b0dd2f0 100644 --- a/include/configs/am3517_crane.h +++ b/include/configs/am3517_crane.h @@ -359,10 +359,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h index d44eeec..f797f3f 100644 --- a/include/configs/am3517_evm.h +++ b/include/configs/am3517_evm.h @@ -360,10 +360,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h index fc7f0cd..6ef6a87 100644 --- a/include/configs/devkit8000.h +++ b/include/configs/devkit8000.h @@ -344,11 +344,6 @@ #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3
-#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) - #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h index 12acb27..65b3b78 100644 --- a/include/configs/hawkboard.h +++ b/include/configs/hawkboard.h @@ -138,11 +138,8 @@ #define CONFIG_SYS_NAND_BAD_BLOCK_POS 0 #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 10 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) #define CONFIG_SYS_NAND_OOBSIZE 64 -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) + #endif /* CONFIG_SYS_USE_NAND */
/* diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 970810c..7a240bb 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -426,10 +426,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index 2ce3959..1fcb7af 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -121,10 +121,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h index 2f879c0..362fa1d 100644 --- a/include/configs/omap3_evm_quick_nand.h +++ b/include/configs/omap3_evm_quick_nand.h @@ -91,10 +91,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
On Tue, Dec 13, 2011 at 12:33 PM, Stefano Babic sbabic@denx.de wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de
Acked-by: Tom Rini trini@ti.com
V4: - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood) - drop parenthesis around defines (Scott Wood)
V3: - use local defines for CONFIG_SYS_NAND_ECCSTEPS and CONFIG_SYS_NAND_ECCTOTAL (Tom Rini) - drop CONFIG_SYS_NAND_ECCSTEPS from board config files
V2: - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok) - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
drivers/mtd/nand/nand_spl_simple.c | 42 ++++++++++++------------------- include/configs/am3517_crane.h | 4 --- include/configs/am3517_evm.h | 4 --- include/configs/devkit8000.h | 5 ---- include/configs/hawkboard.h | 5 +--- include/configs/omap3_beagle.h | 4 --- include/configs/omap3_evm.h | 4 --- include/configs/omap3_evm_quick_nand.h | 4 --- 8 files changed, 17 insertions(+), 55 deletions(-)
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c index ed821f2..5a1e1a9 100644 --- a/drivers/mtd/nand/nand_spl_simple.c +++ b/drivers/mtd/nand/nand_spl_simple.c @@ -27,6 +27,11 @@ static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS; static nand_info_t mtd; static struct nand_chip nand_chip;
+#define ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \
- CONFIG_SYS_NAND_ECCSIZE)
+#define ECCTOTAL (ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES)
#if (CONFIG_SYS_NAND_PAGE_SIZE <= 512) /* * NAND command for small page NAND devices (512) @@ -145,30 +150,22 @@ static int nand_is_bad_block(int block) static int nand_read_page(int block, int page, uchar *dst) { struct nand_chip *this = mtd.priv;
- u_char *ecc_calc;
- u_char *ecc_code;
- u_char *oob_data;
- u_char ecc_calc[ECCTOTAL];
- u_char ecc_code[ECCTOTAL];
- u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
- int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
- int eccsteps = ECCSTEPS;
uint8_t *p = dst; int stat;
- /*
- * No malloc available for now, just use some temporary locations
- * in SDRAM
- */
- ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
- ecc_code = ecc_calc + 0x100;
- oob_data = ecc_calc + 0x200;
nand_command(block, page, 0, NAND_CMD_READOOB); this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE); nand_command(block, page, 0, NAND_CMD_READ0);
/* Pick the ECC bytes out of the oob data */
- for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
- for (i = 0; i < ECCTOTAL; i++)
ecc_code[i] = oob_data[nand_ecc_pos[i]];
@@ -185,25 +182,18 @@ static int nand_read_page(int block, int page, uchar *dst) static int nand_read_page(int block, int page, void *dst) { struct nand_chip *this = mtd.priv;
- u_char *ecc_calc;
- u_char *ecc_code;
- u_char *oob_data;
- u_char ecc_calc[ECCTOTAL];
- u_char ecc_code[ECCTOTAL];
- u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
- int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
- int eccsteps = ECCSTEPS;
uint8_t *p = dst; int stat;
nand_command(block, page, 0, NAND_CMD_READ0);
- /* No malloc available for now, just use some temporary locations
- * in SDRAM
- */
- ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
- ecc_code = ecc_calc + 0x100;
- oob_data = ecc_calc + 0x200;
for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { if (this->ecc.mode != NAND_ECC_SOFT) this->ecc.hwctl(&mtd, NAND_ECC_READ); @@ -213,10 +203,10 @@ static int nand_read_page(int block, int page, void *dst) this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
/* Pick the ECC bytes out of the oob data */
- for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
- for (i = 0; i < ECCTOTAL; i++)
ecc_code[i] = oob_data[nand_ecc_pos[i]];
- eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
- eccsteps = ECCSTEPS;
p = dst;
for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h index 0a0c261..b0dd2f0 100644 --- a/include/configs/am3517_crane.h +++ b/include/configs/am3517_crane.h @@ -359,10 +359,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \
- CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \
- CONFIG_SYS_NAND_ECCSTEPS)
#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h index d44eeec..f797f3f 100644 --- a/include/configs/am3517_evm.h +++ b/include/configs/am3517_evm.h @@ -360,10 +360,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \
- CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \
- CONFIG_SYS_NAND_ECCSTEPS)
#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h index fc7f0cd..6ef6a87 100644 --- a/include/configs/devkit8000.h +++ b/include/configs/devkit8000.h @@ -344,11 +344,6 @@ #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3
-#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \
- CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \
- CONFIG_SYS_NAND_ECCSTEPS)
#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h index 12acb27..65b3b78 100644 --- a/include/configs/hawkboard.h +++ b/include/configs/hawkboard.h @@ -138,11 +138,8 @@ #define CONFIG_SYS_NAND_BAD_BLOCK_POS 0 #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 10 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \
- CONFIG_SYS_NAND_ECCSIZE)
#define CONFIG_SYS_NAND_OOBSIZE 64 -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \
- CONFIG_SYS_NAND_ECCSTEPS)
#endif /* CONFIG_SYS_USE_NAND */
/* diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 970810c..7a240bb 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -426,10 +426,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \
- CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \
- CONFIG_SYS_NAND_ECCSTEPS)
#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index 2ce3959..1fcb7af 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -121,10 +121,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \
- CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \
- CONFIG_SYS_NAND_ECCSTEPS)
#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h index 2f879c0..362fa1d 100644 --- a/include/configs/omap3_evm_quick_nand.h +++ b/include/configs/omap3_evm_quick_nand.h @@ -91,10 +91,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \
- CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \
- CONFIG_SYS_NAND_ECCSTEPS)
#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
-- 1.7.5.4
On 12/13/2011 08:33 PM, Stefano Babic wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babicsbabic@denx.de CC: Ilya Yanokyanok@emcraft.com CC: Scott Woodscottwood@freescale.com CC: Tom Rinitom.rini@gmail.com CC: Simon Schwarzsimonschwarzcor@googlemail.com CC: Wolfgang Denkwd@denx.de
V4:
- Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
- drop parenthesis around defines (Scott Wood)
V3:
- use local defines for CONFIG_SYS_NAND_ECCSTEPS and CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
- drop CONFIG_SYS_NAND_ECCSTEPS from board config files
V2:
- CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
- drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
[SNIP]
Hi Stefano,
on which repo is this patch based? I tried to test it on u-boot but the patch does not apply.
Regards Simon
On 14/12/2011 09:22, Simon Schwarz wrote:
On 12/13/2011 08:33 PM, Stefano Babic wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babicsbabic@denx.de CC: Ilya Yanokyanok@emcraft.com CC: Scott Woodscottwood@freescale.com CC: Tom Rinitom.rini@gmail.com CC: Simon Schwarzsimonschwarzcor@googlemail.com CC: Wolfgang Denkwd@denx.de
V4:
- Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
- drop parenthesis around defines (Scott Wood)
V3:
- use local defines for CONFIG_SYS_NAND_ECCSTEPS and CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
- drop CONFIG_SYS_NAND_ECCSTEPS from board config files
V2:
- CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
- drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
[SNIP]
Hi Stefano,
on which repo is this patch based? I tried to test it on u-boot but the patch does not apply.
Because I thought that the patch will be merged by Tom, I rebased the patch on u-boot-ti. This is also because there is Ilya's patch:
nand_spl_simple: add support for software ECC
that modifies the same file. The patch can be applied flawlessy on top of u-boot-ti, I have checked now again.
For testing I have then applied your SPL Linux patches, and tried to boot linux, as I know there is a conflict with the ECC area.
Tom, it is surely better (I forget to mention) that you merge the patch into u-boot-ti, because you have already merged Ilya's patch.
Regards, Stefano
On 12/13/2011 01:33 PM, Stefano Babic wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de
V4:
- Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
- drop parenthesis around defines (Scott Wood)
V3:
- use local defines for CONFIG_SYS_NAND_ECCSTEPS and CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
- drop CONFIG_SYS_NAND_ECCSTEPS from board config files
V2:
- CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
- drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
drivers/mtd/nand/nand_spl_simple.c | 42 ++++++++++++------------------- include/configs/am3517_crane.h | 4 --- include/configs/am3517_evm.h | 4 --- include/configs/devkit8000.h | 5 ---- include/configs/hawkboard.h | 5 +--- include/configs/omap3_beagle.h | 4 --- include/configs/omap3_evm.h | 4 --- include/configs/omap3_evm_quick_nand.h | 4 --- 8 files changed, 17 insertions(+), 55 deletions(-)
After this patch a hawkboard_nand build gives this:
Configuring for hawkboard_nand - Board: hawkboard, Options: NAND_U_BOOT /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c: In function 'nand_read_page': /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:148:17: error: 'CONFIG_SYS_NAND_ECCSTEPS' undeclared (first use in this function) /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:148:17: note: each undeclared identifier is reported only once for each function it appears in /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:164:18: error: 'CONFIG_SYS_NAND_ECCTOTAL' undeclared (first use in this function) make[1]: *** [/tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.o] Error 1 make: *** [nand_spl] Error 2 make: *** Waiting for unfinished jobs....
Should I drop hawkboard from this patch, or add the change to nand_spl/nand_boot.c? Is a hawkboard conversion to the new SPL pending in some other tree?
-Scott
On Tue, Jan 10, 2012 at 4:01 PM, Scott Wood scottwood@freescale.com wrote:
On 12/13/2011 01:33 PM, Stefano Babic wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de
V4: - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood) - drop parenthesis around defines (Scott Wood)
V3: - use local defines for CONFIG_SYS_NAND_ECCSTEPS and CONFIG_SYS_NAND_ECCTOTAL (Tom Rini) - drop CONFIG_SYS_NAND_ECCSTEPS from board config files
V2: - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok) - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
drivers/mtd/nand/nand_spl_simple.c | 42 ++++++++++++------------------- include/configs/am3517_crane.h | 4 --- include/configs/am3517_evm.h | 4 --- include/configs/devkit8000.h | 5 ---- include/configs/hawkboard.h | 5 +--- include/configs/omap3_beagle.h | 4 --- include/configs/omap3_evm.h | 4 --- include/configs/omap3_evm_quick_nand.h | 4 --- 8 files changed, 17 insertions(+), 55 deletions(-)
After this patch a hawkboard_nand build gives this:
Configuring for hawkboard_nand - Board: hawkboard, Options: NAND_U_BOOT /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c: In function 'nand_read_page': /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:148:17: error: 'CONFIG_SYS_NAND_ECCSTEPS' undeclared (first use in this function) /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:148:17: note: each undeclared identifier is reported only once for each function it appears in /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:164:18: error: 'CONFIG_SYS_NAND_ECCTOTAL' undeclared (first use in this function) make[1]: *** [/tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.o] Error 1 make: *** [nand_spl] Error 2 make: *** Waiting for unfinished jobs....
Should I drop hawkboard from this patch, or add the change to nand_spl/nand_boot.c? Is a hawkboard conversion to the new SPL pending in some other tree?
A hawkboard conversion is pending Albert ack'ing/commenting on a generic ARM fixup.
Hi,
I am slowly catching up on my e-mail, so...
Le 11/01/2012 00:24, Tom Rini a écrit :
A hawkboard conversion is pending Albert ack'ing/commenting on a generic ARM fixup.
Can you point me to this patch series? I'll look it up as soon as possible.
Amicalement,
On Fri, Feb 3, 2012 at 1:17 PM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi,
I am slowly catching up on my e-mail, so...
Le 11/01/2012 00:24, Tom Rini a écrit :
A hawkboard conversion is pending Albert ack'ing/commenting on a generic ARM fixup.
Can you point me to this patch series? I'll look it up as soon as possible.
Latest version starts at http://patchwork.ozlabs.org/patch/139114/ and is in my queue for Monday, assuming there's no further feedback.
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de ---
V5: - no changes, rebased on current u-boot-ti
V4: - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood) - drop parenthesis around defines (Scott Wood)
V3: - use local defines for CONFIG_SYS_NAND_ECCSTEPS and CONFIG_SYS_NAND_ECCTOTAL (Tom Rini) - drop CONFIG_SYS_NAND_ECCSTEPS from board config files
V2: - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok) - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
drivers/mtd/nand/nand_spl_simple.c | 42 ++++++++++++------------------- include/configs/am3517_crane.h | 4 --- include/configs/am3517_evm.h | 4 --- include/configs/devkit8000.h | 5 ---- include/configs/hawkboard.h | 5 +--- include/configs/omap3_beagle.h | 4 --- include/configs/omap3_evm.h | 4 --- include/configs/omap3_evm_quick_nand.h | 4 --- 8 files changed, 17 insertions(+), 55 deletions(-)
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c index 7eb08a3..4a4d02f 100644 --- a/drivers/mtd/nand/nand_spl_simple.c +++ b/drivers/mtd/nand/nand_spl_simple.c @@ -27,6 +27,11 @@ static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS; static nand_info_t mtd; static struct nand_chip nand_chip;
+#define ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ + CONFIG_SYS_NAND_ECCSIZE) +#define ECCTOTAL (ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES) + + #if (CONFIG_SYS_NAND_PAGE_SIZE <= 512) /* * NAND command for small page NAND devices (512) @@ -145,29 +150,21 @@ static int nand_is_bad_block(int block) static int nand_read_page(int block, int page, uchar *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[ECCTOTAL]; + u_char ecc_code[ECCTOTAL]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; - int eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + int eccsteps = ECCSTEPS; uint8_t *p = dst;
- /* - * No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - nand_command(block, page, 0, NAND_CMD_READOOB); this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE); nand_command(block, page, 0, NAND_CMD_READ0);
/* Pick the ECC bytes out of the oob data */ - for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) + for (i = 0; i < ECCTOTAL; i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
@@ -184,24 +181,17 @@ static int nand_read_page(int block, int page, uchar *dst) static int nand_read_page(int block, int page, void *dst) { struct nand_chip *this = mtd.priv; - u_char *ecc_calc; - u_char *ecc_code; - u_char *oob_data; + u_char ecc_calc[ECCTOTAL]; + u_char ecc_code[ECCTOTAL]; + u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; int i; int eccsize = CONFIG_SYS_NAND_ECCSIZE; int eccbytes = CONFIG_SYS_NAND_ECCBYTES; - int eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + int eccsteps = ECCSTEPS; uint8_t *p = dst;
nand_command(block, page, 0, NAND_CMD_READ0);
- /* No malloc available for now, just use some temporary locations - * in SDRAM - */ - ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000); - ecc_code = ecc_calc + 0x100; - oob_data = ecc_calc + 0x200; - for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { if (this->ecc.mode != NAND_ECC_SOFT) this->ecc.hwctl(&mtd, NAND_ECC_READ); @@ -211,10 +201,10 @@ static int nand_read_page(int block, int page, void *dst) this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
/* Pick the ECC bytes out of the oob data */ - for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++) + for (i = 0; i < ECCTOTAL; i++) ecc_code[i] = oob_data[nand_ecc_pos[i]];
- eccsteps = CONFIG_SYS_NAND_ECCSTEPS; + eccsteps = ECCSTEPS; p = dst;
for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h index 0a0c261..b0dd2f0 100644 --- a/include/configs/am3517_crane.h +++ b/include/configs/am3517_crane.h @@ -359,10 +359,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h index d44eeec..f797f3f 100644 --- a/include/configs/am3517_evm.h +++ b/include/configs/am3517_evm.h @@ -360,10 +360,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h index 758326b..2b6a6ee 100644 --- a/include/configs/devkit8000.h +++ b/include/configs/devkit8000.h @@ -343,11 +343,6 @@ #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3
-#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) - #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h index 12acb27..65b3b78 100644 --- a/include/configs/hawkboard.h +++ b/include/configs/hawkboard.h @@ -138,11 +138,8 @@ #define CONFIG_SYS_NAND_BAD_BLOCK_POS 0 #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 10 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) #define CONFIG_SYS_NAND_OOBSIZE 64 -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) + #endif /* CONFIG_SYS_USE_NAND */
/* diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 91af8a0..4c7a686 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -418,10 +418,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h index 2ce3959..1fcb7af 100644 --- a/include/configs/omap3_evm.h +++ b/include/configs/omap3_evm.h @@ -121,10 +121,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h index 2f879c0..362fa1d 100644 --- a/include/configs/omap3_evm_quick_nand.h +++ b/include/configs/omap3_evm_quick_nand.h @@ -91,10 +91,6 @@ 10, 11, 12, 13} #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 -#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ - CONFIG_SYS_NAND_ECCSIZE) -#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * \ - CONFIG_SYS_NAND_ECCSTEPS) #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
On 12/15/2011 03:55 AM, Stefano Babic wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de
Acked-by: Scott Wood scottwood@freescale.com
-Scott
On 12/15/2011 03:55 AM, Stefano Babic wrote:
Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM which is likely to contain already loaded data. The patch saves the oob data and the ecc on the stack replacing the fixed address in RAM.
Signed-off-by: Stefano Babic sbabic@denx.de CC: Ilya Yanok yanok@emcraft.com CC: Scott Wood scottwood@freescale.com CC: Tom Rini tom.rini@gmail.com CC: Simon Schwarz simonschwarzcor@googlemail.com CC: Wolfgang Denk wd@denx.de
Applied to u-boot-nand-flash
-Scott
participants (8)
-
Albert ARIBAUD -
Ilya Yanok -
Scott Wood -
Simon Schwarz -
Stefano Babic -
stefano babic
-
Tom Rini -
Wolfgang Denk