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

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

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

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)

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

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

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.

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 :)

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

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

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

Regards,

Graeme