Re: [PATCH] ASoC: tlv320aic3x: add AGC, MIC_BIAS

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

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

On Tue, Jun 26, 2012 at 03:04:27PM +0200, Prchal Jiří wrote:

>  	SOC_DOUBLE_R("AGC Switch", LAGC_CTRL_A, RAGC_CTRL_A, 7, 0x01, 0),
> +	SOC_ENUM("Left AGC Target level", aic3x_enum[LAGC_LEV_ENUM]),
> +	SOC_ENUM("Right AGC Target level", aic3x_enum[RAGC_LEV_ENUM]),
> +	SOC_ENUM("Left AGC Attack time", aic3x_enum[LAGC_ATT_ENUM]),
> +	SOC_ENUM("Right AGC Attack time", aic3x_enum[RAGC_ATT_ENUM]),
> +	SOC_ENUM("Left AGC Decay time", aic3x_enum[LAGC_DEC_ENUM]),
> +	SOC_ENUM("Right AGC Decay time", aic3x_enum[RAGC_DEC_ENUM]),

Please move everything out of the big array or if you insist on having
the array then use the indexes in the assignments.  Otherwise the code
is just error prone and hard to read.

> +
> +	SOC_ENUM("Mic Bias", aic3x_enum[MIC_BIAS_ENUM]),

No, this should be platform data or done with a callback from the
machine driver.  It's very unlikely that it's sensible to vary at
runtime without coordination with other driver code.

> +	/* set to avoid artifacts on the audio output during power-on/off */
> +	snd_soc_write(codec, AIC3X_HEADSET_DETECT_CTRL_B, 0x80); /*ac-coupled*/
> +	snd_soc_write(codec, HPOUT_POP_REDUCTION, 0x4e); /* 10 + 4 ms, reference*/
> +
> +	/* short circuit protection */
> +	snd_soc_write(codec, HPRCOM_CFG, 0x04);
> +

This appears to be unrelated to the change.  What is it for?

>  	/* DAC to Mono Line Out default volume and route to Output mixer */
> -	snd_soc_write(codec, DACL1_2_MONOLOPM_VOL, DEFAULT_VOL | ROUTE_ON);
> -	snd_soc_write(codec, DACR1_2_MONOLOPM_VOL, DEFAULT_VOL | ROUTE_ON);
> +	/* mix both channels with -6dB level */
> +	snd_soc_write(codec, DACL1_2_MONOLOPM_VOL, (DEFAULT_VOL + 12) | ROUTE_ON);
> +	snd_soc_write(codec, DACR1_2_MONOLOPM_VOL, (DEFAULT_VOL + 12) | ROUTE_ON);

This is also an unrelated change to the chip defaults (which we were
overriding anyway when we shouldn't be...).  Code like this should just
be removed completely.

> +	/* AGC to -10dB, 20 / 500ms, no clip stepping, noise gate -90dB, hysteresis 3dB*/
> +	snd_soc_write(codec, LAGC_CTRL_A, 0x2f);
> +	snd_soc_write(codec, RAGC_CTRL_A, 0x2f);
> +	snd_soc_write(codec, LAGC_CTRL_C, 0xbe);
> +	snd_soc_write(codec, RAGC_CTRL_C, 0xbe);

Again, just use the chip defaults and let the user override.

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

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