Re: [PATCH] sst: Intel SST audio driver

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Sun, Oct 03, 2010 at 11:22:44AM +0100, Alan Cox wrote:
> > Having things as a series does make things easier for review, reducing
> > reviewer fatigue if nothing else.  I've given this a bit of a once
> > over below but not a deep dive - splitting the series would really
> > help with reviewability.

> It really wouldn't help in this case for most people I think. You end up
> with what there is in the original which is a series of patches for old
> code all quite large and just a big one chopped into chunks, followed
> by a series of updates that fix half the bugs you found reviewing the
> first big splat.

IIRC at least one version had a split where the ALSA integration stuff
was separated out from the underlying DSP interface code - that was
pretty helpful since it helps focus on the ALSA specifics.

> I did think about it, but what history we have is fairly basic because
> it was basically  written then fired in my direction to help sort out

Yeah, I wasn't thinking about history so much as about helping break
down the areas covered for comprehensibility.

> > To be honest all this stuff looks sufficiently generic that it might
> > be worth considering factoring out an abstraction which can be used by
> > other offload engines - having a standard kernel API for them would
> > be a real win.  That's just a nice to have, though.

> Agreed, although gstreamer is pretty good at that it would save work if
> it can be partly generic. It's not trivial however because the offload

Plus the fact that not everyone is using gstreamer at the application
level :/

> interface with suitable firmware loaded does things other than PCM and
> you have very firmware specific interfaces for configuring those.

We ought to be able to come up with something for the core streaming
stuff, though.  Like I say, it's just a nice to have though.

> So are you happy for it to go into staging. I'm hoping that way at
> least all the changes will be visible and trackable. I'll start by
> sending GregKH a follow up patch which is a TODO list summary based on
> your feedback

Happy is a strong term but this looks like exactly the sort of stuff
staging is there for so yes, it should go in.  Probably best to look for
feedback from at least Takashi and/or Jaroslav as well, but obviously we
can update the TODO later.

I do have some nervousness about the concept of staging for embedded
stuff since I worry that inclusion in staging can send the wrong message
to vendors but that's a completely separate issue to this driver.