Re: [PATCH 13/16] ASoC: codecs: Add AB8500 codec-driver

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

[-- Attachment #1.1: Type: text/plain, Size: 2585 bytes --]

On Fri, Mar 16, 2012 at 02:09:38PM +0100, Ola Lilja wrote:
> On 03/15/2012 04:29 PM, Mark Brown wrote:

Please fix the word wrapping in your mail client...

> > If the hardware has no control the idiomatic thing would be to have a
> > pin switch in the machine driver.

> We need to break chains on very specific positions to be able to
> affect only the parts intended. We also have switches (e.g. loopback)
> that is not belonging to a specific pin but is a brigde-switch in the
> middle of two chains, thus I don't think we can use what you suggest.

But if you have a control that actually does something then surely there
should be some interaction with the chip when it is changed?

> The only solution I've found is to use the enum_virt, but if there is
> a possibility to have a switch_virt I would be able to use that.

One of the nice things about open source code is that it's always
possible to extend the core if required.

> Yes, if it were only regulator (and clock) that was needed in our extra
> reference-counted power-functions, it would be fine. But vibra needs to be able
> to turn on the power of the audio-part of AB8500 (our codec) before it can use
> the vibra-functionality. I could move our the reg/clock and assume that the
> vibra-driver does this itself and only have the codec-power-register shared
> between the external interface and the DAPM.

As I said previously there's other drivers with the same setup which
manage to integrate fairly easily without breaking the device model - do
what they do with something like an MFD or just embed a trivial input
driver in the CODEC for the vibra if it's small enough.

> (and earlier) to be in sync with mainline-kernel. However, if we get this driver
> mainlined we can use all input to push harder for our next generation of
> platforms. This driver is for a project that has nearly finished its execution,
> but we would like to mainline this first before we go ahead with new drivers.

Equally well, if it gets mainline without actually meeting mainline
standards that's sending the wrong message :) 

I'd suggest at least adding regulator support into the CODEC driver so
it's managing its own power when required, there's no reason for
anything external to be doing that.  I expect if you bring things into
line with the device model the code will end up being a lot smaller and
clearer and this won't be needed, it feels like a large part of why
you're having to open code all this stuff is that other bits of the code
stepped outside the frameworks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]