[U-Boot] [PATCH 1/3] init_func: Add fundamental framework

[Marek Vasut]

Dear Graeme Russ,

> Hi Marek,
> 
> On Thu, May 3, 2012 at 1:18 PM, Marek Vasut <marex at 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 at 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