Re: [RFC patch 00/32] x86: Refactor the setup code to provide a base for embedded platforms

Thomas Gleixner
2009-08-23T09:19:10Z

On Sat, 22 Aug 2009, Ingo Molnar wrote:
> * Thomas Gleixner <tglx@linutronix.de> wrote:
> > 47 files changed, 622 insertions(+), 808 deletions(-)
>
> Very nice!
>
> One small detail, before we spread out these patches. While looking
> at the patches i noticed that at places our new x86 init namespace
> is very long:
>
> > + platform_setup.timers.setup_percpu_clockev = platform_setup_noop;
> > + platform_cpuhotplug_setup.setup_percpu_clockev = platform_setup_noop;
> > +
>
> I think we should shorten the name-space a bit - we'll use it in a
> _lot_ of places, so the shorter, the better and the easier to use.
>
> I'd suggest something like:
>
> x86_init.timers.init_percpu_clockev = x86_init_noop;
> x86_cpuhotplug_init.init_percpu_clockev = x86_init_noop;
>
> ( This also has the advantage that 'init' is the general term we use
> for kernel structure initialization - 'setup' is a more
> restrictive term we use related to bootloading, most of the time. )

Fair enough.

> An even shorter form would be to use 'x86' as a general template for
> platform details:
>
> x86.timers.init_percpu_ce = x86_init_noop;
> x86_cpuhotplug.init_percpu_ce = x86_init_noop;
>
> this is even shorter, plus it allows us to put runtime details into
> this structure as well. Note that the fields themselves

We should have a separate struct for runtime details otherwise we need
to keep the full init stuff around forever instead of freeing
it. That's why I already have that separate cpuhotplug struct.

> (init_percpu_clockev) already signal the 'init' property
> sufficiently. Plus 'ce' is an existing, well-known abbreviation for
> clockevents. (but 'clockev' would be good too - i might be pushing
> it)

Yes you do. Next you suggest to have x86_i.pcpu_ce :)

Thanks,

tglx