Re: [PATCH 1/2] gpiolib: acpi: Program debounce when finding GPIO

[Hans de Goede <hansg@kernel.org>]

Hi Mario,

On 24-Jun-25 10:22 PM, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> When soc-button-array looks up the GPIO to use it calls acpi_find_gpio()
> which will parse _CRS.
> 
> acpi_find_gpio.cold (drivers/gpio/gpiolib-acpi-core.c:953)
> gpiod_find_and_request (drivers/gpio/gpiolib.c:4598 drivers/gpio/gpiolib.c:4625)
> gpiod_get_index (drivers/gpio/gpiolib.c:4877)
> 
> The GPIO is setup basically, but the debounce information is discarded.
> The platform will assert what debounce should be in _CRS, so program it
> at the time it's available.
> 
> Cc: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpio/gpiolib-acpi-core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
> index 12b24a717e43f..475cac2d95aa1 100644
> --- a/drivers/gpio/gpiolib-acpi-core.c
> +++ b/drivers/gpio/gpiolib-acpi-core.c
> @@ -944,6 +944,7 @@ 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))
> @@ -957,6 +958,9 @@ 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);
> +	ret = gpio_set_debounce_timeout(desc, info.debounce * 10);
> +	if (ret)
> +		return ERR_PTR(ret);

IIRC this is going to fail sometimes, depending on which range of
debounce values the GPIO controller support. Note that there already
is another code-path in gpiolib-acpi-core.c which calls
gpio_set_debounce_timeout() in acpi_request_own_gpiod() and it does:

        /* 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);

Making this a warning was done in commit cef0d022f553 ("gpiolib: acpi: Make
set-debounce-timeout failures non fatal").

Otherwise I think this is fine.

Regards,

Hans