Re: [PATCH v1] ASoC: Intel: byt-max98090: Add GPIO ACPI mapping table

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, 2017-06-09 at 12:12 -0700, Dmitry Torokhov wrote:
> On Fri, Jun 9, 2017 at 11:38 AM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > In order to make GPIO ACPI library stricter prepare users of
> > gpiod_get_index() to correctly behave when there no mapping is
> > provided by firmware.
> > 
> > Here we add explicit mapping between _CRS GpioIo() resources and
> > their names used in the driver.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> This is a bit more verbose than simply using the same connection name,
> but I guess will provide better diagnostic to userspace as we will
> have distinct GPIO names.
> 
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks!

But please test it.

See also below comment, just in case you have time to test that as well.

> 
> > ---
> >  sound/soc/intel/boards/byt-max98090.c | 22 ++++++++++++++++++----
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/sound/soc/intel/boards/byt-max98090.c
> > b/sound/soc/intel/boards/byt-max98090.c
> > index d9f81b8d915d..62f5a09aa613 100644
> > --- a/sound/soc/intel/boards/byt-max98090.c
> > +++ b/sound/soc/intel/boards/byt-max98090.c
> > @@ -67,20 +67,29 @@ static struct snd_soc_jack_pin hs_jack_pins[] =
> > {
> > 
> >  static struct snd_soc_jack_gpio hs_jack_gpios[] = {
> >         {
> > -               .name           = "hp-gpio",
> > +               .name           = "hp",
> >                 .idx            = 0,
> >                 .report         = SND_JACK_HEADPHONE |
> > SND_JACK_LINEOUT,
> >                 .debounce_time  = 200,
> >         },
> >         {
> > -               .name           = "mic-gpio",
> > -               .idx            = 1,
> > +               .name           = "mic",
> > +               .idx            = 0,

> >                 .invert         = 1,

I dunno how this one is used inside ASoC core, but if we change it to 0
and...

> >                 .report         = SND_JACK_MICROPHONE,
> >                 .debounce_time  = 200,
> >         },
> >  };
> > 
> > +static const struct acpi_gpio_params hp_gpios = { 0, 0, false };

> > +static const struct acpi_gpio_params mic_gpios = { 1, 0, false };

...change here to true, would it work?

> > +
> > +static const struct acpi_gpio_mapping acpi_byt_max98090_gpios[] = {
> > +       { "hp-gpios", &hp_gpios, 1 },
> > +       { "mic-gpios", &mic_gpios, 1 },
> > +       {},
> > +};
> > +
> >  static int byt_max98090_init(struct snd_soc_pcm_runtime *runtime)
> >  {
> >         int ret;
> > @@ -140,8 +149,9 @@ static struct snd_soc_card byt_max98090_card = {
> > 
> >  static int byt_max98090_probe(struct platform_device *pdev)
> >  {
> > -       int ret_val = 0;
> > +       struct device *dev = &pdev->dev;
> >         struct byt_max98090_private *priv;
> > +       int ret_val;
> > 
> >         priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
> >         if (!priv) {
> > @@ -149,6 +159,10 @@ static int byt_max98090_probe(struct
> > platform_device *pdev)
> >                 return -ENOMEM;
> >         }
> > 
> > +       ret_val = devm_acpi_dev_add_driver_gpios(dev,
> > acpi_byt_max98090_gpios);
> > +       if (ret_val)
> > +               dev_dbg(dev, "Unable to add GPIO mapping table\n");
> > +
> >         byt_max98090_card.dev = &pdev->dev;
> >         snd_soc_card_set_drvdata(&byt_max98090_card, priv);
> >         ret_val = devm_snd_soc_register_card(&pdev->dev,
> > &byt_max98090_card);
> > --
> > 2.11.0
> > 
> 
> Thanks.
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel