From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Taylor Date: Tue, 06 May 2014 12:52:40 -0400 Subject: [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file In-Reply-To: <5368F5D1.4080802@denx.de> References: <1175F677D729FCC42037B0B8@[172.22.2.41]> <5368F5D1.4080802@denx.de> Message-ID: <53691358.10606@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> 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