Re: [PATCH] ASoC: add RT5640 CODEC driver

[Mark Brown <broonie@kernel.org>]

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

On Tue, Apr 16, 2013 at 04:29:40PM -0600, Stephen Warren wrote:

> a) There are many different VDD inputs to the chip. I imagine that some
> of these only need to be enabled/active under certain conditions. For
> example, perhaps the SPKVDD or MICVDD inputs are only required if
> actively using the speakers or microphones, and could be disabled at
> other times? If so, all of these "optional" or "part-time" VDD inputs
> should be represented as regulators, and retrieved/manipulated using
> Linux APIs such as regulator_get(), regulator_enable(), etc.

There's framework support for regulator supplies in ASoC so you just
need to specify the name and the core will do the rest.

> b) Some VDD inputs are optional; for example, either I believe that a
> board should either provide DCVDD, or provide LDO1_IN and assert the
> LDO1_EN input signal. Similarly, I think that either MICVDD should be
> provided, or SPKVDDL be provided, coupled with LDO2_EN register bit
> being set. This may require the RT5640 driver to provide two regulator
> objects (LDO1, LDO2), which the board file or device tree can connect
> back to the DCVDD and MICVDD inputs if appropriate for the board's HW
> configuration. In the case where LDO1 is used, a separate fixed
> regulator with GPIO should be used to control the LDO1_EN GPIO input.

That said if boards generally don't use external supplies and use the
built in regulators then it's probably best to at least have the driver
assume that by default.

> > Bard: You are right. The mic type should be defined according to circuit.
> >           And it should be change in run time.
> >          So, should we just remove this control in codec driver?

> Mark's point is that the driver should either/both accept some platform
> data structure, or parse device tree, to configure the input pins. I
> don't think completely removing all mechanisms to configure this would
> be a good idea, since then how would the board/device-tree author
> configure the CODEC?

Indeed, there definitely should be configuration - however what I was
commenting on there was a user-visible ALSA control which should
definitely go.

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

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