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

[Scott Wood]

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?

> >> +#ifndef CONFIG_SPL_INIT_MINIMAL
> >> +struct mtd_info nand_info[CONFIG_SYS_MAX_NAND_DEVICE];
> >> +
> >> +int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
> >> +{
> >> +	return 0;
> >> +}
> >> +
> >> +int nand_read_skip_bad(struct mtd_info *nand, loff_t offset, size_t *length,
> >> +		size_t *actual, loff_t lim, u_char *buffer)
> >> +{
> >> +	nand_load(offset, *length, buffer);
> >> +	return 0;
> >> +}
> >> +#endif
> > What does this have to do with minimal init?
> This has nothing to do with minimal init.

Then what does it have to do with CONFIG_SPL_INIT_MINIMAL, which is
specifically for configuring minimial init?

> These function will comes into during CONFIG_SPL_BUILD and !defined 
> CONFIG_SPL_INIT_MINIMAL.
>
> These function will be used for reading env variables.

Then use some SPL symbol indicating environment support is needed.

-Scott