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

[Wolfgang Denk]

Dear Graeme Russ,

In message <CALButC+W1ekU6XmaW5FvVOFc6Y4bhchVWvFGBV_FQPw=pUSQdA at mail.gmail.com> you wrote:
> 
> > 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 guess this will not be a single big change, but a step by step
approach.  We will probably start by merging Power and ARM
architectures, and probably x86.  Others can follow one by one.

> 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 ;)

OK with me - eventually we will have some kind of architecture
cleanup, dropping dead architectures on the way ;-)

> > 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.

The problem is that "clear documentation" is no replacement for
clear (and easily readable) code - if you maintain such information in
two different places (doc and code) you have a guarantee that these
two get out of sync rather sooner than later.

> >   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

Hm... I don't really consider this an improvement.

> >   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.

I'm not talking about the documentation - I'm worried about the
implementation - especially when some board stops working at some
pooint, or when trying to understand patches submitted for a new
board.  It has always been hard enough to debug issues in the early
init code, and I don't expect this to improve with the suggested
changes.

BTW: did you check the impact on the memory footprint yet?

> > 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.

Agreed.

But for me it is _way_ more important to be able to see the _complete_
list of all init functions, and their specific order of execution for
a given board configuration.

> 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

Yes, I can.  SORT_BY_NAME() was introduced with binutils version 2.16,
so there will be problems when building with all tool chains using
older binutils.


> 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

I tend to see this as a disadvantage, actually [as it makes it easily
to lose track about which code is running when, or at all].

>  - It's more like Linux :)

I don't see why this would be an advantage per se.  Let's move this to
the "neutral" part ;-)

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

Agreed.

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

I don't understand what you mean here.

> 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

You are actually wrong here - guess why the current init loop does not
contain such a debug()?  There are some init functions that need to be
run before you can output anything (for exaple, you must initialize
the console driver, including setting the baud rate or similar
parameters from the environment).

>  - 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

Could be done with the same amount of effort in the init loop as well.

> 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 :)

If it works, yes.  My concern is what to do if it doesn't, how to
debug such situations, and how to review such code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I can't understand it. I can't even understand  the  people  who  can
understand it.                    - Queen Juliana of the Netherlands.