Re: CONFIG_GENERIC_PPC32

[Paul Mackerras <paulus@samba.org>]

Michael Sokolov writes:

> Some commentary is in order on this cset. I strongly recommend that Paulus,
> people interested in bi_recs enhancements, and people interested in GT-64260
> support check it out and give it a good look.

I have had a quick look...

>  Any number of platforms can be included in the
> configuration. Each is selected with a bool in config.in so the user can
> include only the platform(s) that s/he needs, while distribution makers can
> ship one kernel for the world.

A laudable goal.  I like the idea of having each platform selected
with a bool rather than just having a single choice of platform.
I also agree about the CONFIG_ALL_PPC name but no-one has ever been
able to come up with a better name for the PReP/PMac/CHRP combination.

However, there are some aspects of the way you have done your
CONFIG_GENERIC_PPC32 that I don't like.  In particular I don't like
the list of ifdefs in setup.c.  Whenever that sort of thing appears it
is a sign that we need to rethink how we do things.  The old
drivers/net/Space.c was a classic example, and it was a mistake and it
basically became unmaintainable.  Fortunately we have better ways to
handle that sort of thing these days.  In particular we can use ELF
sections in creative ways to make lists of things without having to
have strings of ifdefs.

In fact, any place where we have lists of things, one per platform or
one per config option, is a place where we are going to potentially
get maintainability problems as the number of platforms we support
grows.  At the moment we can't really avoid that completely in the
Makefiles and config.in files.  We can avoid it in C source and we can
mostly avoid it in header files (but not completely at this stage).
As a guideline, ultimately I want it to be the case that you can add
support for a new board without changing *any* existing source files
in the kernel tree, i.e. just by adding files.  (That means that
anything that lists platforms or config options will need to be
autogenerated.)

The other thing I don't like is the bi_rec changes.  While I like the
idea of passing in device information in bi_recs, I really don't like
the use of binary tags for the various specific pieces of information
that we want to specify for the different devices.  This is ultimately
once again a maintainability concern.  Using an enumeration like that
basically forces us into having a central repository for assigning the
numbers and that is going to get us into problems down the track.

Instead I think that both the names of the pieces of information, and
the values of things like the device type, should be represented as
strings.  Using strings just gives us orders of magnitude more
flexibility and extensibility, and is much more suitable for the sort
of decentralized and distributed development that we do.

Now if we are worried about space then we can get creative about how
the strings are stored in the bi_recs, for instance we could store
each unique string exactly once in a string table and then just use a
16-bit index in each place where we want to refer to a string.  We
could put the string table in a bi_rec of its own, and even gzip it if
necessary.

About the early_serial_init changes - using early_serial_init is nice
when the serial driver is built in, but the problem is that when the
serial driver is a module, we don't get given the opportunity to do
the early_serial_init calls between when the module is loaded and when
it runs rs_init.  Otherwise I would have decreed the use of
early_serial_init some time ago. :)

Regards,
Paul.

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/