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

[Prabhakar Kushwaha]

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.
>> @@ -396,7 +398,6 @@ void env_relocate_spec(void)
>>   		return;
>>   	}
>>   #endif
>> -
>>   	ret = readenv(CONFIG_ENV_OFFSET, (u_char *)buf);
>>   	if (ret) {
>>   		set_default_env("!readenv() failed");
> Remove unrelated whitespace changes.

Sure

>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>> index 366dee6..06d5d14 100644
>> --- a/drivers/mtd/nand/Makefile
>> +++ b/drivers/mtd/nand/Makefile
>> @@ -24,6 +24,7 @@ COBJS-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o
>>   COBJS-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o
>>   COBJS-$(CONFIG_SPL_NAND_BASE) += nand_base.o
>>   COBJS-$(CONFIG_SPL_NAND_INIT) += nand.o
>> +COBJS-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_spl.o
> No, it's still a minimal NAND driver (i.e. it doesn't use fsl_ifc.o).
> Minimal NAND drivers are not related to minimal SPL init.

I will take care of it

>> diff --git a/drivers/mtd/nand/fsl_ifc_spl.c b/drivers/mtd/nand/fsl_ifc_spl.c
>> index d462265..e7edacf 100644
>> --- a/drivers/mtd/nand/fsl_ifc_spl.c
>> +++ b/drivers/mtd/nand/fsl_ifc_spl.c
>> @@ -11,6 +11,28 @@
>>   #include <asm/io.h>
>>   #include <asm/fsl_ifc.h>
>>   #include <linux/mtd/nand.h>
>> +#ifndef CONFIG_SPL_INIT_MINIMAL
>> +#include <linux/mtd/mtd.h>
>> +#endif
>> +
>> +static void nand_load(unsigned int offs, int uboot_size, uchar *dst);
>> +
>> +#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.
These function will comes into during CONFIG_SPL_BUILD and !defined 
CONFIG_SPL_INIT_MINIMAL.

These function will be used for reading env variables.

Regards,
Prabhakar