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,