All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] ASoC: Intel: byt-max98090: Add GPIO ACPI mapping table
@ 2017-06-09 18:38 Andy Shevchenko
  2017-06-09 19:12 ` Dmitry Torokhov
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2017-06-09 18:38 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Nicolas Porcel, alsa-devel, Dmitry Torokhov
  Cc: Andy Shevchenko

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>
---
 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,
 		.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 };
+
+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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] ASoC: Intel: byt-max98090: Add GPIO ACPI mapping table
  2017-06-09 18:38 [PATCH v1] ASoC: Intel: byt-max98090: Add GPIO ACPI mapping table Andy Shevchenko
@ 2017-06-09 19:12 ` Dmitry Torokhov
  2017-06-09 19:19   ` Andy Shevchenko
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2017-06-09 19:12 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Nicolas Porcel, Mark Brown, Liam Girdwood, alsa-devel

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>

> ---
>  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,
>                 .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 };
> +
> +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.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] ASoC: Intel: byt-max98090: Add GPIO ACPI mapping table
  2017-06-09 19:12 ` Dmitry Torokhov
@ 2017-06-09 19:19   ` Andy Shevchenko
  2017-06-10 11:32     ` Andy Shevchenko
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2017-06-09 19:19 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Nicolas Porcel, Mark Brown, Liam Girdwood, alsa-devel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] ASoC: Intel: byt-max98090: Add GPIO ACPI mapping table
  2017-06-09 19:19   ` Andy Shevchenko
@ 2017-06-10 11:32     ` Andy Shevchenko
  0 siblings, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2017-06-10 11:32 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Nicolas Porcel, Mark Brown, Liam Girdwood, alsa-devel

On Fri, 2017-06-09 at 22:19 +0300, Andy Shevchenko wrote:
> 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.


I have to check the device node I'm attaching the mapping table to.

On deeper glance it looks now wrong (attaching table to the card
platform device when it should be codec's one?). 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-06-10 11:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-09 18:38 [PATCH v1] ASoC: Intel: byt-max98090: Add GPIO ACPI mapping table Andy Shevchenko
2017-06-09 19:12 ` Dmitry Torokhov
2017-06-09 19:19   ` Andy Shevchenko
2017-06-10 11:32     ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.