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

[Christian Riesch]

Tom,
Thank you very much for your investigations :-)

--On April 26, 2014 13:34 -0400 Tom Taylor <ttaylor.tampa at 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.

> 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?

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 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?

Christian