[U-Boot] DA850EVM with USE_NAND config does not pad the AIS file

[Tom Taylor <ttaylor.tampa@gmail.com>]

Hello Heiko,

On 5/6/2014 10:46 AM, Heiko Schocher wrote:
> Hello Christian,
>
> Am 06.05.2014 13:30, schrieb Christian Riesch:
>> Tom,
>> Thank you very much for your investigations :-)
>>
>> --On April 26, 2014 13:34 -0400 Tom Taylor <ttaylor.tampa@gmail.com> 
>> wrote:
>>
>>> I'm a U-Boot newbie so please feel free to correct how I'm reporting 
>>> this
>>> issue..
>>>
>>> I recently downloaded the 2014.04-rc3 snapshot to build U-Boot for my
>>> custom DA850-based board. The only change was to add a new target
>>> "dav850evm_nand" in boards.cfg with the added parameter "USE_NAND".
>>>
>>> The resulting AIS file was programmed into EVM-compatible NAND using
>>> standard sfh_OMAP-L138 method.
>>>
>>> The board failed to boot, and stayed in a loop printing the SPL console
>>> message repeatedly.
>>>
>>> After some debugging with CCS 5.5 and an XDS100v2, I found that 
>>> incorrect
>>> code was being loaded into the 0xc108000 RAM destination. The 
>>> da850evm.h
>>> file defines CONFIG_SYS_NAND_U_BOOT_OFFS as 0x28000, which 
>>> corresponds to
>>> an AIS offset of 0x8000 but the u-boot header did not appear there 
>>> in the
>>> AIS file. A search revealed that the Makefile catenated u-boot
>>> immediately after the SPL without any padding.
>>>
>>> Further investigation revealed that the target Makefile needs
>>> CONFIG_SPL_MAX_SIZE to be defined as 0x8000 in order for the padding to
>>> be performed properly; however, this constant was apparently deleted
>>> during a series of changes in April, 2013 to accommodate separate code
>>> and BSS size limits for another target. In its place,
>>> CONFIG_SPL_MAX_FOOTPRINT was defined as 32768. Unfortunately, the
>>> da850evm Makefile does not refer to this constant.
>>>
>>> To solve the problem, I added the following 2 lines in my 
>>> custom-modified
>>> da850evm.h:
>>> # define CONFIG_SPL_PAD_TO 0x8000
>>> # define CONFIG_SPL_MAX_SIZE 0x8000
>>>
>>> although the first line may not be strictly required.
>>
>> Yes, CONFIG_SPL_PAD_TO is currently not used for the 'make 
>> u-boot.ais' target in the Makefile. Instead, the Makefile uses 
>> CONFIG_SPL_MAX_SIZE for padding the SPL, which is probably wrong.
>
> Yes, CONFIG_SPL_PAD_TO is the correct define. On the other hand
> the question is is CONFIG_SPL_PAD_TO not always equal to
> CONFIG_SPL_MAX_SIZE ?
>
>>> This solved the
>>> problem and allowed the board to boot.
>>>
>>> Doesn't this mean that other similar targets may be broken?
>>
>> I think yes.
>>
>> I think the right fix would be to change the Makefile to use 
>> CONFIG_SPL_PAD_TO instead of CONFIG_SPL_MAX_SIZE for the u-boot.ais 
>> target.
>>
>> diff --git a/Makefile b/Makefile
>> index ff38a43..869f442 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -890,7 +890,7 @@ MKIMAGEFLAGS_u-boot-spl.ais = -s -n $(if 
>> $(CONFIG_AIS_CONFIG_FILE), \
>> spl/u-boot-spl.ais: spl/u-boot-spl.bin FORCE
>> $(call if_changed,mkimage)
>>
>> -OBJCOPYFLAGS_u-boot.ais = -I binary -O binary 
>> --pad-to=$(CONFIG_SPL_MAX_SIZE)
>> +OBJCOPYFLAGS_u-boot.ais = -I binary -O binary 
>> --pad-to=$(CONFIG_SPL_PAD_TO)
>> u-boot.ais: spl/u-boot-spl.ais u-boot.img FORCE
>> $(call if_changed,pad_cat)
>>
>>
>> And then check all ARM926EJS/Davinci configurations that use SPL:
>>
>> (extending Tom Rini's grep command from his email)
>>
>> $ git grep -l ARM926EJS include/configs/ | xargs grep -l DAVINCI | 
>> xargs grep -l _SPL_
>> include/configs/cam_enc_4xx.h
>> include/configs/da830evm.h
>> include/configs/da850evm.h
>> include/configs/hawkboard.h
>> include/configs/ipam390.h
>>
>> For the cam_enc_4xx CONFIG_SPL_PAD_TO is already defined, so it 
>> should work fine after fixing the Makefile. Heiko, any comments on 
>> this? Are you actually using the u-boot.ais target?
>
> I have no board to test, but I think it is used.
>
>> da830evm and hawkboard did not use CONFIG_SPL_MAX_SIZE, so no need to 
>> fix them.
>>
>> da850evm: We should add CONFIG_SPL_PAD_TO as already suggested by you.
>
>>
>> ipam390.h: I think the #define CONFIG_SPL_MAX_SIZE 0x20000 should be 
>> removed or replaced by #define CONFIG_SPL_PAD_TO 0x20000. But 
>> actually the board has been added after the commits that replace grep 
>> -lr CONFIG_SPL_MAX_SIZE by CONFIG_SPL_MAX_FOOTPRINT, so why
>> didn't that issue come up when adding the board to mainline? Heiko, 
>> any comments? Are you using make u-boot.ais here or something else?
>
> I am not sure, if we can just remove CONFIG_SPL_MAX_SIZE for this
> board, as it maybe has only 0x20000 space for the SPL ?
>
> maybe:
>
> #if !defined(CONFIG_SPL_PAD_TO)
> define CONFIG_SPL_PAD_TO CONFIG_SPL_MAX_SIZE
> #endif
>
> is better? Heh, thats the case, see:
>
> ./include/config_fallbacks.h
>
> so, your Makefile patch should be Ok ...
>
> bye,
> Heiko
There's currently only 0x8000 space allocated for the SPL.  The first 
0x20000 byte block is allocated for the NAND-resident environment.  From 
da850evm.h:

#ifdef CONFIG_USE_NAND
   :
#define CONFIG_ENV_OFFSET        0x0 /* Block 0--not used by bootcode */
#define CONFIG_ENV_SIZE            (128 << 10)

Anyone customizing the build has to of course ensure that that the boot 
offset passed to nand_spl_load_image() is consistent .  Again from 
da850evm.h:

#define CONFIG_SYS_NAND_U_BOOT_OFFS    0x28000

Because of include/config_fallbacks, it would seem like Christian's 
suggestion to change the Makefile to use CONFIG_SPL_PAD_TO would give 
the most flexibility and leave CONFIG_SPL_MAX_SIZE as an optional sanity 
check.

Y'all are much more familiar with U-Boot innards and patch procedures 
than I am.  I'm happy to have helped find the problem, and whatever you 
recommend to do will be fine with me.  I can test the da850 target 
fairly well using my custom board that has a LogicPD SOM-M1 on a 
baseboard that includes a da850evm-compatible NAND.

Tom Taylor