Dear Graeme Russ,
Hi Marek,
On Thu, May 3, 2012 at 1:18 PM, Marek Vasut marex@denx.de wrote:
Dear Graeme Russ,
Hi Marek,
Thanks for taking a look
+The INIT_FUNC macro allows initialisation functions (i.e. functions which are +executed before the main loop) to be easily added to the init sequence +
+Specifying an Initialisation Function and is Dependencies +--------------------------------------------------------- +The format of the INIT_FUNC macro is:
+INIT_FUNC(fn, grp, man_reqs, pre_reqs, pst_reqs)
+fn is the name of the init function to call. This function must have the +following prototype:
+int foo(void);
What if I want to pass some data to such a func ? Clearly, I can think of this being doable, but extra hard.
The idea is that no changes are being made to the existing init methodology.
Well ... what if I want to pass eg. pdata?
Do what we do now - Global Data is the only way to pass data between initialisation functions. Sure, this _might_ change in the future (but I doubt it) but we can deal with that later
Global data won't die, but they should shrink as much as possible ... so I don't like the idea of using global data that way.
+Each init function must return 0 to indicate success - any other return value +indicates failure and the init sequence will stop
+grp is the name of the group that the init function belongs to. grp may be +the same as fn for any individual init function, but between init functions, +fn and grp must be unique.
+The purpose of groups is to allow functions to be grouped together so other +functions can specify the group as a whole as a dependency rather than having +to list every function in the group in the dependency list + +man_reqs is a space seperated list of functions or groups that MUST exist and +MUST run BEFORE fn
+pre_reqs is a space seperated list of functions or groups that MAY exist and +(if they do) MUST run BEFORE fn
+pst_reqs is a space seperated list of functions or groups that MAY exist and +(if they do) MUST run AFTER fn
What's the point? Can't you create a kind of proxy object that the pst_reqs will have as a pre_req ?
Maybe you should create this:
INIT_FUNC(fn, grp, prereqs, postreqs) and for each function from prereqs and postreqs, specify per-function attributes via the GCC __attribute__(()) directive, like if the function must run before something or may run before something etc?
Eep, I would like to see that implemented - I'm sure it would be 'interesting'
Pervy and sadistic at least :-)
The way INIT_FUNC and friends work is to simply build a list of static strings (which we just happen to shove in a seperate section so they can be filtered in/out based on the link stage)
Yes, I got the understanding of it. I was more bothered by the man_reqs and pre_reqs, what if we soon need functions that are executed under another weird condition? And if you look at it the other way -- if you need function that's executed conditionally, why not wrap the condition into the pre_req (or some proxy object, whatever).
It's not about being executed conditionally - It's about 'I need to be initialised after 'X', but if 'X' does not exist, I can still initialise'
Network is one example - If you have a PCI network adapter, it must be intialised after PCI. But if the adapter is not PCI, you still want to be able to initialise it
So why don't we make function, that can depend on "OR-group" and/or "AND-group" then? Won't that be more logical? I mean, group where you must execute as much as possible, to be OR-group and group where you must execute everything to be AND-group.
Timer is another - Hopefully the timer infrastructure will soon be an arch independent API. But each arch (and potentially down to the board level) may have a role to play in getting the timer infrastucture running (FPGA, PLL, PIT intialiastion). So potentially, the common driver code would be intialised with:
Well, I hope to create unified timer api with the DM biz.
INIT_FUNC(timer_api, timer, arch_timer, board_timer, );
So the arch_timer function MUST exist and it MUST run before timer_api. The board specific timer initialisation is optional. If the board has timer related code, it MUST run before timer_api but timer_api can still run if board_timer does not exist (i.e. all the timer init done at the arch level)
Ok, I see. OR-groups won't fit in here?
Now depending on the arch and board, board_timer may be either:
INIT_FUNC(board_timer, timer, arch_timer, , timer_api);
i.e. arch -> board -> api
or
INIT_FUNC(board_timer, timer, , , arch_timer timer_api);
i.e. board -> arch -> api
NOTE: In both of the above example, including timer_api as a post-req is redundant. The timer_api INIT_FUNC definition already mandated the order. This is not a problem and including or excluding it makes no difference to the output. Of course, including timer_api in the above examples means we could have done:
INIT_FUNC(timer_api, timer, arch_timer, , );
But adding the redundant references makes it clearer when you are looking at that bit of code. The 'black-box' tool will warn you if you created a circular reference and will print out what the init sequence is that created the circular reference.
Neat.
Now, here is where the fun begins :) - Lets say you need timer support early, but you can't get the low level infrastructure for hi-res timers running until later...
board/vendor_me/common/timer.c INIT_FUNC(me_timer_init, me_late_timer, me_pit_init, , ); INIT_FUNC(me_pit_init, me_late_timer, , fpga_init pll_init, me_timer_init);
(again, me_late_timer in the second INIT_FUNC is redundant)
board/vendor_me/board_a/timer.c INIT_FUNC(fpga_init, me_late_timer, , , me_pit_init);
board/vendor_me/board_b/timer.c INIT_FUNC(pll_init, me_late_timer, , , me_pit_init);
(again, me_pit_init in these INIT_FUNCs is redundant)
board/vendor_me/board_c/timer.c /*
- This board has the PIT hard-wired to a crystal oscillator so there
- is now board_level init function - So we can actually initialise
- the PIT earlier :)
*/ int me_pit_early_init(void) { return me_pit_init(); } INIT_FUNC(me_pit_early_init, timer, arch_timer, , );
/*
- Don't need any late timer init - This will skip both me_timer_init and
- me_pit_init as they have been defined in the me_late_timer group
*/ INIT_SKIP(me_late_timer);
The idea is that me_late_timer() will perform some tweak to the timer API to redirect it to the hi-res clock source. The hi-res clock source is driven by a PIT common to all the boards manufacture by this vendor. But the PIT has a different raw clock source depending on a particular board Board A has an FPGA, board B has a PLL and board C uses a hard-wired crystal oscillator
I see.
+Skipping or Replacing a Function or Group +----------------------------------------- +Occassionally, a board may provide a completely seperate implementation for +an initialisation function that is provided in the common arch, SoC or +common code.
+SKIP_INIT(fn_or_group)
+After the initialisation function dependencies are calculated, all functions +and groups listed in any SKIP_INITs are removed - This may result in +dependent functions being removed - It is up to the board code developer +to ensure suitable replacements are in place
+REPLACE_INIT(old_fn_or_group, new_fn_or_group)
+Like SKIP_INIT but replaces on function with another (or one group with +another)
+Example: In the SoC code yoy may have
Yoy :)
Yes. The ultimate goal is to remove a heap of '#ifdef mess' and delegate the selection of initialiasation functions to where is is most appropriate. CPU init in ARCH code with board specific init in board code with the ARCH code 100% unaware of what the board is doing. This will also make it a lot easier for vendors to implement multi-arch solutions :)
Correct, alongside kbuild and driver model, we'll make an awesome bootloader. But fix that s/yoy/you/ please ;-)
Ah, I see now... Will do
+INIT_FUNC(init_cpu_f, RESET, , , );
+In the board code, you may want a slightly tweaked version, so you might +have:
+int my_new_init_cpu_f(void) +{
...+} +REPLACE_INIT(init_cpu_f, my_new_init_cpu_f);
[snip]
diff --git a/tools/mkinitseq.c b/tools/mkinitseq.c new file mode 100644 index 0000000..b150de4 --- /dev/null +++ b/tools/mkinitseq.c @@ -0,0 +1,1512 @@
Ok, this is the worst part. I think this could be reimplemented in shell ;-)
Be my guest :) - Have a really good look at what is happening behind the scenes - Lots of list processing and checking. In particular, the processing of the REPLACE and SKIP macros would be particularly nasty.
That's what sed can do for you, can't it ?
So below - If you can show me (i.e. write the whole thing in shell) I might include it
I'll think about it ;-)
I have no problem with this being done in shell code. BUT if I have to do so in order for it to be accepted, then I'll bin this patch series right now. I'm more than happy to integrate somebody elses shell-based tool into the series. Sorry to be so blunt, but I do not have the time or energy to re-implement this myself.
Ooh, I smell flame-fuel :-) I'd certainly prefer to see this in shell, since I believe the substitutions and reordering of text might be easier there.
See above - be my guest ;)
My argument is that this is a black-box tool like gcc/ld. Once we prove it to do what it does correctly, we can 'leave it alone'(tm) and test cases are fairly trivial. You can even mock-up the input file in vi :)
Ewwwwww ... like gcc/ld ... or maybe like Windows (TM)(C)(R)(?) :-D
No, like FLOSS gcc/ld :P
:-)
And is a shell based implementation going to be any easier to understand, modify, debug, etc?
I wonder :-)
Well, if someone manages to do it in shell, we will see - Take a look at main() - I have been very carefull to layout the way the tool operates in a very linear way but ther lists are global, so sharing data between the shell stages will be a pain (pipes?)
Bash can do locals ;-)
+/*
- (C) Copyright 2012
- Graeme Russ graeme.russ@gmail.com
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+/**
- container_of - cast a member of a structure out to the containing
structure + * @ptr: the pointer to the member.
- @type: the type of the container struct this is embedded in.
- @member: the name of the member within the struct.
- */
+#define container_of(ptr, type, member) ({ \
const typeof( ((type *)0)->member ) *__mptr = (ptr); \(type *)( (char *)__mptr - offsetof(type,member) );})Don't we already have this defined in uboot ?
mkinitseq is host-code so it only brings in host headers (not U-Boot headers) - If you don't have Linux kernel headers installed, you can't get this macro. Actually, even though I do have them, I had a hard time getting this macro in, so I gave up - Any hints?
Reimplement this thing in shell :-)
You already know my answer to this ;)
As an aside, I wonder if the kernel headers are required for the list macros?
Nope.
Good :)
Regards,
Graeme