* [PATCH 6.17 REGRESSION FIX] gpiolib: acpi: Make set debounce errors non fatal
@ 2025-09-20 20:12 Hans de Goede
2025-09-21 18:08 ` Mario Limonciello (AMD) (kernel.org)
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2025-09-20 20:12 UTC (permalink / raw)
To: Mika Westerberg, Andy Shevchenko, Bartosz Golaszewski,
Linus Walleij
Cc: Hans de Goede, linux-gpio, linux-acpi, stable, Mario Limonciello
Commit 16c07342b542 ("gpiolib: acpi: Program debounce when finding GPIO")
adds a gpio_set_debounce_timeout() call to acpi_find_gpio() and makes
acpi_find_gpio() fail if this fails.
But gpio_set_debounce_timeout() failing is a somewhat normal occurrence,
since not all debounce values are supported on all GPIO/pinctrl chips.
Making this an error for example break getting the card-detect GPIO for
the micro-sd slot found on many Bay Trail tablets, breaking support for
the micro-sd slot on these tablets.
acpi_request_own_gpiod() already treats gpio_set_debounce_timeout()
failures as non-fatal, just warning about them.
Add a acpi_gpio_set_debounce_timeout() helper which wraps
gpio_set_debounce_timeout() and warns on failures and replace both existing
gpio_set_debounce_timeout() calls with the helper.
Since the helper only warns on failures this fixes the card-detect issue.
Fixes: 16c07342b542 ("gpiolib: acpi: Program debounce when finding GPIO")
Cc: stable@vger.kernel.org
Cc: Mario Limonciello <superm1@kernel.org>
Signed-off-by: Hans de Goede <hansg@kernel.org>
---
drivers/gpio/gpiolib-acpi-core.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index 284e762d92c4..67c4c38afb86 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -291,6 +291,19 @@ acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio, int polarity)
return GPIOD_ASIS;
}
+static void acpi_gpio_set_debounce_timeout(struct gpio_desc *desc,
+ unsigned int acpi_debounce)
+{
+ int ret;
+
+ /* ACPI uses hundredths of milliseconds units */
+ acpi_debounce *= 10;
+ ret = gpio_set_debounce_timeout(desc, acpi_debounce);
+ if (ret)
+ gpiod_warn(desc, "Failed to set debounce-timeout %u: %d\n",
+ acpi_debounce, ret);
+}
+
static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
struct acpi_resource_gpio *agpio,
unsigned int index,
@@ -300,18 +313,12 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio, polarity);
unsigned int pin = agpio->pin_table[index];
struct gpio_desc *desc;
- int ret;
desc = gpiochip_request_own_desc(chip, pin, label, polarity, flags);
if (IS_ERR(desc))
return desc;
- /* ACPI uses hundredths of milliseconds units */
- ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout * 10);
- if (ret)
- dev_warn(chip->parent,
- "Failed to set debounce-timeout for pin 0x%04X, err %d\n",
- pin, ret);
+ acpi_gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
return desc;
}
@@ -944,7 +951,6 @@ struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
bool can_fallback = acpi_can_fallback_to_crs(adev, con_id);
struct acpi_gpio_info info = {};
struct gpio_desc *desc;
- int ret;
desc = __acpi_find_gpio(fwnode, con_id, idx, can_fallback, &info);
if (IS_ERR(desc))
@@ -959,10 +965,7 @@ struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
acpi_gpio_update_gpiod_flags(dflags, &info);
acpi_gpio_update_gpiod_lookup_flags(lookupflags, &info);
- /* ACPI uses hundredths of milliseconds units */
- ret = gpio_set_debounce_timeout(desc, info.debounce * 10);
- if (ret)
- return ERR_PTR(ret);
+ acpi_gpio_set_debounce_timeout(desc, info.debounce);
return desc;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 6.17 REGRESSION FIX] gpiolib: acpi: Make set debounce errors non fatal
2025-09-20 20:12 [PATCH 6.17 REGRESSION FIX] gpiolib: acpi: Make set debounce errors non fatal Hans de Goede
@ 2025-09-21 18:08 ` Mario Limonciello (AMD) (kernel.org)
2025-09-21 19:03 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Mario Limonciello (AMD) (kernel.org) @ 2025-09-21 18:08 UTC (permalink / raw)
To: Hans de Goede, Mika Westerberg, Andy Shevchenko,
Bartosz Golaszewski, Linus Walleij
Cc: linux-gpio, linux-acpi, stable
On 9/20/2025 3:12 PM, Hans de Goede wrote:
> Commit 16c07342b542 ("gpiolib: acpi: Program debounce when finding GPIO")
> adds a gpio_set_debounce_timeout() call to acpi_find_gpio() and makes
> acpi_find_gpio() fail if this fails.
>
> But gpio_set_debounce_timeout() failing is a somewhat normal occurrence,
> since not all debounce values are supported on all GPIO/pinctrl chips.
>
> Making this an error for example break getting the card-detect GPIO for
> the micro-sd slot found on many Bay Trail tablets, breaking support for
> the micro-sd slot on these tablets.
>
> acpi_request_own_gpiod() already treats gpio_set_debounce_timeout()
> failures as non-fatal, just warning about them.
>
> Add a acpi_gpio_set_debounce_timeout() helper which wraps
> gpio_set_debounce_timeout() and warns on failures and replace both existing
> gpio_set_debounce_timeout() calls with the helper.
>
> Since the helper only warns on failures this fixes the card-detect issue.
>
> Fixes: 16c07342b542 ("gpiolib: acpi: Program debounce when finding GPIO")
> Cc: stable@vger.kernel.org
> Cc: Mario Limonciello <superm1@kernel.org>
> Signed-off-by: Hans de Goede <hansg@kernel.org>
Looks pretty much identical now to what I sent in my v3 and that Andy
had requested we change to make it fatal [1].
Where is this bad GPIO value coming from? It's in the GpioInt()
declaration? If so, should the driver actually be supporting this?
https://lore.kernel.org/linux-gpio/20250811164356.613840-1-superm1@kernel.org/
[1]
> ---
> drivers/gpio/gpiolib-acpi-core.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
> index 284e762d92c4..67c4c38afb86 100644
> --- a/drivers/gpio/gpiolib-acpi-core.c
> +++ b/drivers/gpio/gpiolib-acpi-core.c
> @@ -291,6 +291,19 @@ acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio, int polarity)
> return GPIOD_ASIS;
> }
>
> +static void acpi_gpio_set_debounce_timeout(struct gpio_desc *desc,
> + unsigned int acpi_debounce)
> +{
> + int ret;
> +
> + /* ACPI uses hundredths of milliseconds units */
> + acpi_debounce *= 10;
> + ret = gpio_set_debounce_timeout(desc, acpi_debounce);
> + if (ret)
> + gpiod_warn(desc, "Failed to set debounce-timeout %u: %d\n",
> + acpi_debounce, ret);
> +}
> +
> static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
> struct acpi_resource_gpio *agpio,
> unsigned int index,
> @@ -300,18 +313,12 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
> enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio, polarity);
> unsigned int pin = agpio->pin_table[index];
> struct gpio_desc *desc;
> - int ret;
>
> desc = gpiochip_request_own_desc(chip, pin, label, polarity, flags);
> if (IS_ERR(desc))
> return desc;
>
> - /* ACPI uses hundredths of milliseconds units */
> - ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout * 10);
> - if (ret)
> - dev_warn(chip->parent,
> - "Failed to set debounce-timeout for pin 0x%04X, err %d\n",
> - pin, ret);
> + acpi_gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
>
> return desc;
> }
> @@ -944,7 +951,6 @@ struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
> bool can_fallback = acpi_can_fallback_to_crs(adev, con_id);
> struct acpi_gpio_info info = {};
> struct gpio_desc *desc;
> - int ret;
>
> desc = __acpi_find_gpio(fwnode, con_id, idx, can_fallback, &info);
> if (IS_ERR(desc))
> @@ -959,10 +965,7 @@ struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
> acpi_gpio_update_gpiod_flags(dflags, &info);
> acpi_gpio_update_gpiod_lookup_flags(lookupflags, &info);
>
> - /* ACPI uses hundredths of milliseconds units */
> - ret = gpio_set_debounce_timeout(desc, info.debounce * 10);
> - if (ret)
> - return ERR_PTR(ret);
> + acpi_gpio_set_debounce_timeout(desc, info.debounce);
>
> return desc;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 6.17 REGRESSION FIX] gpiolib: acpi: Make set debounce errors non fatal
2025-09-21 18:08 ` Mario Limonciello (AMD) (kernel.org)
@ 2025-09-21 19:03 ` Andy Shevchenko
2025-09-21 20:11 ` Hans de Goede
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2025-09-21 19:03 UTC (permalink / raw)
To: Mario Limonciello (AMD) (kernel.org)
Cc: Hans de Goede, Mika Westerberg, Andy Shevchenko,
Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-acpi,
stable
On Sun, Sep 21, 2025 at 9:09 PM Mario Limonciello (AMD) (kernel.org)
<superm1@kernel.org> wrote:
> On 9/20/2025 3:12 PM, Hans de Goede wrote:
...
> Looks pretty much identical now to what I sent in my v3 and that Andy
> had requested we change to make it fatal [1].
>
> Where is this bad GPIO value coming from? It's in the GpioInt()
> declaration? If so, should the driver actually be supporting this?
Since it's in acpi_find_gpio() it's about any GPIO resource type.
Sorry, it seems I missed this fact. I was under the impression that v4
was done only for the GpioInt() case. With this being said, the
GpioIo() should not be fatal (it's already proven by cases in the wild
that sometimes given values there are unsupported by HW), but
GpioInt() in my opinion needs a justification to become non-fatal.
OTOH, for the first case we can actually run SW debounce. But it might
be quite an intrusive change to call it "a fix".
So, taking the above into account I suggest that the helper should
return int and check the info.gpioint flag and in case of being set,
return an error, otherwise ignore wrong settings. Or something like
this. In such a case we may also use it in the
acpi_dev_gpio_irq_wake_get_by().
> https://lore.kernel.org/linux-gpio/20250811164356.613840-1-superm1@kernel.org/
> [1]
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 6.17 REGRESSION FIX] gpiolib: acpi: Make set debounce errors non fatal
2025-09-21 19:03 ` Andy Shevchenko
@ 2025-09-21 20:11 ` Hans de Goede
2025-09-21 20:25 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2025-09-21 20:11 UTC (permalink / raw)
To: Andy Shevchenko, Mario Limonciello (AMD) (kernel.org)
Cc: Mika Westerberg, Andy Shevchenko, Bartosz Golaszewski,
Linus Walleij, linux-gpio, linux-acpi, stable
Hi All,
On 21-Sep-25 9:03 PM, Andy Shevchenko wrote:
> On Sun, Sep 21, 2025 at 9:09 PM Mario Limonciello (AMD) (kernel.org)
> <superm1@kernel.org> wrote:
>> On 9/20/2025 3:12 PM, Hans de Goede wrote:
>
> ...
>
>> Looks pretty much identical now to what I sent in my v3 and that Andy
>> had requested we change to make it fatal [1].
>>
>> Where is this bad GPIO value coming from? It's in the GpioInt()
>> declaration? If so, should the driver actually be supporting this?
>
> Since it's in acpi_find_gpio() it's about any GPIO resource type.
> Sorry, it seems I missed this fact. I was under the impression that v4
> was done only for the GpioInt() case. With this being said, the
> GpioIo() should not be fatal (it's already proven by cases in the wild
> that sometimes given values there are unsupported by HW), but
> GpioInt() in my opinion needs a justification to become non-fatal.
GpioInt() debounce setting not succeeding already is non fatal in
the acpi_request_own_gpiod() case, which is used for ACPI events
(_AEI resources) and that exact use-case is why it was made non-fatal,
so no this is not only about GpioIo() resources. See commit
cef0d022f553 ("gpiolib: acpi: Make set-debounce-timeout failures non
fatal")
IOW we need set debounce failures to be non-fatal for both the GpioIo
and GpioInt cases and this fix is correct as is.
It is very likely too late to fix this *regression* for 6.17.0, please
queue this up for merging ASAP so that we can get a fix added to 6.17.1
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 6.17 REGRESSION FIX] gpiolib: acpi: Make set debounce errors non fatal
2025-09-21 20:11 ` Hans de Goede
@ 2025-09-21 20:25 ` Andy Shevchenko
2025-09-22 16:21 ` Mario Limonciello (AMD) (kernel.org)
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2025-09-21 20:25 UTC (permalink / raw)
To: Hans de Goede
Cc: Mario Limonciello (AMD) (kernel.org), Mika Westerberg,
Andy Shevchenko, Bartosz Golaszewski, Linus Walleij, linux-gpio,
linux-acpi, stable
On Sun, Sep 21, 2025 at 11:11 PM Hans de Goede <hansg@kernel.org> wrote:
> On 21-Sep-25 9:03 PM, Andy Shevchenko wrote:
> > On Sun, Sep 21, 2025 at 9:09 PM Mario Limonciello (AMD) (kernel.org)
> > <superm1@kernel.org> wrote:
> >> On 9/20/2025 3:12 PM, Hans de Goede wrote:
...
> >> Looks pretty much identical now to what I sent in my v3 and that Andy
> >> had requested we change to make it fatal [1].
> >>
> >> Where is this bad GPIO value coming from? It's in the GpioInt()
> >> declaration? If so, should the driver actually be supporting this?
> >
> > Since it's in acpi_find_gpio() it's about any GPIO resource type.
> > Sorry, it seems I missed this fact. I was under the impression that v4
> > was done only for the GpioInt() case. With this being said, the
> > GpioIo() should not be fatal (it's already proven by cases in the wild
> > that sometimes given values there are unsupported by HW), but
> > GpioInt() in my opinion needs a justification to become non-fatal.
>
> GpioInt() debounce setting not succeeding already is non fatal in
> the acpi_request_own_gpiod() case, which is used for ACPI events
> (_AEI resources) and that exact use-case is why it was made non-fatal,
> so no this is not only about GpioIo() resources. See commit
> cef0d022f553 ("gpiolib: acpi: Make set-debounce-timeout failures non
> fatal")
>
> IOW we need set debounce failures to be non-fatal for both the GpioIo
> and GpioInt cases and this fix is correct as is.
Okay, since it doesn't change the state of affairs with for
acpi_dev_gpio_irq_wake_get_by(), it's fair enough to get it as is.
Mario, do you agree with Hans' explanations?
> It is very likely too late to fix this *regression* for 6.17.0, please
> queue this up for merging ASAP so that we can get a fix added to 6.17.1
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 6.17 REGRESSION FIX] gpiolib: acpi: Make set debounce errors non fatal
2025-09-21 20:25 ` Andy Shevchenko
@ 2025-09-22 16:21 ` Mario Limonciello (AMD) (kernel.org)
0 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello (AMD) (kernel.org) @ 2025-09-22 16:21 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede
Cc: Mika Westerberg, Andy Shevchenko, Bartosz Golaszewski,
Linus Walleij, linux-gpio, linux-acpi, stable
On 9/21/2025 3:25 PM, Andy Shevchenko wrote:
> On Sun, Sep 21, 2025 at 11:11 PM Hans de Goede <hansg@kernel.org> wrote:
>> On 21-Sep-25 9:03 PM, Andy Shevchenko wrote:
>>> On Sun, Sep 21, 2025 at 9:09 PM Mario Limonciello (AMD) (kernel.org)
>>> <superm1@kernel.org> wrote:
>>>> On 9/20/2025 3:12 PM, Hans de Goede wrote:
>
> ...
>
>>>> Looks pretty much identical now to what I sent in my v3 and that Andy
>>>> had requested we change to make it fatal [1].
>>>>
>>>> Where is this bad GPIO value coming from? It's in the GpioInt()
>>>> declaration? If so, should the driver actually be supporting this?
>>>
>>> Since it's in acpi_find_gpio() it's about any GPIO resource type.
>>> Sorry, it seems I missed this fact. I was under the impression that v4
>>> was done only for the GpioInt() case. With this being said, the
>>> GpioIo() should not be fatal (it's already proven by cases in the wild
>>> that sometimes given values there are unsupported by HW), but
>>> GpioInt() in my opinion needs a justification to become non-fatal.
>>
>> GpioInt() debounce setting not succeeding already is non fatal in
>> the acpi_request_own_gpiod() case, which is used for ACPI events
>> (_AEI resources) and that exact use-case is why it was made non-fatal,
>> so no this is not only about GpioIo() resources. See commit
>> cef0d022f553 ("gpiolib: acpi: Make set-debounce-timeout failures non
>> fatal")
>>
>> IOW we need set debounce failures to be non-fatal for both the GpioIo
>> and GpioInt cases and this fix is correct as is.
>
> Okay, since it doesn't change the state of affairs with for
> acpi_dev_gpio_irq_wake_get_by(), it's fair enough to get it as is.
> Mario, do you agree with Hans' explanations?
>
Yeah it's fine as is, no concerns.
>> It is very likely too late to fix this *regression* for 6.17.0, please
>> queue this up for merging ASAP so that we can get a fix added to 6.17.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-22 16:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-20 20:12 [PATCH 6.17 REGRESSION FIX] gpiolib: acpi: Make set debounce errors non fatal Hans de Goede
2025-09-21 18:08 ` Mario Limonciello (AMD) (kernel.org)
2025-09-21 19:03 ` Andy Shevchenko
2025-09-21 20:11 ` Hans de Goede
2025-09-21 20:25 ` Andy Shevchenko
2025-09-22 16:21 ` Mario Limonciello (AMD) (kernel.org)
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).