[alsa-devel] [PATCH] ASoC: nau8825: Add driver for headset chip Nuvoton 8825

[Mark Brown]

On Mon, Aug 03, 2015 at 08:13:40PM -0700, Anatol Pomozov wrote:
> On Fri, Jul 31, 2015 at 11:27 AM, Mark Brown <broonie at kernel.org> wrote:
> > On Mon, Jul 27, 2015 at 04:13:57PM -0700, Anatol Pomozov wrote:

> >> +     /* Setup SAR ADC */
> >> +     regmap_write(regmap, NAU8825_REG_SAR_CTRL, 0x0280);

> > Lots of magic numbers in these things...

> These init config settings recommended by Nuvoton engineers. I found
> that chip requires quite a lot of initial preconfiguration to get it
> into operating state.

Random register write lists from the vendor for documented registers are
not generally a good thing to rely on too much, they frequently set up
use case specific things in the middle of other perhaps useful changes.

> > some of them I can guess
> > what's going on but this one is a bit obscure, perhaps it should be user
> > controllable?

> This setting configures SARADC (ADC for sensing button presses). It
> sets sample rate, series resistor and ADC voltage range. It does not
> look like they need to be user-controllable.

Those sound like they may well be system dependent, the sample rate
will most likely feed back into the sensitivity of the detection (how
long the button needs to be pressed for and so on) and the other bits
sound like they could change depending on the configuration of the
accessory.

> >> +     /* Setup ADC x128 OSR */Moved to
> >> +     regmap_write(regmap, NAU8825_REG_ADC_RATE, 0x0002);
> >> +     /* Setup DAC x128 OSR */
> >> +     regmap_write(regmap, NAU8825_REG_DAC_CTRL1, 0x0082);

> > I'd expect this to be user controllable.

> The oversampling configuration is important for chip audio quality.
> There is audible hissing without these settings.

> My understanding that all users need to set these values and it is
> better to move it to the driver initialization sequence.

That sounds like spectacularly poor quality of implementation, but even
if there's a noticeable reduction in audio quality the control should
still be given to the user, they may have an application where power is
much more important than audio quality (eg, if they are doing some form
of recognition on the audio then all they need is something good enough
for their algorithm).

> > I'd expect any initial register initialistion to happen here (if only so
> > we save power until the card registers).

> Moved most of the initialization here. The only part is left in
> codec_probe() is interruption initialization via I2S master mode
> toggling. That toggling trick depends on MCLK signal to initialize
> interruption block correctly. In our case (a tegra SoC device) MCLK is
> initialized later at audio platform driver probe.

If you need to control MCLK you should be using the clock API to get
MCLK which you can do in the driver model probe.  Relying on the machine
driver to control MCLK for you in this way is at best fragile.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20150804/12469c60/attachment.sig>