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

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

[Mario Limonciello (AMD)]

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.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v4:
 * Just add a direct call instead
 * drop tag
 * update commit message
---
 drivers/gpio/gpiolib-acpi-core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index 12b24a717e43f..6388e8e363dee 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,11 @@ 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);
+
 	return desc;
 }
 
-- 
2.43.0

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

[Mika Westerberg]

Hi,

On Sun, Aug 17, 2025 at 08:52:07PM -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.

The spec says for GpioInt():

  DebounceTimeout is an optional argument specifying the debounce wait
  time, in hundredths of milliseconds. The bit field name _DBT is
  automatically created to refer to this portion of the resource
  descriptor.

I was not sure but wanted to check that if it is left out, does ACPICA fill
it with 0? If yes (I would expect so) then this is fine.

> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v4:

You missed "v4" in the $subject.

>  * Just add a direct call instead
>  * drop tag
>  * update commit message
> ---
>  drivers/gpio/gpiolib-acpi-core.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
> index 12b24a717e43f..6388e8e363dee 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,11 @@ 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);
> +
>  	return desc;
>  }
>  
> -- 
> 2.43.0

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

[Mario Limonciello]

On 8/17/25 11:55 PM, Mika Westerberg wrote:
> Hi,
> 
> On Sun, Aug 17, 2025 at 08:52:07PM -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.
> 
> The spec says for GpioInt():
> 
>    DebounceTimeout is an optional argument specifying the debounce wait
>    time, in hundredths of milliseconds. The bit field name _DBT is
>    automatically created to refer to this portion of the resource
>    descriptor.
> 
> I was not sure but wanted to check that if it is left out, does ACPICA fill
> it with 0? If yes (I would expect so) then this is fine.

Yeah AFAICT you're right.  The ACPI resource is zero'ed out, so if the 
field was empty it should default to zero.

> 
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> v4:
> 
> You missed "v4" in the $subject.

Whoops, thanks.  If there ends up being a reason to spin I'll send the 
next one as a v5.

> 
>>   * Just add a direct call instead
>>   * drop tag
>>   * update commit message
>> ---
>>   drivers/gpio/gpiolib-acpi-core.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
>> index 12b24a717e43f..6388e8e363dee 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,11 @@ 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);
>> +
>>   	return desc;
>>   }
>>   
>> -- 
>> 2.43.0

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

[Mika Westerberg]

On Mon, Aug 18, 2025 at 01:03:48PM -0500, Mario Limonciello wrote:
> On 8/17/25 11:55 PM, Mika Westerberg wrote:
> > Hi,
> > 
> > On Sun, Aug 17, 2025 at 08:52:07PM -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.
> > 
> > The spec says for GpioInt():
> > 
> >    DebounceTimeout is an optional argument specifying the debounce wait
> >    time, in hundredths of milliseconds. The bit field name _DBT is
> >    automatically created to refer to this portion of the resource
> >    descriptor.
> > 
> > I was not sure but wanted to check that if it is left out, does ACPICA fill
> > it with 0? If yes (I would expect so) then this is fine.
> 
> Yeah AFAICT you're right.  The ACPI resource is zero'ed out, so if the field
> was empty it should default to zero.

Okay good.

Then from my perspective,

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

> > 
> > > Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> > > ---
> > > v4:
> > 
> > You missed "v4" in the $subject.
> 
> Whoops, thanks.  If there ends up being a reason to spin I'll send the next
> one as a v5.
> 
> > 
> > >   * Just add a direct call instead
> > >   * drop tag
> > >   * update commit message
> > > ---
> > >   drivers/gpio/gpiolib-acpi-core.c | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
> > > index 12b24a717e43f..6388e8e363dee 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,11 @@ 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);
> > > +
> > >   	return desc;
> > >   }
> > > -- 
> > > 2.43.0

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

[Andy Shevchenko]

On Tue, Aug 19, 2025 at 08:18:13AM +0200, Mika Westerberg wrote:
> On Mon, Aug 18, 2025 at 01:03:48PM -0500, Mario Limonciello wrote:
> > On 8/17/25 11:55 PM, Mika Westerberg wrote:
> > > On Sun, Aug 17, 2025 at 08:52:07PM -0500, Mario Limonciello (AMD) wrote:

...

> > > I was not sure but wanted to check that if it is left out, does ACPICA fill
> > > it with 0? If yes (I would expect so) then this is fine.
> > 
> > Yeah AFAICT you're right.  The ACPI resource is zero'ed out, so if the field
> > was empty it should default to zero.
> 
> Okay good.
> 
> Then from my perspective,
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Seems I came from vacation just in time :-)
Please, send v5 with all nits and tags and I will apply it.

-- 
With Best Regards,
Andy Shevchenko