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.
