[U-Boot] [RFC 4/5] SPL:Defines function required to env read for IFC & env_nand

[Scott Wood]

On Wed, 2013-09-25 at 09:47 +0530, Prabhakar Kushwaha wrote:
> On 09/23/2013 11:49 PM, Scott Wood wrote:
> > On Wed, 2013-09-18 at 16:40 +0530, Prabhakar Kushwaha wrote:
> >> Thanks Scott for taking time and reviewing the RFC patch.
> >>
> >> Please find my reply in-lined.
> >>
> >> On 09/17/2013 05:23 AM, Scott Wood wrote:
> >>> On Mon, 2013-09-16 at 21:35 +0530, Prabhakar Kushwaha wrote:
> >>>> fsl_ifs_spl.c reads data from NAND and store at a memory location in raw mode.
> >>>> It does not used MTD layer.
> >>>> To read env variable from NAND MTD layer read/write required.
> >>>>
> >>>> Hence, add mtd_block_isbad & nand_read_skip_bad function required during
> >>>> env variable read.
> >>>>
> >>>> Also, avoid nand_info during env read for SPL
> >>>>
> >>>> Signed-off-by: Prabhakar Kushwaha <prabhakar at freescale.com>
> >>>> ---
> >>>>    common/env_nand.c              |    7 ++++---
> >>>>    drivers/mtd/nand/Makefile      |    2 +-
> >>>>    drivers/mtd/nand/fsl_ifc_spl.c |   22 ++++++++++++++++++++++
> >>>>    3 files changed, 27 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/common/env_nand.c b/common/env_nand.c
> >>>> index 7530962..7a7107f 100644
> >>>> --- a/common/env_nand.c
> >>>> +++ b/common/env_nand.c
> >>>> @@ -246,11 +246,13 @@ int readenv(size_t offset, u_char *buf)
> >>>>    	u_char *char_ptr;
> >>>>    
> >>>>    	blocksize = nand_info[0].erasesize;
> >>>> +#ifndef CONFIG_SPL_BUILD
> >>>>    	if (!blocksize)
> >>>>    		return 1;
> >>>> -
> >>>>    	len = min(blocksize, CONFIG_ENV_SIZE);
> >>>> -
> >>>> +#else
> >>>> +	len = CONFIG_ENV_SIZE;
> >>>> +#endif
> >>> Use positive logic (ifdef/else, not ifndef/else).
> >> I will fix it.
> >>
> >>> Are you sure that CONFIG_ENV_SIZE will always be appropriate?  Shouldn't
> >>> you use CONFIG_SYS_NAND_BLOCK_SIZE in place of nand_info[0].erasesize?
> >> I can use CONFIG_SYS_NAND_BLOCK_SIZE . but i can not use in SPL as its
> >> is defined as 128K.
> > You can't redefine the block size to suit your needs.  It's a property
> > of the hardware.  What difference does it make, as long as
> > CONFIG_ENV_SIZE is smaller?
> Earlier code was like
>   len = min(blocksize, CONFIG_ENV_SIZE) as SPL never defines nand_info. 
> So the block size is 0.
> causing len = 0.
> 
> I am avoiding it by setting  = CONFIG_ENV_SIZE. No redefinition of block 
> size.

This is generic code.  It's possible (even if a bit unlikely) that some
board has an environment that is larger than the block size.  That's why
the loop exists in the first place (though I'm not sure why it can't be
replaced with a single call to nand_read_skip_bad).

Since your problem is with how you determine the block size, just
replace the source of the block size but don't touch the min().

-Scott