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

[Leon Romanovsky]

On Tue, Dec 13, 2011 at 21:03, Stephen Warren <swarren at nvidia.com> wrote:
> Leon Romanovsky wrote at Tuesday, December 13, 2011 11:27 AM:
>> On Tue, Dec 13, 2011 at 19:23, Stephen Warren <swarren at nvidia.com> wrote:
>> >
>> > Leon Romanovsky wrote at Monday, December 12, 2011 11:36 PM:
>> > > 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 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.
>> >
>> > I'm fine adding the feature later. But, I think you shouldn't call
>> > snd_soc_jack_add_gpios() until the platform data is there, or you'll
>> > end up enabling the feature already, but using the wrong GPIO.
>>
>> Does it matter ? Anyway, this file is useless without proper audio
>> support, and currently only paz00 will be using this file.
>
> Yes, I think so.
>
> The issue is that by calling snd_soc_jack_add_gpios(), ASoC will internally
> call gpio_request() and request_any_context_irq() for GPIO 0, which:
>
> a) Will prevent it from being used as a GPIO by any other code.
>
> b) Will configure the GPIO HW to make this pin an input, such that if
> the pin is enabled as a GPIO, might stop Tegra driving the pin if it
> was previously configured as an output.
>
> c) If the GPIO is attached to some random piece of HW that toggles the
> line, this could cause an interrupt storm, or at least bogus jack detect
> events.
>
> Just removing that one call and the unused GPIO structure would solve
> all this; it can easily be added back when adding the platform data
> support.
Thanks for explanation, I'll try to resend the patch till the weekends.

>
> --
> nvpublic
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



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