[U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it

[Simon Glass]

Hi Graeme,

On Fri, Dec 30, 2011 at 7:49 AM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Simon,
>
> Sorry for the delay in reviewing this - I've been doing a lot of work on
> the x86 side of things. I now have a working solution to the
> board_init_f_r() / global data clobbering problem which involves having the
> gd 'variable' as a register like all other arch's. The solution is
> non-trivial and gd access is slightly more expensive than the vanilla
> variable approach, but it makes this a lot cleaner cross-arch wise...
>
> Here's a hint ;)
>
> static inline gd_t *get_fs_gd_ptr(void)
> {
>       gd_t *gd_ptr;
>
>       asm volatile("fs movl 0, %0\n" : "=r" (gd_ptr));
>
>       return gd_ptr;
> }
>
> #define gd      get_fs_gd_ptr()
>
> On 28/12/11 17:35, Simon Glass wrote:
>> This series creates a generic board.c implementation which contains
>> the essential functions of the various arch/xxx/lib/board.c files.
>>
>> What is the motivation for this change?
>
> [snip]
>
> I think that we can all agree that there is strong motivation for change.
>
> However, I think this approach is not the right one - and I think the CFI
> driver backs me up. Your plan is to create generic code which you want ALL
> arches to cross over to, but you only look to migrate two initially and
> migrate the rest 'later'. This is similar to what happened with the CFI
> driver, and there are still boards with custom flash.c files which are
> completely redundant.
>
> But, creating a single patch-set to migrate everyone in one go is going to
> be too massive a job to do in one go, and too prone to introducing breakage.

Yes to some extent. However, my patch basically splits the two
board_init_x() functions into parts, and puts them in a function
table.

I don't think it is a huge job to do this for PowerPC also, and that
seems to be the most feature-full architecture.

Also it does depend on expectations. I would hope that moving an
architecture over would be a fairly small task:

- getting generic relocation working
- adding functions for anything that is missing from board init code
- removing things which don't work on the architecture?
- worrying about differences in ordering between functions

>
>> All the functions of board_init_f() and board_init_r() are broken into
>> separate function calls so that they can easily be included or excluded
>> for a particular architecture. It also makes it easier to adopt Graeme's
>> initcall proposal later if desired.
>
> I think we should look at this sooner rather than later. I've shown with
> x86 that the init sequence has three distinct phases:
>
>  1) Execute in flash using temporary RAM
>  2) Copy U-Boot from flash to RAM
>  3) Execute in RAM
>
> My latest work now has all init functions having the same signature (i.e.
> void parameter returning int). For x86, there is a little 'magic' that
> needs to be done as gd is copied from temporary RAM to SDRAM, but for other
> arches using a dedicated register, this is otherwise trivial.
>
> So instead of trying to pluck out something (relocation in this case) from
> the molasses of ten different board.c files and having to perform
> open-heart surgery on them all to get the code grafted in, I think we
> should approach it from the generic init sequence angle.
>
> If we work each individual arch to use a generic init sequence (like the
> proposed x86 code) then the init processing naturally falls out as common
> code and the patch to realise this is trivial. From there, we can start to
> pull out common init code like init_baudrate() and hang() and change the
> function signatures of the functions that require wrappers and move some
> #ifdef's into more appropriate locations - One example in board.c:

Well it seems like a lot of work to refactor each arch/xxx/board.c
file into functions with a function pointer list, then later remove
this code function by function.

My feeling is that with a generic board, it should hopefully be a
fairly small amount of work for someone familiar with an architecture
to find the bugs and patch the generic code to suit their
architecture. It is something that needs to be done once, not every
time there is a new patch removing (almost) common code.

>
> #ifdef CONFIG_BITBANGMII
> int bb_miiphy_init_r(void)
> {
>        bb_miiphy_init();
>
>        return 0;
> }
> #endif
>
> Ouch!
>
> The other big benefit is that you only touch one architecture at a time up
> until you 'pull the switch'. And when you do pull the switch, you should be
> factoring out identical code so the chances of breaking something should be
> vastly reduced. Take a look at the history of ARM relocation for example -
> that was constrained to one arch but still the amount of breakage was massive.