Re: [PATCH 2/2] Revert "Input: soc_button_array - debounce the buttons"

[Limonciello, Mario]

On 6/25/25 10:10 AM, Andy Shevchenko wrote:
> On Wed, Jun 25, 2025 at 03:02:18PM +0000, Limonciello, Mario wrote:
>> On 6/25/25 9:41 AM, Mario Limonciello wrote:
>>> On 6/25/25 9:31 AM, Hans de Goede wrote:
>>>> On 25-Jun-25 4:09 PM, Mario Limonciello wrote:
>>>>> On 6/25/25 4:09 AM, Hans de Goede wrote:
>>>>>> On 24-Jun-25 10:22 PM, Mario Limonciello wrote:
> 
> ...
> 
>>>> Ok, so specifically the gpiod_set_debounce() call with 50 ms
>>>> done by gpio_keys.c is the problem I guess?
>>>
>>> Yep.
>>>
>>>> So amd_gpio_set_debounce() does accept the 50 ms debounce
>>>> passed to it by gpio_keys.c as a valid value and then setting
>>>> that breaks the wake from suspend?
>>>
>>> That's right.
> 
>>>>> Also comparing the GPIO register in Windows (where things work)
>>>>> Windows never programs a debounce.
>>>>
>>>> So maybe the windows ACPI0011 driver always uses a software-
>>>> debounce for the buttons? Windows not debouncing the mechanical
>>>> switches at all seems unlikely.
>>>>
>>>> I think the best way to fix this might be to add a no-hw-debounce
>>>> flag to the data passed from soc_button_array.c to gpio_keys.c
>>>> and have gpio_keys.c not call gpiod_set_debounce()  when the
>>>> no-hw-debounce flag is set.
>>>>
>>>> I've checked and both on Bay Trail and Cherry Trail devices
>>>> where soc_button_array is used a lot hw-debouncing is already
>>>> unused. pinctrl-baytrail.c does not accept 50 ms as a valid
>>>> value and pinctrl-cherryview.c does not support hw debounce
>>>> at all.
>>>
>>> That sounds a like a generally good direction to me.
> 
> Thinking a bit more of this, perhaps the HW debounce support flag should be
> per-GPIO-descriptor thingy. In such cases we don't need to distinguish the
> platforms, the GPIO ACPI lib may simply set that flag based on 0 read from
> the ACPI tables. It will also give a clue to any driver that uses GPIOs
> (not only gpio-keys).
> 

But 0 doesn't mean hardware debounce support is there, 0 means that 
hardware debounce is not required to be programmed for this GPIO.

That is - if another system had a non-zero value in the GpioInt entry I 
would expect this to be translated into the GPIO register.