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@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.