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

[Marek Vasut]

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?

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

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

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

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

> 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

> And is a shell based implementation going to be any easier to understand,
> modify, debug, etc?

I wonder :-)

> >> +/*
> >> + * (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 :-)

> 
> As an aside, I wonder if the kernel headers are required for the list
> macros?

Nope.
> 
> Regards,
> 
> Graeme