Re: [U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack
On Fri, Feb 3, 2012 at 1:17 PM, Albert ARIBAUD 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. -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack
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, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack
On Tue, Jan 10, 2012 at 4:01 PM, Scott Wood wrote: > On 12/13/2011 01:33 PM, Stefano Babic wrote: >> Currently nand_spl_simple puts it's temp data at 0x1 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 >> CC: Ilya Yanok >> CC: Scott Wood >> CC: Tom Rini >> CC: Simon Schwarz >> CC: Wolfgang Denk >> --- >> 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. -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack
On 12/13/2011 01:33 PM, Stefano Babic wrote: > Currently nand_spl_simple puts it's temp data at 0x1 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 > CC: Ilya Yanok > CC: Scott Wood > CC: Tom Rini > CC: Simon Schwarz > CC: Wolfgang Denk > --- > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack
On 15/12/2011 10:03, Simon Schwarz wrote: > Hi Stefano, > > On 12/14/2011 09:49 AM, Stefano Babic wrote: > [SNIP] >> 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 >> > > hmm. I tried it on u-boot-ti (master and next) and it didn't apply. > > The problem was with drivers/mtd/nand/nand_spl_simple.c:145. The last > commit touching the file was from Anatolij on 3th december. I see, maybe it was not already merged when I tested. I send a rebased patch. Stefano -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack
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 0x1 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 >> CC: Ilya Yanok >> CC: Scott Wood >> CC: Tom Rini >> CC: Simon Schwarz >> CC: Wolfgang Denk >> --- >> 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 -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack
On 12/13/2011 08:33 PM, Stefano Babic wrote: Currently nand_spl_simple puts it's temp data at 0x1 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 CC: Ilya Yanok CC: Scott Wood CC: Tom Rini CC: Simon Schwarz CC: Wolfgang Denk --- 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack
On Tue, Dec 13, 2011 at 12:33 PM, Stefano Babic wrote:
> Currently nand_spl_simple puts it's temp data at 0x1 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
> CC: Ilya Yanok
> CC: Scott Wood
> CC: Tom Rini
> CC: Simon Schwarz
> CC: Wolfgang Denk
Acked-by: Tom Rini
> ---
> 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 + 0x1);
> - 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 + 0x1);
> - 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,
