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@xxxxxxxxx>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
