[lkml]   [2012]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 2/4] Add Wolfson Microelectronics WM8766 codec ALSA driver
On Tue, Apr 17, 2012 at 06:35:32PM +0200, Ondrej Zary wrote:
> On Tuesday 17 April 2012 16:59:29 Mark Brown wrote:

> > No, this should be supported in ASoC - anything adding new code in
> > sound/i2c is *deeply* suspicious.

> I agree that sound/i2c is wrong. I worked on tea575x-tuner before which lives
> in this directory too - but it's neither an I2C device nor a sound chip...
> There should probably be something like sound/codecs instead that could be
> used by any sound card drivers.

That's what sound/soc/codecs is - anything with sufficiently split up
hardware to have distinct CODECs ought to be within that framework.
"soc" in this case to a large extent just means "mix and match CODEC and
CPU interface".

> > > +struct snd_wm8766_ops {
> > > + void (*write)(struct snd_wm8766 *wm, u16 addr, u16 data);
> > > +};

> > You should in general use regmap rather than open coding register I/O
> > for I2C devices.

> regmap seems like an overkill here. It requires the i2c bus to be registered
> in the kernel i2c subsystem (which is not in the case of ice1712).

Oh, that's fail. I did notice that patch 4 looked to be open coding
rather generic stuff - I guess this is why.

> > I've not yet looked at the rest of the code but this looks awfully like
> > you've just invented a minimal version of the ASoC interfaces...

> The _set functions are just simple register writes - the caller needs to know
> what to write there (only _set_if is used by psc724).

You can obviously do stuff as hard coded register write sequences, many
of which will be very short.
[unhandled content-type:application/pgp-signature]
 \ /
  Last update: 2012-04-17 18:57    [from the cache]
©2003-2011 Jasper Spaans