linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal
@ 2021-08-16  9:38 Hans de Goede
  2021-08-16 10:25 ` Mika Westerberg
  2021-09-21 18:28 ` Linus Walleij
  0 siblings, 2 replies; 8+ messages in thread
From: Hans de Goede @ 2021-08-16  9:38 UTC (permalink / raw)
  To: Mika Westerberg, Andy Shevchenko, Linus Walleij
  Cc: Hans de Goede, linux-gpio, linux-acpi

Commit 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
made the gpiolib-acpi code call gpio_set_debounce_timeout() when requesting
GPIOs.

This in itself is fine, but it also made gpio_set_debounce_timeout()
errors fatal, causing the requesting of the GPIO to fail. This is causing
regressions. E.g. on a HP ElitePad 1000 G2 various _AEI specified GPIO
ACPI event sources specify a debouncy timeout of 20 ms, but the
pinctrl-baytrail.c only supports certain fixed values, the closest
ones being 12 or 14 ms and pinctrl-baytrail.c responds with -EINVAL
when specified a value which is not one of the fixed values.

This is causing the acpi_request_own_gpiod() call to fail for 3
ACPI event sources on the HP ElitePad 1000 G2, which in turn is causing
e.g. the battery charging vs discharging status to never get updated,
even though a charger has been plugged-in or unplugged.

Make gpio_set_debounce_timeout() errors non fatal, warning about the
failure instead, to fix this regression.

Note we should probably also fix various pinctrl drivers to just
pick the first bigger discrete value rather then returning -EINVAL but
this will need to be done on a per driver basis, where as this fix
at least gets us back to where things were before and thus restores
functionality on devices where this was lost due to
gpio_set_debounce_timeout() errors.

Fixes: 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpio/gpiolib-acpi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 411525ac4cc4..47712b6903b5 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -313,9 +313,11 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
 
 	ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
 	if (ret)
-		gpiochip_free_own_desc(desc);
+		dev_warn(chip->parent,
+			 "Failed to set debounce-timeout for pin 0x%04X, err %d\n",
+			 pin, ret);
 
-	return ret ? ERR_PTR(ret) : desc;
+	return desc;
 }
 
 static bool acpi_gpio_in_ignore_list(const char *controller_in, int pin_in)
-- 
2.31.1


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

