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

[Takashi Iwai <tiwai@suse.de>]

At Fri, 14 Nov 2008 16:10:35 -0500,
Andres Salomon wrote:
> 
> On Fri, 14 Nov 2008 13:45:50 -0500
> Jeremy Katz <katzj@redhat.com> 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
> 
> Wow.  You have it so backwards, it's not even funny.
> 
> When stuff is upstream, there is pressure from lots of people to make
> things "correct".  When stuff isn't upstream, there's very little pressure;
> that's how the ALSA stuff has managed to sit outside of the upstream
> kernel for ages.  It's a tiny bit of code, easy to port to new kernels,
> and therefore can happily be forgotten about in an out-of-tree kernel.
> 
> You say "it's work that should have been done from the beginning", and
> yet this is the first time that I've heard anyone request it.  If
> the geode code had remained out of mainline, I wouldn't have heard the
> request at all.
> 
> I *really* don't want this to turn into OFW/promfs mess; attempts at
> mainline inclusion fail, relevant parties who we contact ignore us
> regarding the "correct" way forward, and therefore things sit outside
> of mainline for ages with no progress whatsoever.
> 
> 
> [...]
> > > 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 :/
> > 
> 
> It is huge amounts, because it's API work.  If we were just twiddling
> hardware bits, that would be easy.  However, we're trying to come up
> with an interface that works not only with existing upstream drivers, but
> also the remaining out-of-mainline drivers that we maintain.  I also
> expressed interest in perhaps using the generic GPIO API for geode,
> but that's something that requires a good deal of thinking about.  I
> don't want to rush through it just to get the ALSA driver upstream.
> 
> The DCON driver has not gone upstream exactly because the GPIO API
> does not work well for it right now (and out of tree, we were banging
> on GPIO pins directly).  Obviously, any changes to the API should also
> take the DCON into account; I'd rather fix this all the *right* way
> than to just write a quick patch that munges the API to make you happy,
> and then have to have even more code churn later to completely redo
> the API.

Well, the GPIO API fix is still not in clear sight, I took back
cs5535audio patches again to sound tree with the build workaround
like Andres posted (but with a proper fix of ifdef) for 2.6.29.
OLPC-dependent part won't be compiled in unless you have
CONFIG_MGEODE_LX=y, but it's still better than now.

This is really a workaround, and I hope the GPIO stuff will be
fixed in arch/x86 in a better way...


thanks,

Takashi