[U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3

[Stefan Roese <sr@denx.de>]

Hi Albert,

On 20.06.2013 19:51, Albert ARIBAUD wrote:
>>> The correct fix (read: the one I won't NAK) is thus to add a #else
>>> clause in the code above, in which r8 will be set to =gdata, and to
>>> remove the corresponding assignments in the various places where they
>>> reside.
>>
>> Here's the problem. Setting r8 in _main is too late. As it has already
>> been used (in the current implementation) to store some data (e.g.
>> clocks for baudrate generation etc). Here the code from
>> arch/arm/cpu/armv7/omap3/board.c:s_init():
>>
>> #ifdef CONFIG_SPL_BUILD
>> 	gd = &gdata;
>>
>> 	preloader_console_init();
>>
>> 	timer_init();
>> #endif
>>
>> Note that this is done *before* _main() is called (we are talking about
>> SPL for OMAP here).
> 
> Yes, it is done before _main... right before it. Like, really *right*
> *before* *it*. Like, s_init is called at the very end of lowlevel_init,
> which was branched straight into from cpu_init_crit which itself is
> called just before _main.
> 
> In other words, after s_init(), _main immediately kicks in, sets up sp
> and r8 (which was done also in lowlevel_init and will thus be a no-op
> once gdata is handled in crt0.S too) and calls board_init_f().
> 
> So, calling s_init() last in lowlevel_init() would be the same as
> calling it first in board_init_f().
>
> The difference with the current situation is, s_init() is C code, and C
> runtime is supposed not to be available until just before entering
> board_init_f(). This is the only reason why sp and r8 are set up in
> lowlevel_init: because s_init() is called at a time when C runtime is
> not officially set up yet.
> 
> Note that as far as setting the C runtime environment is concerned,
> crt0.S does *exactly* the same thing as lowlevel_init -- or will, once
> the gdata stuff is added in crt0.S as I suggest.
> 
> --- 8< ---
> So you just need to add the gdata stuff in crt0.S as I previously
> suggested, move the s_init() call in crt0.S (conditioned on building
> SPL) just before the call to board_init_f(), and then make
> lowlevel_init empty since it would then be useless.
> --- 8< ---

Useless? Its still calling cpy_clk_code(). Isn't this needed anymore?

>> And it did cost me quite some time to find this
>> problem, that r8 was re-configured in _main() and all the already set
>> values disappeared again (no serial output etc).
> 
> Actually your trouble precisely shows why gd/r8 should only be touched
> in a single file and nowhere else: because it is set in many places,
> its value was hard to control.

Full ack on this.

>> Yes, this needs some cleanup/fixup. Unfortunately I won't find the time
>> to look into such a cleanup in the next days. Perhaps somebody else
>> might jump in...
> 
> There'll have to be a V2 for this patch anyway.
> 
> Here's my offer: you submit V2 of this patch as described above between
> the '--- 8< ---' markers, and I handle the scrubbing of all spurious
> assignments to gd myself. Deal?

Yes, I'll try to squeeze a few cycles from the other projects to get
this done hopefully tomorrow.

Thanks,
Stefan