Re: [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file
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.
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
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
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
Hello Heiko,
--On May 06, 2014 16:46 +0200 Heiko Schocher hs@denx.de 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 ?
I guess yes.
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 ?
I had another look at ipam390.h, currently there is
#define CONFIG_SPL_MAX_SIZE 0x20000 #define CONFIG_SPL_MAX_FOOTPRINT 32768
So according to README, the linker checks if the SPL including BSS is smaller than 32kB, and if the SPL excluding BSS is smaller than 128 kB. So the check against CONFIG_SPL_MAX_SIZE is always fulfilled. Therefore it is save to replace CONFIG_SPL_MAX_SIZE with CONFIG_SPL_PAD_TO.
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 ...
Ok, so no change is required for ipam390.
I'll send a patch.
Christian
bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
participants (4)
-
Christian Riesch -
Christian Riesch -
Heiko Schocher -
Tom Taylor