[U-Boot] 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(-)

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