From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Tue, 06 May 2014 16:46:41 +0200 Subject: [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file In-Reply-To: <1175F677D729FCC42037B0B8@[172.22.2.41]> References: <1175F677D729FCC42037B0B8@[172.22.2.41]> Message-ID: <5368F5D1.4080802@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany