Re: [PATCH] powerpc: Better machine descriptions and kill magic numbers

[Olof Johansson <olof@lixom.net>]

On Sun, Jan 15, 2006 at 08:04:46AM +1100, Benjamin Herrenschmidt wrote:
> 
> > Because of this, I suggest keeping the mach_ part in the syntax, i.e,
> > to use:
> > 
> > machine_is(mach_powermac) instead, and not do preprocessor mangling of
> > the label name. The same would go for define_machine().
> > 
> > It would seem to make some sense to keep the platform types looking
> > like defines (i.e. MACH_POWERMAC instead of mach_powermac) to keep it
> > consistent with the CPU and firmware feature testing, but the way it's
> > implemented that doesn't make much sense. That's a bit of a shame.
> 
> I'm not sure about these ... I'm definitely not fan of going uppercase (and
> it makes no sense vs. the implementation as it's not macros). I also don't

Yeah, that's what I meant with doesn't make sense in this case. I was
just thinking out loud.

> get your problem with grep... the "mach_" construct is totally invisible,
> so people wouldn't grep for it, but for "powermac" alone.
> 
> Or do you mean a grep on "powermac" might return too many things unrelated ?
> 
> It should be easy enough in this case to grep for machine_is(powermac) instead
> then, after all, that's the only possible use...

One drawback is that source navigation tools like tags and cscope probably
won't find it, and the base strings are quite generic (powermac, cbe/cell,
etc). A stringbased grep will, sure.

> > In the probe loop, ppc_md is copied over for each probe, that seems
> > wasteful. 
> 
> I changed that from the old way indeed to allow the probe() function to
> override things in ppc_md.
> 
> > Shouldn't the probe routines use and modify their own
> > machdep_calls instead of ppc_md, so the copying can be done only once,
> > after a match is found?
> 
> I don't like modifying their own machdep calls because the name of the
> structure is hidden. In general, I prefer that things continue to only
> access ppc_md. and not their own machine structure that goes away. That
> means that the code "patching" it can be moved away etc... without
> special case instead of having a special case in probe() that has to
> reference the machine specific copy...

Ok.

> I doubt the memcpy per machine at boot will cause any kind of
> significant performance issue ...

Yeah, that's why I said it seemed wasteful, not that it'd be a
performance issue.