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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
