Re: [PATCH v2 1/1] ASoC: TWL4030: Add support Voice DAI

[Peter Ujfalusi <peter.ujfalusi@nokia.com>]

On Monday 20 April 2009 13:21:35 ext Joonyoung Shim wrote:
> Add Voice DAI to support the PCM voice interface of the twl4030 codec.
>

This does look good.

>
> +	/* clock inversion */
> +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	case SND_SOC_DAIFMT_IB_NF:
> +		format &= ~(TWL4030_VIF_FORMAT);
> +		break;
> +	case SND_SOC_DAIFMT_NB_IF:
> +		format |= TWL4030_VIF_FORMAT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

I think here it would be better to use:
switch (fmt & (SND_SOC_DAIFMT_FORMAT_MASK |
			 SND_SOC_DAIFMT_INV_MASK)) {
case (SND_SOC_DAIFMT_DSP_A|SND_SOC_DAIFMT_IB_NF):
	format &= ~(TWL4030_VIF_FORMAT);
	break;
case (SND_SOC_DAIFMT_DSP_A|SND_SOC_DAIFMT_NB_IF):
	format |= TWL4030_VIF_FORMAT;
	break;
default:
	return -EINVAL;
}

or:
if ((fm & SND_SOC_DAIFMT_FORMAT_MASK) != SND_SOC_DAIFMT_DSP_A)
	/* Only DSP_A mode is supported for the Voice interface */
	return -EINVAL;

Just rule out any misconfiguration attempt and also act like an interface 
documentation on what format is supported in Voice mode

But even without this, I think this patch looks OK,

---
Acked-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>