Hi Wolfgang,
On Wed, Aug 24, 2011 at 3:38 PM, Wolfgang Denk wd@denx.de wrote:
Dear Graeme Russ,
In message CALButCJg5BPP_Z0VUMEfQgjpRYO=WDQBvvBFho6VbNovbRjFzQ@mail.gmail.com you wrote:
So we end up with:
#DEFINE INIT_GLOBAL_START 10000 #DEFINE INIT_X86_CPU_F INIT_GLOBAL_START + 1 #DEFINE INIT_ARM_CPU_F INIT_GLOBAL_START + 1 ... #DEFINE INIT_X86_<foo> INIT_X86_<bar> + 1 ... #DEFINE INIT_X86_TIMER INIT_X86_<foo> + 1 #DEFINE INIT_ARM_TIMER INIT_ARM_CPU_F + 1 ... #DEFINE INIT_ENET_TIMER INIT_X86_TIMER + 1
So this gives you a single central location to quickly see the init sequence of any arch, SoC, or board.
But frankly: do you consider this list above _readable_?
So how do I figure out which steps are actually being executed? I could, for example, assume that INIT_X86_TIMER and INIT_ARM_TIMER are run at the same time (in random order) on one board - OK, here it is obvious from the names that this is probably not the case, but we will have other, less obvious situations.
grep is your friend - All you need to to is grep for GLOBAL (actually I think COMMON is a better name) and the ARCH, SOC, and BOARD keywords in the namespace for your board and voila - You have a sorted list of the init sequence for you board
#define CUSTOM_INIT_STEP STANDARD_INIT_STEP + 1Hm... I don't really consider this an improvement.
Why not? See above - I think centralising EVERY init sequence in init.h rather than splatering board specific #ifdef's in arch/board.c is an improvement - YMMV
You keep the definitions in one place, but in a mor or less unreadable form.
I don't see how it is unreadable after grep - I would argue that it is way more readable than the #ifdef mess we are heading down
That was released in May 2005 and in U-Boot parlance 'ancient' ;) - But I am well aware that lack of SORT_BY_NAME() is a total killer for what I am proposing. I need to know if anyone is using a linker that does not support SORT_BY_NAME(). If there are (and if those users have no option to use a linker that doeS) then the whole initcall idea is dead in the water anyway
We do have customers who are still using ELDK 2.1.0 from April 2003 (gcc version 2.95.4, binutils version version 2.11.93.0.2). Agreed, these are not using recent versions of U-Boot either.
And the bad...
- Init sequence not clearly obvious in a single location
Well actually, I think I can resolve this
How? Above suggestion is not a solution, but actually shows the problem.
Take timer initialisation for example - This is at a different step in every arch. I think this is easily resolved with a good #define namespace and putting it all in init.h
And this becomes easy to read and understand, then? :-(
- 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).
The current method cannot print the init function name unless the init function itself does so. I have included an additional component to the
It would be trivial to enable printing the names from the loop; we can't do it because we don't have the needed prerequisites yet in most cases.
How so? I cannot see how you could create a macro which when used in the static array initialiser would send the function pointers to one array and the string names (or pointers to) to another array.
INIT_FUNC macro which adds the string name of the function and builds a second table of pointers to these strings. The init loop then simply steps through the function name pointers while stepping throught the function pointers. If we simply add a CONSOLE_INITIALISED flag into gd->flags then the output of the function names can be suppressed until console output is available.
You know that debugging becomes easy as soon as we have printf(). But how does this help you for the init steps _before_ you have a working console?
How does debugging in that case work now? There is no difference between the two implementations - the only thing I am changing is: - Where the array of function pointers gets placed the U-Boot image (.initfunctions.number versus .data) - The fact that you can inject an arbitrary function pointer (at compile time) into the array without messy #ifdef's
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.
Debug is no different to now - The code is stepping through a table of function pointers. The new method can add the printing of the function
The difference is that I have a source file for reference.
Huh? You still do in the initcall case
With the init.h as above I have no such reference.
No reference to what? - the static array of function pointers? This is of little use anyway as all your debug stepping will do is pick up the next value from the array
about to be called (after console init) which makes life a little easier. As for patches, the inclusion of INIT_FUNC() in the patch is a red-flag to look more closely - Any new step will also include an addition to init.h
So init.h will quickly become an incomprehensible mess.
No, it becomes a clean linear list of the init sequence from which you can easily grep out the noise. This list will never have an init step defined out-of-order. If INIT_YOUR_BOARD_ETHERNET occurs after INIT_YOUR_ARCH_PCI in the list then you know your board initialises its Ehternet after the arch has initialised PCI
Regards,
Graeme