[U-Boot] [PATCH V4 2/5] omap-common: add nand spl support

[Simon Schwarz]

Dear Scott Wood,

On 07/27/2011 11:38 PM, Scott Wood wrote:
> On Wed, 27 Jul 2011 10:42:22 +0200
> Simon Schwarz<simonschwarzcor at googlemail.com>  wrote:
>
>> Dear Scott Wood,
>>
>> On 07/26/2011 08:06 PM, Scott Wood wrote:
>>> On Tue, 26 Jul 2011 14:09:15 +0200
>>> Simon Schwarz<simonschwarzcor at googlemail.com>   wrote:
>>>
>>>> +#ifdef CONFIG_SPL_NAND_SUPPORT
>>>> +static void nand_load_image(void)
>>>> +{
>>>> +	gpmc_init();
>>>> +	nand_init();
>>>> +	nand_copy_image(CONFIG_SYS_NAND_U_BOOT_OFFS,
>>>> +		CONFIG_SYS_NAND_U_BOOT_SIZE,
>>>> +		(uchar *)CONFIG_SYS_NAND_U_BOOT_DST);
>>>> +#ifdef CONFIG_NAND_ENV_DST
>>>> +	nand_copy_image(CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE,
>>>> +		(uchar *)CONFIG_NAND_ENV_DST);
>>>> +#ifdef CONFIG_ENV_OFFSET_REDUND
>>>> +	nand_copy_image(CONFIG_ENV_OFFSET_REDUND, CONFIG_ENV_SIZE,
>>>> +		(uchar *)CONFIG_NAND_ENV_DST + CONFIG_ENV_SIZE);
>>>> +#endif
>>>> +#endif
>>>> +	parse_image_header((struct image_header *)CONFIG_SYS_NAND_U_BOOT_DST);
>>>> +}
>>>> +#endif /* CONFIG_SPL_NAND_SUPPORT */
>>>
>>> I'm not sure that "load" versus "copy" conveys the difference between this
>>> function and the low-level nand_copy_image.
>>
>> The actual difference is that nand_load has an mtd_info struct as
>> additional paramter.
>
> Hmm?  nand_load_image() takes no arguments.  I don't see a nand_load().
>
> The actual difference is that one is a low-level "move this from here to
> there" function, and the other is driving hardware init and then performing
> a series of calls to the low-level function, supplying the information
> about what is to be loaded where.

We have different code - sorry for the confusion. see below.

>
>> The device to use is selected in nand_init and I
>> don't see a reason why this should be passed around in the interface -
>> in the spl all data is typically loaded from one chip - this also was
>> the implementation before.
>
> Sure.
>
>>> Where is nand_copy_image() defined?
>> It's in drivers/mtd/nand/nand_spl.c
>
> Where is drivers/mtd/nand/nand_spl.c?  It's not in Wolfgang's
> current tree, nor in u-boot-ti, and I didn't see it in these patches.  Did
> you forget to "git add"?
arrghh. You are right. I forgot a git add. V6 will change this...
>
> Note that there will not be one implementation of nand_copy_image suitable
> for all hardware, just as currently nand_spl/nand_boot.c is not used for
> all NAND SPL targets.

Hm. I know that. I just adapated the old nand_boot.c.

AFAIK the other implementations use prefixes for the function names - 
therefore we can just add them to the nand-spl-library and gcc will do 
the rest.

Regards & thx for the review!
Simon