Re: [PATCH v2 1/4] ASoC: Add TI tlv320aic32x4 codec support.

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

On Tue, Mar 01, 2011 at 03:02:05PM +0100, Javier Martin wrote:

> +	SOC_DOUBLE_R_TLV("ADC Level Volume", AIC32X4_LADCVOL,
> +			AIC32X4_RADCVOL, 0, 0x28, 0, tlv_step_0_5),
> +	SOC_DOUBLE_R_TLV("PGA Gain Level Volume", AIC32X4_LMICPGAVOL,
> +			AIC32X4_RMICPGAVOL, 0, 0x5f, 0, tlv_step_0_5),

I suspect you don't want to have Gain and Level in there.

> +	SOC_SINGLE("AGC Left Enable Switch", AIC32X4_LAGC1, 7, 1, 0),
> +	SOC_SINGLE("AGC Right Enable Switch", AIC32X4_RAGC1, 7, 1, 0),

A switch is obviously an enable.

> +static const struct aic32x4_configs aic32x4_reg_init[] = {

As I said in reply to your previous posting this looks like it shouldn't
be here.  A few things jump out as suspicious but not everything you're
doing is clear:

> +	{AIC32X4_LDOCTL, AIC32X4_LDOCTLEN},

This looks like it should be dynamically managed at runtime, either via
DAPM or in the bias level functions.

> +	{AIC32X4_CMMODE, AIC32X4_LDOIN_18_36 | AIC32X4_LDOIN2HP},
> +	{AIC32X4_CLKMUX, AIC32X4_PLLCLKIN},
> +	{AIC32X4_IFACE3, AIC32X4_DACMOD2BCLK},
> +	{AIC32X4_DACSETUP,
> +	 AIC32X4_LDAC2LCHN | AIC32X4_RDAC2RCHN | AIC32X4_SSTEP2WCLK},
> +	{AIC32X4_LMICPGANIN, AIC32X4_LMICPGANIN_IN2R_10K},
> +	{AIC32X4_RMICPGANIN, AIC32X4_RMICPGANIN_IN1L_10K},

These look like platform configuration.

> +	{AIC32X4_LMICPGAVOL, 0x00},
> +	{AIC32X4_RMICPGAVOL, 0x00},

These look like volume controls which should be exposed to users.

> +	/* Unmute ADC left and right channels */
> +	{AIC32X4_ADCFGA, 0x00},

This looks like it should be user visible.

> +	/* MICBIAS = 2.075V(CM=0.75V) generated from LDOIN */
> +	{AIC32X4_MICBIAS, 0x68},

This should be configured by the platform.