Re: [PATCH - BT and FM audio for zoom2 1/2] Add clock-only codec to provide McBSP clock source.

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

On Monday 03 August 2009 07:29:14 ext sean.mcneil@ti.com wrote:
> From: Sean McNeil <sean.mcneil@ti.com>
>
>
> Signed-off-by: Sean McNeil <sean.mcneil@ti.com>

I'm not sure what is the rationale behind..
Can you explain what are you trying to achieve with these changes?
Also, it would be better if you split the patch to a series with smaller patches 
(and write some explanation what you are doing and why).

As Mark already pointed out, please use the latest code for basing your patches 
(see the first comment below).

Few comments:

>         0x00, /* REG_AVTXL2PGA          (0xC)   */
>         0x00, /* REG_AVTXR2PGA          (0xD)   */
>         0x01, /* REG_AUDIO_IF           (0xE)   */
> -       0x00, /* REG_VOICE_IF           (0xF)   */
> +       0x04, /* REG_VOICE_IF           (0xF)   */

The VOICE_IF:VIF_TRI_EN (and the AUDIO_IF:AIF_TRI_EN) bit has been already used 
in the latest code (see twl4030_set_tristate and twl4030_voice_set_tristate 
functions).

>  struct snd_soc_dai twl4030_dai[] = {
>  {
>         .name = "twl4030",
>         .playback = {
>                 .stream_name = "HiFi Playback",
> -               .channels_min = 2,
> +               .channels_min = 1,

The HiFi (Audio interface in twl terms) either supports 2 or 4 channels, it does 
not support mono audio.

>                 .channels_max = 4,
>                 .rates = TWL4030_RATES | SNDRV_PCM_RATE_96000,
> -               .formats = TWL4030_FORMATS,},
> +               .formats = TWL4030_FORMATS,
> +       },
>         .capture = {
>                 .stream_name = "Capture",
> -               .channels_min = 2,
> +               .channels_min = 1,

Same here, mono is not supported by the codec.

>                 .channels_max = 4,
>                 .rates = TWL4030_RATES,
> -               .formats = TWL4030_FORMATS,},
> +               .formats = TWL4030_FORMATS,
> +       },
>         .ops = &twl4030_dai_ops,
>  },
>  {
> @@ -2092,17 +2356,37 @@ struct snd_soc_dai twl4030_dai[] = {
>         .playback = {
>                 .stream_name = "Voice Playback",
>                 .channels_min = 1,
> -               .channels_max = 1,
> +               .channels_max = 2,

The Voice interface only supports mono playback. Stereo is only supported on the 
capture path.

>                 .rates = SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000,
> -               .formats = SNDRV_PCM_FMTBIT_S16_LE,},
> +               .formats = SNDRV_PCM_FMTBIT_S16_LE,
> +       },

-- 
Péter