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

[Stephen Warren]

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.

-- 
nvpublic