Re: [PATCH v3] gpiolib: acpi: Program debounce when finding GPIO

[Mario Limonciello <superm1@kernel.org>]

On 8/11/25 3:42 PM, Andy Shevchenko wrote:
> On Mon, Aug 11, 2025 at 11:43:56AM -0500, Mario Limonciello (AMD) wrote:
>> 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.
>>
>> As this is considered non fatal if it fails, introduce a helper for
>> programming debounce and show a warning when failing to program.
> 
> I think I already commented on this previously. Let me do that below anyway.
> 
> ...
> 
> 
>> +static void acpi_set_debounce_timeout(struct gpio_desc *desc, unsigned int timeout)
>> +{
>> +	int ret;
>> +
>> +	/* ACPI uses hundredths of milliseconds units */
>> +	ret = gpio_set_debounce_timeout(desc, timeout * 10);
>> +	if (ret)
>> +		dev_warn(&desc->gdev->dev,
>> +			 "Failed to set debounce-timeout: %d\n", ret);
>> +}
> 
> I would make it still return the code to the caller. See below why.
> 
> 
> ...
> 
>> -			/* ACPI uses hundredths of milliseconds units */
>> -			ret = gpio_set_debounce_timeout(desc, info.debounce * 10);
>> -			if (ret)
>> -				return ret;
>> +			acpi_set_debounce_timeout(desc, info.debounce);
> 
> The commit message fails to explain why we do relax the condition here. This is
> about GpioInt() resource and so far I haven't heard about misused debounce
> values there. If we drop the fatality, it has to be a separate patch explaining
> why. But only if we have practical use cases. AS long as there no failed
> platforms, I can't agree on this piece of change.
> 

Thanks for the feedback.  I thought that I got your feedback the first 
time when I originally squashed, but I must have failed.

In that case I don't think we really need a helper at all.  I'll change 
it for v4 to just add the extra call without the use of a helper.