[U-Boot] [RFC] New init sequence processing without init_sequence array

[Graeme Russ]

Hi Wolfgang,

On Tue, Aug 23, 2011 at 6:10 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Graeme Russ,
>
> In message <1313587343-3693-1-git-send-email-graeme.russ at gmail.com> you wrote:
>> I have been thinking about the problem of the pesky init_sequence arrays
>> and the inevitable #ifdefs and empty stub functions that result so I
>> thought I'de have a crack at a more dynamic implementation. And like all
>> good programmers, I stole the solution ;). This implementation is based
>> on Linux's __initcall(fn) et. al. macros
>>
>> If this works cross-platform, we can finally move board_init_* into
>> /lib/ - Wouldn't that be nice
>>
>> Thoughts?
>
> My initial thoughts are these two:
>
> 1. I think we should change the code in a different order.  I would
>   prefer to first unify the init code across architectures (the big
>   ARM reorganization we just seem to have overcome was an important
>   prerequisite for this), before we start changing the init code.
>

I agree - I am currently auditing the init sequences and teasing out any
common ordering and identifying where arches call an otherwise common
init function out-of-order to the others. This is, generally speaking,
very ugly - There are:

 - Two archictectures that do not use the init loop
 - Two that do not relocate and therefore have a mix of (what other arches
   consider) _f and _r int functions in a single loop
 - Functions that could be made init functions after the init loop in
   board_init_r
 - Some arches have an equivalent init_f loop, others have an init_r loop
   x86 is the only one that does both _f anf _r init sequences as loops

I think, as a first step, all arches need to implement an init_f and
init_r loop and collapse board_init_f and board_init_r into trival
loop processing functions - Combine them into one function (which takes
a pointer to the relavent init_function array) and move it into
/lib/init.c - Easy ;)

> 2. One of the advantages of the current implementation is that there
>   is a central place in the code (well, at least per architecture,
>   untill we unify these as mentioned above) where you can see exactly
>   which steps get performed in which sequence.

This new method can have the same by putting all the INIT_FUNC()
in a single location, much like the existing array. I don't think this
is a good idea though. I prefer good clear documentation.

>   I understand (and appreciate) your intentions, does the initcall
>   concept not mean that we will have some sort of sequence numbers
>   distributed all over the code?  Maybe I'm mising something - but
>   does this not make it really difficult to actually see (from the
>   source code) what the exact init sequence is?

Well the bulk of the init sequence will be defined by init steps clearly
defined in include/init.h. Where there is as arch, SoC or board which
needs an init function called immediately after a particular point in the
standard sequence, then there will be:

	#define CUSTOM_INIT_STEP	STANDARD_INIT_STEP + 1

> 3. Hardware initialization in inherently very much hardware depen-
>   dent ;-)  We have some boards where PCI must be initialized early
>   because it is needed to access some other peripherals like memory.
>   And we have other boards where PCI can only be initialized late
>   because it depends on a lot of other functions that must be working
>   first.

Yes, I am seeing that in my audit

>   I explained this a number of times before: the current code was
>   designed to allow even for completely board specific init
>   sequences, by simply adding #define for the list of init functions
>   to the board config file - see for example here:
>
>   http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/33951/focus=36436
>
>   http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/72131/focus=72136

Yes, I saw some of the macro'd init functions in my audit, and I must say
that I am not a fan. If we continue down this path, we will have an init
sequence array made up of purely of macros and then it will be a PITA to
search throught the code resolving the macro for your particular arch /
Soc / board. Also, it does not allow the functions to be put in an
arbitrary order in a central location - Do do so will require a really bad
mess of #ifdef's - This will be a big barrier to centralising the whole
init sequence in say /lib/init.c as each arch will still need to put the
critical init function array in /arch/lib/board.c

>
>   If you look at the current code - heavily larded with #ifdefs of
>   all shapes and colors - I cannot see any good way to transform this
>   into an initcall (and thus sequence number based) implementation.
>   Do you have any specific ideas how to address this?

The _really_ nice thing about initcall is that if you take something like
PCI, you put the INIT_FUNC() in pci.c and it only gets included if you
define CONFIG_PCI - No #ifdefs. The problem, as you quite rightly point
out, is how to clearly define INIT_STEP_PCI. From what I am seeing, these
are mostly corner cases that can be clearly documented.

Now to answer some points in your other email:

> >  For a board to add an arbitrary initialisation function is trivial -
> >  simply add a INIT_FUNC() entry with an appropriate sequence number
>
> This is actually hte biffest voncern I have: I cannot imagine how you
> will try to figure out the exact init sequence when writing the code
> (or, even worse, when reading somebody else's code).

Good point - The init sequence audit I am doing now will help. It will
become a lot clearer when the init sequences are unified. For the rest,
good clear documentation will help, but I think we will only know how it
looks and feels when a complete patchs hits the list.

One nicity of putting INIT_FUNC() after a function body (or just before
it which is easier to see if the body is longer that a screen) is that
you can see immediately that the function is involved in the init
sequence - I kind of like that.

> >  I imagine the sequence numbers could be #defined, but I don't exactly
> >  now what the linker will do - I use leading zeros in the sequence numbers
> >  to ensure correct ordering.
>
> This is another concern: what exactly will the linker do, and
> different versions of linkers with differen options and/or levels of
> optimization?

I have another patch now that #define's the step numbers. As long as the
step numbers are 100-999 (or 1000-9999, 10000-99999 etc) then everything
works fine. The linker MUST support SORT_BY_NAME, KEEP and DISCARD - can
anyone think of a linker that does not? Optimization will only be an issue
if the linker disobeys SORT_BY_NAME, KEEP or DISCARD as part of the
optimisation (which I would think would be brain-dead as these directives
are clearly telling the linker to do something very particular)

OK, so what do I see as adventageous about using initcall versus current
implementation...

 - Any arch, SoC or board can cleanly inject an init function anywhere in
   the init sequence without touching a single line of code outside that
   arch/SoC/board. The current system requires a mod to /arch/lib/board.c
   with the potential addittion of ugly #ifdef's
 - The _f init function pointers are put in a section outside of the data
   copied to RAM during relocation, slightly reducing the RAM footprint
 - No more #ifdef in the init sequence array
 - sub-systems (PCI, IDE, PCMCIA, SCSI) define the init function in their
   respective .c files and it gets pulled in when the Makefile pulls in
   the .c file
 - It's more like Linux :)

And the bad...
 - Init sequence not clearly obvious in a single location
 - Maybe brain-dead linkers will do something wrong

And the neutral
 - Dealing with specific arch/Soc/board requiring an init function at a
   different sequence step than everything else

Some neat features that are trivial to implement with initcall and
difficult or ugly with init array:
 - The names of the function can be (conditionally by DEBUG) added and
   printed out as the init sequence is being run - Very nice for debugging
   the init sequence
 - Boot profiling - Store the step number and time to execute then after
   bootup completes, a simple 'show boot profile information' can print
   each function name and the time taken to execute

Honestly, after having a good play with this and fully converting x86 to
trivial board_init_f and board_init_r functions, I really like the initcall
method - It really does 'just work' like a dream :)

Regards,

Graeme