[U-Boot] [RFC] Removal of superfluous gd assignments

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hello Stefan,

On Thu, 26 Nov 2015 07:23:51 +0100, Stefan Roese
<stefan.roese@gmail.com> wrote:
> Hi Albert,
> 
> first, thank you very much for taking the time to go over this
> in such depth.
> 
> On 25.11.2015 16:20, Albert ARIBAUD wrote:
> > Hello all,
> >
> > This is a follow-up to discussions on the IRC chan re: the fact that gd
> > (the global data pointer) is being assigned in various places, which is
> > not too much of a good thing.
> >
> > In all architectures, common/init/board_init.c is built in, which
> > provides board_init_f_{mem,init_reserve}, in which gd is assigned by
> > calling arch_setup_gd().
> >
> > (two architectures, ARM and x86, cannot assign gd from C code, so they
> > don't call arch_setup_gd() but assign GD from another single location)
> >
> > However, there are several places apart from arch_setup_gd() where gd is
> > assigned to. Superfluous assignments lead, at best, to confusion, and
> > at worst, to subtle as well as not-so-subtle bugs, so we need to find
> > and remove such superfluous assignments.
> >
> > This post gives a list of places where gd is being assigned (courtesy
> > of a git grep '^\s\+gd\s*=\s*.*$' command) and suggestions re removal.
> >
> > Comments welcome, of course.
> >
> > -----------------------------------
> >
> > arch/arm/cpu/arm926ejs/mxs/spl_boot.c:126:      gd = &gdata;
> >
> > 	This assignment is in mxs_common_spl_init() which is called
> > 	from a board_init_ll() defined by various boards, and itself is
> > 	called from arch/arm/cpu/arm926ejs/mxs/start.S. Setting gs is
> > 	required because mxs_common_spl_init() calls lots of functions
> > 	which depend on gd.
> >
> > 	Removal: requires MXS SPL to move over to using crt0.S.
> >
> > 	Note: since MXS SPL actually returns to ROM to get U-Boot
> > 	launched, it may require cooperation from crt0.S for saving
> > 	important registers at reset entry point and restoring them and
> > 	returning to ROM. See OMAP (for register saving) and FEL.
> >
> > 	Boards affected are:
> >
> > 	mx28evk_nand xfi3 bg0900 sansa_fuze_plus mx23evk m28evk
> > 	sc_sps_1 mx28evk_spi axm mx28evk apx4devkit mx23_olinuxino
> > 	firefly-rk3288 x600 mx28evk_auart_console taurus
> 
> This list of affected boards does not seem to be correct. I fail to
> see, how firefly-rk3288 or x600 are affected by this "mxs" file.

Correct -- I'd patched arch/arm/cpu/arm926ejs/mxs/spl_boot.c with
a #error directive then blindly listed all boards given by buildman -s
which included boards not building cleanly even for other reasons that
the one I was looking for.

> > arch/arm/lib/spl.c:40:  gd = &gdata;
> >
> > 	This assignment is in a weak board_init_f(), therefore executed
> > 	from crt0.S (no ARM board calls board_init_f() from elsewhere);
> > 	therefore gd is already set when this assignment is reached.
> >
> > 	Removal: unconditional.
> 
> I'm wondering, if this weak default board_init_f() function is
> called for any ARM platform at all? If not, it would be clearer
> to remove this function completely.

I'll try and find out if the weak board_init_f is used at all, and if
it is not, I'll amend the suggestion to 'removal of the whole function'

> Thanks,

Thanks for your review!

> Stefan

Amicalement,
-- 
Albert.