| Message ID | 20170609183801.43892-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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. >
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?).
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);
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(-)