* [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file
@ 2014-05-06 11:30 Christian Riesch
2014-05-06 14:46 ` Heiko Schocher
0 siblings, 1 reply; 7+ messages in thread
From: Christian Riesch @ 2014-05-06 11:30 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file
2014-05-06 11:30 [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file Christian Riesch
@ 2014-05-06 14:46 ` Heiko Schocher
2014-05-06 16:52 ` Tom Taylor
2014-05-06 18:41 ` Christian Riesch
0 siblings, 2 replies; 7+ messages in thread
From: Heiko Schocher @ 2014-05-06 14:46 UTC (permalink / raw)
To: u-boot
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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file
2014-05-06 14:46 ` Heiko Schocher
@ 2014-05-06 16:52 ` Tom Taylor
2014-05-06 18:41 ` Christian Riesch
1 sibling, 0 replies; 7+ messages in thread
From: Tom Taylor @ 2014-05-06 16:52 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file
2014-05-06 14:46 ` Heiko Schocher
2014-05-06 16:52 ` Tom Taylor
@ 2014-05-06 18:41 ` Christian Riesch
1 sibling, 0 replies; 7+ messages in thread
From: Christian Riesch @ 2014-05-06 18:41 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file
@ 2014-05-06 8:07 Christian Riesch
0 siblings, 0 replies; 7+ messages in thread
From: Christian Riesch @ 2014-05-06 8:07 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v4 00/10] ARMv7: add PSCI support to U-Boot
@ 2014-04-26 12:17 Marc Zyngier
2014-04-26 12:17 ` [U-Boot] [PATCH v4 10/10] ARM: HYP/non-sec/PSCI: emit DT nodes Marc Zyngier
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2014-04-26 12:17 UTC (permalink / raw)
To: u-boot
PSCI is an ARM standard that provides a generic interface that
supervisory software can use to manage power in the following
situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware,
and rely on common kernel side code that just calls into PSCI.
More importantly, it gives a way to ensure that CPUs enter the kernel
at the appropriate exception level (ie HYP mode, to allow the use of
the virtualization extensions), even across events like CPUs being
powered off/on or suspended.
The main idea here is to turn some of the existing U-Boot code into a
separate section that can live in secure RAM (or a reserved page of
memory), containing a secure monitor that will implement the PSCI
operations. This code will still be alive when U-Boot is long gone,
hence the need for a piece of memory that will not be touched by the
OS.
This patch series contains 3 parts:
- the first four patches are just bug fixes
- the next two refactor the HYP/non-secure code to allow relocation
in secure memory
- the last four contain the generic PSCI code and DT infrastructure
This implements the original 0.1 spec, as nobody implements the new
0.2 version so far. I plan to update this support to 0.2 once there is
an official binding available (and support in the kernel).
Most of the development has been done on an Allwinner A20 SoC, which
is the main user of this code at the moment. I hope new SoCs will be
using this method in the future (my primary goal for this series being
to avoid more stupid SMP code from creeping up in the Linux
kernel). As instructed, I've removed the A20 support code and made it
a separate series, as there is now an effort to mainline this code
(see Ian Campbell patch series).
With these three series applied, the A20 now boots in HYP mode, Linux
finds the secondary CPU without any SMP code present in the kernel,
and runs KVM out of the box. The Xen/ARM guys managed to do the same
fairly easily, as did at least one XVizor user.
This code has also been tested on a VExpress TC2, running KVM with all
5 CPUs, in order to make sure there was no obvious regression.
The code is also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/u-boot.git wip/psci-v4
A fully merged branch with the A20 support is in the wip/psci-v4-a20
branch of the same repo.
Cheers,
M.
^ permalink raw reply [flat|nested] 7+ messages in thread* [U-Boot] [PATCH v4 10/10] ARM: HYP/non-sec/PSCI: emit DT nodes
2014-04-26 12:17 [U-Boot] [PATCH v4 00/10] ARMv7: add PSCI support to U-Boot Marc Zyngier
@ 2014-04-26 12:17 ` Marc Zyngier
2014-04-26 17:34 ` [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file Tom Taylor
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2014-04-26 12:17 UTC (permalink / raw)
To: u-boot
Generate the PSCI node in the device tree.
Also add a reserve section for the "secure" code that lives in
in normal RAM, so that the kernel knows it'd better not trip on
it.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/cpu/armv7/Makefile | 1 +
arch/arm/cpu/armv7/virt-dt.c | 100 +++++++++++++++++++++++++++++++++++++++++++
arch/arm/include/asm/armv7.h | 1 +
arch/arm/lib/bootm-fdt.c | 12 +++++-
4 files changed, 112 insertions(+), 2 deletions(-)
create mode 100644 arch/arm/cpu/armv7/virt-dt.c
diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
index de1cc1a..44329cd 100644
--- a/arch/arm/cpu/armv7/Makefile
+++ b/arch/arm/cpu/armv7/Makefile
@@ -21,6 +21,7 @@ endif
ifneq ($(CONFIG_ARMV7_NONSEC)$(CONFIG_ARMV7_VIRT),)
obj-y += nonsec_virt.o
obj-y += virt-v7.o
+obj-y += virt-dt.o
endif
ifneq ($(CONFIG_ARMV7_PSCI),)
diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c
new file mode 100644
index 0000000..0b0d6a7
--- /dev/null
+++ b/arch/arm/cpu/armv7/virt-dt.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright (C) 2013 - ARM Ltd
+ * Author: Marc Zyngier <marc.zyngier@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <common.h>
+#include <stdio_dev.h>
+#include <linux/ctype.h>
+#include <linux/types.h>
+#include <asm/global_data.h>
+#include <libfdt.h>
+#include <fdt_support.h>
+#include <asm/armv7.h>
+#include <asm/psci.h>
+
+static int fdt_psci(void *fdt)
+{
+#ifdef CONFIG_ARMV7_PSCI
+ int nodeoff;
+ int tmp;
+
+ nodeoff = fdt_path_offset(fdt, "/cpus");
+ if (nodeoff < 0) {
+ printf("couldn't find /cpus\n");
+ return nodeoff;
+ }
+
+ /* add 'enable-method = "psci"' to each cpu node */
+ for (tmp = fdt_first_subnode(fdt, nodeoff);
+ tmp >= 0;
+ tmp = fdt_next_subnode(fdt, tmp)) {
+ const struct fdt_property *prop;
+ int len;
+
+ prop = fdt_get_property(fdt, tmp, "device_type", &len);
+ if (!prop)
+ continue;
+ if (len < 4)
+ continue;
+ if (strcmp(prop->data, "cpu"))
+ continue;
+
+ fdt_setprop_string(fdt, tmp, "enable-method", "psci");
+ }
+
+ nodeoff = fdt_path_offset(fdt, "/psci");
+ if (nodeoff < 0) {
+ nodeoff = fdt_path_offset(fdt, "/");
+ if (nodeoff < 0)
+ return nodeoff;
+
+ nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
+ if (nodeoff < 0)
+ return nodeoff;
+ }
+
+ tmp = fdt_setprop_string(fdt, nodeoff, "compatible", "arm,psci");
+ if (tmp)
+ return tmp;
+ tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
+ if (tmp)
+ return tmp;
+ tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_suspend", ARM_PSCI_FN_CPU_SUSPEND);
+ if (tmp)
+ return tmp;
+ tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_off", ARM_PSCI_FN_CPU_OFF);
+ if (tmp)
+ return tmp;
+ tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_on", ARM_PSCI_FN_CPU_ON);
+ if (tmp)
+ return tmp;
+ tmp = fdt_setprop_u32(fdt, nodeoff, "migrate", ARM_PSCI_FN_MIGRATE);
+ if (tmp)
+ return tmp;
+#endif
+ return 0;
+}
+
+int armv7_update_dt(void *fdt)
+{
+#ifndef CONFIG_ARMV7_SECURE_BASE
+ /* secure code lives in RAM, keep it alive */
+ fdt_add_mem_rsv(fdt, (unsigned long)__secure_start,
+ __secure_end - __secure_start);
+#endif
+
+ return fdt_psci(fdt);
+}
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index 11476dd..323f282 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -79,6 +79,7 @@ void v7_outer_cache_inval_range(u32 start, u32 end);
#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
int armv7_init_nonsec(void);
+int armv7_update_dt(void *fdt);
/* defined in assembly file */
unsigned int _nonsec_init(void);
diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c
index 8394e15..d4f1578 100644
--- a/arch/arm/lib/bootm-fdt.c
+++ b/arch/arm/lib/bootm-fdt.c
@@ -17,13 +17,14 @@
#include <common.h>
#include <fdt_support.h>
+#include <asm/armv7.h>
DECLARE_GLOBAL_DATA_PTR;
int arch_fixup_fdt(void *blob)
{
bd_t *bd = gd->bd;
- int bank;
+ int bank, ret;
u64 start[CONFIG_NR_DRAM_BANKS];
u64 size[CONFIG_NR_DRAM_BANKS];
@@ -32,5 +33,12 @@ int arch_fixup_fdt(void *blob)
size[bank] = bd->bi_dram[bank].size;
}
- return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
+ ret = fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+ if (ret)
+ return ret;
+
+ ret = armv7_update_dt(blob);
+#endif
+ return ret;
}
--
1.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file
2014-04-26 12:17 ` [U-Boot] [PATCH v4 10/10] ARM: HYP/non-sec/PSCI: emit DT nodes Marc Zyngier
@ 2014-04-26 17:34 ` Tom Taylor
2014-05-05 13:09 ` Tom Rini
0 siblings, 1 reply; 7+ messages in thread
From: Tom Taylor @ 2014-04-26 17:34 UTC (permalink / raw)
To: u-boot
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. This solved the
problem and allowed the board to boot.
Doesn't this mean that other similar targets may be broken?
How can I assist with contributing the patch to fix this? One problem I
have is that I only have an EXP, not an EVM board. My custom target
board shares some features with the EVM such as NOR and NAND memory, but
other things are different like the use of a fixed PHY. This makes it
impossible for me to completely verify a da850evm build.
I would like to see more target configurations available for the
DA850EVM but it seems like development for this has stopped. NAND boot
was a simple change, but adding USB support required me to copy & paste
code from the da830evm target. Are there any future plans to do this,
or will I need to do this development for my custom board only?
Tom Taylor
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file
2014-04-26 17:34 ` [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file Tom Taylor
@ 2014-05-05 13:09 ` Tom Rini
0 siblings, 0 replies; 7+ messages in thread
From: Tom Rini @ 2014-05-05 13:09 UTC (permalink / raw)
To: u-boot
On Sat, Apr 26, 2014 at 01:34:58PM -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
Thanks for looking into this. I saw a similar issue on my am18xx EVM
but was 5 bugs deep and didn't have time to bisect it then.
I think the answer here is to post a patch fixing the boards listed in:
$ git grep -l ARM926EJS include/configs/ | xargs grep -l DAVINCI
As they all share that set of constraints.
Thanks for digging into this!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140505/1063bf1b/attachment.pgp>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-05-06 18:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-06 11:30 [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file Christian Riesch
2014-05-06 14:46 ` Heiko Schocher
2014-05-06 16:52 ` Tom Taylor
2014-05-06 18:41 ` Christian Riesch
-- strict thread matches above, loose matches on Subject: below --
2014-05-06 8:07 Christian Riesch
2014-04-26 12:17 [U-Boot] [PATCH v4 00/10] ARMv7: add PSCI support to U-Boot Marc Zyngier
2014-04-26 12:17 ` [U-Boot] [PATCH v4 10/10] ARM: HYP/non-sec/PSCI: emit DT nodes Marc Zyngier
2014-04-26 17:34 ` [U-Boot] DA850EVM with USE_NAND config does not pad the AIS file Tom Taylor
2014-05-05 13:09 ` Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox