[U-Boot] arch/arm/lib/board.c - uninitialized vars
Heiko Schocher
hs at denx.de
Thu Nov 6 13:10:33 CET 2014
Hello Wolfgang,
Am 06.11.2014 12:28, schrieb Wolfgang Denk:
> Hi,
>
> I'm trying to clean up some warnings/errors detected when running
> "cppcheck" on the U-Boot source tree. For arch/arm/lib/board.c
> I get this:
>
> [arch/arm/lib/board.c:445]: (error) Uninitialized variable: id
> [arch/arm/lib/board.c:422]: (error) Uninitialized variable: addr_sp
>
> The problem is not usually detected by GCC depending on which macros
> are active, here especially CONFIG_SPL_BUILD
>
> The relevant code was last touched / introduced by commit f1d2b313:
> "ARM: add relocation support" some two years ago...
>
> I have some questions regarding the CONFIG_SPL_BUILD "else" case
> (i. e. when building with CONFIG_SPL_BUILD defined):
>
> 422 addr_sp += 128; /* leave 32 words for abort-stack */
>
> Is this correct? The stack is growing downward, so should the '+' not
> be replaced by a '-', like we do a few lines above:
No, this is a typo ... it must be a "-"
>
> 412 /* leave 3 words for abort-stack */
> 413 addr_sp -= 12;
>
> Why do we need 128 words in the CONFIG_SPL_BUILD case, but only 3
> otherwise?
Good question ...
> Should we not move the "alignment for ABI compliance" part outside the
> CONFIG_SPL_BUILD if/else case, i. e. should this not always be done?
Yes, I think thats correct.
But looking into common/board_f.c ... there it is also done only
for the not SPL case ...
going back in history ...
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/arm926ejs/start.S;h=cf40ce12928359c5238d02a32ed361ccc153c101;hb=a59e27997637a2395ae2cc7f809127f24119a167
here I see:
leave 32 words for abort-stack
leave 3 words for abort-stack
8-byte alignment for ABI compliance
without SPL (former PRELOADER) define mess ...
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/arm1136/start.S;h=41eb82dae246b9509545f051daac605a02b7db05;hb=a59e27997637a2395ae2cc7f809127f24119a167#l179
here:
#ifdef CONFIG_PRELOADER
leave 32 words for abort-stack
#else
leave 3 words for abort-stack
#endif
8-byte alignment for ABI compliance
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/pxa/start.S;h=e07c8c2e0e70744c45152c1649cef65f87825708;hb=a59e27997637a2395ae2cc7f809127f24119a167
here
leave 3 words for abort-stack
8-byte alignment for ABI compliance
no "leave 32 words for abort-stack"
... :-(
> And of course, how should we correctly initialize the "id" and
> "addr_sp" variable in both cases?
Hmm.. they are initialized for the !SPL case ... and in the
SPL case, board__init_f is used from ./arch/arm/lib/spl.c
or overwritten from SoC dependend code, like done in:
./arch/arm/cpu/arm926ejs/davinci/spl.c
-> so I think, we can drop the CONFIG_SPL_BUILD in
u-boot:/arch/arm/lib/board.c board_init_f(), as it is not used, thats
maybe the reason, why this issue never poped up!
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
More information about the U-Boot
mailing list