[alsa-devel] [PATCH v2] ASoC: Tegra machine ASoC driver for boards using ALC5332 codec

[Leon Romanovsky]

On Mon, Dec 12, 2011 at 22:20, Stephen Warren <swarren at nvidia.com> wrote:
> Leon Romanovsky wrote at Monday, December 12, 2011 12:46 PM:
>> At this stage only Toshiba AC100/Dynabook supported.
>
>> +static int tegra_alc5632_asoc_hw_params(struct snd_pcm_substream *substream,
>> +                                     struct snd_pcm_hw_params *params)
> ...
>> +     srate = params_rate(params);
>> +     switch (srate) {
>> +     case 64000:
>> +     case 88200:
>> +     case 96000:
>> +             mclk = 128 * srate;
>> +             break;
>> +     default:
>> +             mclk = 512 * srate;
>> +             break;
>> +     }
>> +     /* FIXME: Codec only requires >= 3MHz if OSR==0 */
>> +     while (mclk < 6000000)
>> +             mclk *= 2;
>
> I think you should replace all that with:
>
> mclk = 512 * srate;
>
> The reason being:
>
> * The codec only supports up to 48KHz as far as I can tell, thus making
> the special-case in the switch statement never fire, and irrelevant.
>
> * The codec wants 512Fs, not min(6MHz, 512Fs) as far as I can tell, so
> the extra while loop at the end is going to set the wrong rate for 8KHz
> and 11.025KHz.
You are right, I'll check it at the spec.

>> +static struct snd_soc_jack_gpio tegra_alc5632_hs_jack_gpio = {
>> +     .name = "Headset Detect",
>> +     .report = SND_JACK_HEADSET,
>> +     .debounce_time = 150,
>> +};
> ...
>> +static int tegra_alc5632_asoc_init(struct snd_soc_pcm_runtime *rtd)
>> +{
> ...
>> +     snd_soc_jack_add_gpios(&tegra_alc5632_hs_jack,
>> +                     1, &tegra_alc5632_hs_jack_gpio);
>
> Since tegra_alc5632_hs_jack_gpio is never set, doesn't this end up using
> GPIO 0 as the headset detect, which most likely is something completely
> unrelated to audio.
This is initial support of board file, at the coming patch set, I'll
update board-paz00-pinmux.c [1] with audio related pins and I'll add
platform device support.
I think It is easy for review in this way.

[1] http://git.kernel.org/?p=linux/kernel/git/olof/tegra.git;a=blob;f=arch/arm/mach-tegra/board-paz00-pinmux.c;h=126892cfddce1ce222b0251f1bac972cb16b6db1;hb=refs/heads/for-next

> Other than those two things, this looks fine to me.

>
> --
> nvpublic
>



-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon at leon.nu