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

Albert ARIBAUD
2015-11-25T15:20:50Z

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

arch/arm/cpu/armv8/fsl-layerscape/spl.c:48: gd = &gdata;

Performed in board_init_f() by boards:

ls1043ardb_nand ls2085ardb_nand ls1043ardb_sdcard
ls2085aqds_nand

These four boards build SPL using arch/arm/lib/crt0_64.S which
unconditionally executes arch_set_gd() and thereby has gd
properly set when arch_set_ board_init_f() is entered, so that
assignment is redundant.

Removal: unconditional.

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.

arch/arm/mach-exynos/spl_boot.c:279: gd = gdp;

This assignment occurs from withing a machine-specific
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.

(non-ARM architectures skipped over)

(non-ARM boards skipped over)

common/board_f.c:982: gd = &data;

Only m68k, avr32 and nds32 use this. It could be conditioned to
these architectures and later removed if and when they switch
to the same boot sequence as ARM et al.

Removal: only if and when arch switches to a boot sequence
similar to ARM's.

common/board_r.c:942: gd = new_gd;

Done by arches other than ARM, AARCH64 and X86. For ARM, gd is
switched from early to final location in crt0. For x86? In any
case, this assignment cannot be removed unless all arches have
switched to a boot sequence similar to ARM's.

Removal: only if and when arch switches to a boot sequence
similar to ARM's.

common/init/board_init.c:28: gd = gd_ptr;

That is the only assignment we should keep, for architectures
other than ARM or x86.

Removal: no.

common/spl/spl.c:450: gd = new_gd;

This is the second place where gd assignment should be kept,
and for ARM it must be fixed the same way arch_setup_gd was,
since this assignment to gd may not work with ARM gcc (this
change will go in v8 of my patch)

Removal: no.

Amicalement,
--
Albert.