* Re: [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal
  2021-08-16  9:38 [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal Hans de Goede
@ 2021-08-16 10:25 ` Mika Westerberg
  2021-08-16 10:39   ` Hans de Goede
  2021-09-21 18:28 ` Linus Walleij
  1 sibling, 1 reply; 8+ messages in thread
From: Mika Westerberg @ 2021-08-16 10:25 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Andy Shevchenko, Linus Walleij, linux-gpio, linux-acpi

On Mon, Aug 16, 2021 at 11:38:56AM +0200, Hans de Goede wrote:
> Commit 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> made the gpiolib-acpi code call gpio_set_debounce_timeout() when requesting
> GPIOs.
> 
> This in itself is fine, but it also made gpio_set_debounce_timeout()
> errors fatal, causing the requesting of the GPIO to fail. This is causing
> regressions. E.g. on a HP ElitePad 1000 G2 various _AEI specified GPIO
> ACPI event sources specify a debouncy timeout of 20 ms, but the
> pinctrl-baytrail.c only supports certain fixed values, the closest
> ones being 12 or 14 ms and pinctrl-baytrail.c responds with -EINVAL
> when specified a value which is not one of the fixed values.
> 
> This is causing the acpi_request_own_gpiod() call to fail for 3
> ACPI event sources on the HP ElitePad 1000 G2, which in turn is causing
> e.g. the battery charging vs discharging status to never get updated,
> even though a charger has been plugged-in or unplugged.
> 
> Make gpio_set_debounce_timeout() errors non fatal, warning about the
> failure instead, to fix this regression.
> 
> Note we should probably also fix various pinctrl drivers to just
> pick the first bigger discrete value rather then returning -EINVAL but
> this will need to be done on a per driver basis, where as this fix
> at least gets us back to where things were before and thus restores
> functionality on devices where this was lost due to
> gpio_set_debounce_timeout() errors.
> 
> Fixes: 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

* Re: [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal
  2021-08-16 10:25 ` Mika Westerberg
@ 2021-08-16 10:39   ` Hans de Goede
  0 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2021-08-16 10:39 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: Andy Shevchenko, Linus Walleij, linux-gpio, linux-acpi

Hi,

On 8/16/21 12:25 PM, Mika Westerberg wrote:
> On Mon, Aug 16, 2021 at 11:38:56AM +0200, Hans de Goede wrote:
>> Commit 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
>> made the gpiolib-acpi code call gpio_set_debounce_timeout() when requesting
>> GPIOs.
>>
>> This in itself is fine, but it also made gpio_set_debounce_timeout()
>> errors fatal, causing the requesting of the GPIO to fail. This is causing
>> regressions. E.g. on a HP ElitePad 1000 G2 various _AEI specified GPIO
>> ACPI event sources specify a debouncy timeout of 20 ms, but the
>> pinctrl-baytrail.c only supports certain fixed values, the closest
>> ones being 12 or 14 ms and pinctrl-baytrail.c responds with -EINVAL

Type 14 should be 24.

>> when specified a value which is not one of the fixed values.
>>
>> This is causing the acpi_request_own_gpiod() call to fail for 3
>> ACPI event sources on the HP ElitePad 1000 G2, which in turn is causing
>> e.g. the battery charging vs discharging status to never get updated,
>> even though a charger has been plugged-in or unplugged.
>>
>> Make gpio_set_debounce_timeout() errors non fatal, warning about the
>> failure instead, to fix this regression.
>>
>> Note we should probably also fix various pinctrl drivers to just
>> pick the first bigger discrete value rather then returning -EINVAL but
>> this will need to be done on a per driver basis, where as this fix
>> at least gets us back to where things were before and thus restores
>> functionality on devices where this was lost due to
>> gpio_set_debounce_timeout() errors.
>>
>> Fixes: 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks, I'll send a v2 with the typo fixed and your Reviewed-by added.

Regards,

Hans


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

* Re: [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal
  2021-08-16  9:38 [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal Hans de Goede
  2021-08-16 10:25 ` Mika Westerberg
@ 2021-09-21 18:28 ` Linus Walleij
  2021-09-21 18:42   ` Hans de Goede
  1 sibling, 1 reply; 8+ messages in thread
From: Linus Walleij @ 2021-09-21 18:28 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mika Westerberg, Andy Shevchenko, open list:GPIO SUBSYSTEM,
	ACPI Devel Maling List

On Mon, Aug 16, 2021 at 11:39 AM Hans de Goede <hdegoede@redhat.com> wrote:

> Commit 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> made the gpiolib-acpi code call gpio_set_debounce_timeout() when requesting
> GPIOs.
>
> This in itself is fine, but it also made gpio_set_debounce_timeout()
> errors fatal, causing the requesting of the GPIO to fail. This is causing
> regressions. E.g. on a HP ElitePad 1000 G2 various _AEI specified GPIO
> ACPI event sources specify a debouncy timeout of 20 ms, but the
> pinctrl-baytrail.c only supports certain fixed values, the closest
> ones being 12 or 14 ms and pinctrl-baytrail.c responds with -EINVAL
> when specified a value which is not one of the fixed values.
>
> This is causing the acpi_request_own_gpiod() call to fail for 3
> ACPI event sources on the HP ElitePad 1000 G2, which in turn is causing
> e.g. the battery charging vs discharging status to never get updated,
> even though a charger has been plugged-in or unplugged.
>
> Make gpio_set_debounce_timeout() errors non fatal, warning about the
> failure instead, to fix this regression.
>
> Note we should probably also fix various pinctrl drivers to just
> pick the first bigger discrete value rather then returning -EINVAL but
> this will need to be done on a per driver basis, where as this fix
> at least gets us back to where things were before and thus restores
> functionality on devices where this was lost due to
> gpio_set_debounce_timeout() errors.
>
> Fixes: 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Bartosz will pick this up I think, I'm a bit off duty with GPIO right now.

Yours,
Linus Walleij

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

* Re: [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal
  2021-09-21 18:28 ` Linus Walleij
@ 2021-09-21 18:42   ` Hans de Goede
  2021-09-22  9:17     ` Bartosz Golaszewski
  0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2021-09-21 18:42 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Mika Westerberg, Andy Shevchenko, open list:GPIO SUBSYSTEM,
	ACPI Devel Maling List

Hi,

On 9/21/21 8:28 PM, Linus Walleij wrote:
> On Mon, Aug 16, 2021 at 11:39 AM Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> Commit 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
>> made the gpiolib-acpi code call gpio_set_debounce_timeout() when requesting
>> GPIOs.
>>
>> This in itself is fine, but it also made gpio_set_debounce_timeout()
>> errors fatal, causing the requesting of the GPIO to fail. This is causing
>> regressions. E.g. on a HP ElitePad 1000 G2 various _AEI specified GPIO
>> ACPI event sources specify a debouncy timeout of 20 ms, but the
>> pinctrl-baytrail.c only supports certain fixed values, the closest
>> ones being 12 or 14 ms and pinctrl-baytrail.c responds with -EINVAL
>> when specified a value which is not one of the fixed values.
>>
>> This is causing the acpi_request_own_gpiod() call to fail for 3
>> ACPI event sources on the HP ElitePad 1000 G2, which in turn is causing
>> e.g. the battery charging vs discharging status to never get updated,
>> even though a charger has been plugged-in or unplugged.
>>
>> Make gpio_set_debounce_timeout() errors non fatal, warning about the
>> failure instead, to fix this regression.
>>
>> Note we should probably also fix various pinctrl drivers to just
>> pick the first bigger discrete value rather then returning -EINVAL but
>> this will need to be done on a per driver basis, where as this fix
>> at least gets us back to where things were before and thus restores
>> functionality on devices where this was lost due to
>> gpio_set_debounce_timeout() errors.
>>
>> Fixes: 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Bartosz will pick this up I think, I'm a bit off duty with GPIO right now.

Added Bartosz to the To: list, to make sure that he actually
sees this. I somehow did not add him in the To/Cc when sending out
the patch, sorry about that.

Regards,

Hans


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

* Re: [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal
  2021-09-21 18:42   ` Hans de Goede
@ 2021-09-22  9:17     ` Bartosz Golaszewski
  2021-09-22 10:55       ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2021-09-22  9:17 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Linus Walleij, Mika Westerberg, Andy Shevchenko,
	open list:GPIO SUBSYSTEM, ACPI Devel Maling List

On Tue, Sep 21, 2021 at 8:42 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 9/21/21 8:28 PM, Linus Walleij wrote:
> > On Mon, Aug 16, 2021 at 11:39 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> >> Commit 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> >> made the gpiolib-acpi code call gpio_set_debounce_timeout() when requesting
> >> GPIOs.
> >>
> >> This in itself is fine, but it also made gpio_set_debounce_timeout()
> >> errors fatal, causing the requesting of the GPIO to fail. This is causing
> >> regressions. E.g. on a HP ElitePad 1000 G2 various _AEI specified GPIO
> >> ACPI event sources specify a debouncy timeout of 20 ms, but the
> >> pinctrl-baytrail.c only supports certain fixed values, the closest
> >> ones being 12 or 14 ms and pinctrl-baytrail.c responds with -EINVAL
> >> when specified a value which is not one of the fixed values.
> >>
> >> This is causing the acpi_request_own_gpiod() call to fail for 3
> >> ACPI event sources on the HP ElitePad 1000 G2, which in turn is causing
> >> e.g. the battery charging vs discharging status to never get updated,
> >> even though a charger has been plugged-in or unplugged.
> >>
> >> Make gpio_set_debounce_timeout() errors non fatal, warning about the
> >> failure instead, to fix this regression.
> >>
> >> Note we should probably also fix various pinctrl drivers to just
> >> pick the first bigger discrete value rather then returning -EINVAL but
> >> this will need to be done on a per driver basis, where as this fix
> >> at least gets us back to where things were before and thus restores
> >> functionality on devices where this was lost due to
> >> gpio_set_debounce_timeout() errors.
> >>
> >> Fixes: 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > Bartosz will pick this up I think, I'm a bit off duty with GPIO right now.
>
> Added Bartosz to the To: list, to make sure that he actually
> sees this. I somehow did not add him in the To/Cc when sending out
> the patch, sorry about that.
>

Yep, I only process patches that are directed to me personally (TO or CC).

Now applied but I'll only be able to send it out for v5.15-rc3.

Bart

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

* Re: [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal
  2021-09-22  9:17     ` Bartosz Golaszewski
@ 2021-09-22 10:55       ` Andy Shevchenko
  2021-09-22 12:47         ` Bartosz Golaszewski
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2021-09-22 10:55 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Hans de Goede, Linus Walleij, Mika Westerberg, Andy Shevchenko,
	open list:GPIO SUBSYSTEM, ACPI Devel Maling List

On Wed, Sep 22, 2021 at 12:17 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
> On Tue, Sep 21, 2021 at 8:42 PM Hans de Goede <hdegoede@redhat.com> wrote:
> > On 9/21/21 8:28 PM, Linus Walleij wrote:
> > > On Mon, Aug 16, 2021 at 11:39 AM Hans de Goede <hdegoede@redhat.com> wrote:

...

> > >> Fixes: 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > >
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > >
> > > Bartosz will pick this up I think, I'm a bit off duty with GPIO right now.
> >
> > Added Bartosz to the To: list, to make sure that he actually
> > sees this. I somehow did not add him in the To/Cc when sending out
> > the patch, sorry about that.
>
> Yep, I only process patches that are directed to me personally (TO or CC).

Hmm... It's then strange that GPIO lib ACPI is not listed for you in
MAINTAINERS.
Perhaps we need to update MAINTAINERS as well.

> Now applied but I'll only be able to send it out for v5.15-rc3.

Thanks!

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal
  2021-09-22 10:55       ` Andy Shevchenko
@ 2021-09-22 12:47         ` Bartosz Golaszewski
  0 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2021-09-22 12:47 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Hans de Goede, Linus Walleij, Mika Westerberg, Andy Shevchenko,
	open list:GPIO SUBSYSTEM, ACPI Devel Maling List

On Wed, Sep 22, 2021 at 12:56 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Sep 22, 2021 at 12:17 PM Bartosz Golaszewski
> <bgolaszewski@baylibre.com> wrote:
> > On Tue, Sep 21, 2021 at 8:42 PM Hans de Goede <hdegoede@redhat.com> wrote:
> > > On 9/21/21 8:28 PM, Linus Walleij wrote:
> > > > On Mon, Aug 16, 2021 at 11:39 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> ...
>
> > > >> Fixes: 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> > > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > >
> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > >
> > > > Bartosz will pick this up I think, I'm a bit off duty with GPIO right now.
> > >
> > > Added Bartosz to the To: list, to make sure that he actually
> > > sees this. I somehow did not add him in the To/Cc when sending out
> > > the patch, sorry about that.
> >
> > Yep, I only process patches that are directed to me personally (TO or CC).
>
> Hmm... It's then strange that GPIO lib ACPI is not listed for you in
> MAINTAINERS.
> Perhaps we need to update MAINTAINERS as well.
>

With the following line in MAINTAINERS:

    7998 F:      drivers/gpio/

My name should pop up for all patches aimed at the GPIO subsystem if
one uses scripts/get_maintainer.pl.

Bart

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

end of thread, other threads:[~2021-09-22 12:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-16  9:38 [PATCH regression fix] gpiolib: acpi: Make set-debounce-timeout failures non fatal Hans de Goede
2021-08-16 10:25 ` Mika Westerberg
2021-08-16 10:39   ` Hans de Goede
2021-09-21 18:28 ` Linus Walleij
2021-09-21 18:42   ` Hans de Goede
2021-09-22  9:17     ` Bartosz Golaszewski
2021-09-22 10:55       ` Andy Shevchenko
2021-09-22 12:47         ` Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).