Re: [PATCH] ALSA: cs5535audio: only build OLPC support if MGEODE_LXis defined

[Jordan Crouse]

Jeremy Katz wrote:
> On Fri, 2008-11-14 at 10:34 -0700, Jordan Crouse wrote:
> > Pavel Machek wrote:
> > > > > > i've zapped this patch meanwhile:
> > > > > >
> > > > > > 1355c96: x86/olpc: make CONFIG_OLPC dependent on
> > > > > > CONFIG_MGEODE_LX
> > > > > >
> > > > > > because it cripples the ability to run distribution kernels
> > > > > > on the OLPC.
> > > > > OK, I reverted also all relevant changes for cs5535audio
> > > > > driver now. The patches are saved in topic/cs5535audio
> > > > > branch, though.
> > > > >
> > > > > Let's fix OLPC-geode coupling first.
> > > > Hm, I'd really rather prefer this to be upstream.  The patch I
> > > > sent adds no regressions, allows the driver to happily coexist
> > > > with existing stuff, and *does* add support if you configure
> > > > OLPC with MGEODE_LX (generic kernels don't get the additional
> > > > benefits, but those configured specifically for OLPC do).
> > > Yes, but the patch is also not a good way of going forward, so it
> > >  should not be in mainline.
> > For the moment, this is a reasonable intermediate solution.  I
> > think the way forward will involve a great deal more work.
>
> It's the work that should have been done from the beginning.  And
> once code is upstream, the motivation to make things "correct" drops
>
> > The driver does need to be able to twiddle the GPIO pins, and the
> > GPIO pins are unique to the CS5536.  In the past, the CS5536 has
> > been only associated with the Geode, and 99.99% of the kernel users
> > in the world don't need Geode, so it make sense to omit specific
> > code of this nature. As you might expect, I'm not in the "you
> > should be able to run an arbitrary kernel on the XO" crowd.
>
> That's nice.  But if the XO is to be more than just an embedded
> platform working in its own world, then being able to run kernels
> other than from your git tree is extremely important.

I spent way more time with Geode platforms then is probably healthy, and
I will never be convinced that the XO should be anything *but* an
embedded platform working in its own world.  In any other situation the
modern distributions would write off a 500Mhz Pentium era processor as
uninteresting.  I think that anybody really serious about making a
practical XO image will eventually zero in on using a custom kernel (and
a custom libc and a custom compiler).  But this being open source we
shouldn't tell anybody what to do with their own hardware.  But if you
want to know the reasoning behind what the code does today, there it is
in a nutshell.

> > That said, its clear that people are interested in doing so - so a
> >  change needs to be made.  The immediate option is to turn on Geode
> >  support for all x86 kernels, but thats a few extra K in the kernel
> >  binary that nobody needs.  So we'll have to go modular.
>
> Modular is nice, but I'm not 100% sure it's required (see below)...
>
> > I talked to Andres about this yesterday.  The immediately obvious 
> > solution would be to port the Geode GPIO support into the existing
> > GPIO subsystem.
>
> This would be a great first step!
>
> > That is attractive, but it omits some of the other Geode goodness
> > that might be of interest to a "standard kernel" not configured for
> > MGEODE_LX (such as the Geode watchdog timer).  So, I think we
> > should move most of the Geode code into a support module and remove
> > MGEODE_LX checks in most places.  The distributions can build the
> > module and put it in the file system, and serious Geode users such
> > as OLPC can build it in to the kernel binary.
>
> This also sounds like good steps.  One thing to think about is if 
> there's any sort of dmi matching that can be done to autoload if
> things are built modular since distros are trying to get udev loading
> things automatically without any need for magic scripts  And most of
> the clock sources are already not able to be built modular

Well you can speak for your distribution - if you are fine with shipping
the Geode support code in the 32 bit kernel binary, then thats cool, and
its a change we can make immediately.

> > Also, while we are at it, we should be more specific and rename the
> >  hooks from geode_ to cs5536_, since there will soon be a system in
> > the wild with a MIPS processor and a CS5536.
> >
> > But needless to say, this will be a goodly amount of work and churn
> > that will need some heavy testing in the OLPC kernel.  In the
> > meantime, Andres' fix will make the upstream kernel happier.
>
> It is work, but I'm not sure it's huge amounts.  And by doing it 
> *upstream first*, you benefit from the feedback and testing of the
> wider community.  Development of things in a silo is what tends to
> lead to these sorts of threads :/

Lets put it this way - the XO is the trigger for this particular change,
and they will be the largest group of testers.  No matter what happens, 
it will have to go into the XO kernel - if not first, then concurrently 
with upstream.

Jordan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/