From mboxrd@z Thu Jan 1 00:00:00 1970 From: Graeme Russ Date: Tue, 23 Aug 2011 09:20:44 +1000 Subject: [U-Boot] [RFC] New init sequence processing without init_sequence array In-Reply-To: <20110822201023.0F8A111EF9D9@gemini.denx.de> References: <1313587343-3693-1-git-send-email-graeme.russ@gmail.com> <20110822201023.0F8A111EF9D9@gemini.denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Wolfgang, On Tue, Aug 23, 2011 at 6:10 AM, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message <1313587343-3693-1-git-send-email-graeme.russ@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