On 25-Jun-25 4:41 PM, Mario Limonciello wrote: > On 6/25/25 9:31 AM, Hans de Goede wrote: <snip> >> 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. > > I think I would still like to see the ASL values translated into the hardware even if the ASL has a "0" value. > So I would keep patch 1 but adjust for the warning you guys both called out. > > As you have this hardware would you be able to work out that quirk? I think we've a bit of miscommunication going on here. My proposal is to add a "no_hw_debounce" flag to struct gpio_keys_platform_data and make the soc_button_array driver set that regardless of which platform it is running on. And then in gpio_keys.c do something like this: diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index f9db86da0818..2788d1e5782c 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -552,8 +552,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev, bool active_low = gpiod_is_active_low(bdata->gpiod); if (button->debounce_interval) { - error = gpiod_set_debounce(bdata->gpiod, - button->debounce_interval * 1000); + if (ddata->pdata->no_hw_debounce) + error = -EINVAL; + else + error = gpiod_set_debounce(bdata->gpiod, + button->debounce_interval * 1000); /* use timer if gpiolib doesn't provide debounce */ if (error < 0) bdata->software_debounce = So keep debouncing, which I believe will always be necessary when dealing with mechanical buttons, but always use software debouncing (which I suspect is what Windows does) to avoid issues like the issue you are seeing. My mention of the BYT/CHT behavior in my previous email was to point out that those already do use software debouncing for the 50 ms debounce-period. It was *not* my intention to suggest to solve this with platform specific quirks/behavior. <semi offtopic> Hmm, I did found one interesting thing looking at further DSDTs the Dell Venue 10 Pro 5056 DSDT actually specifies a non 0 debounce time in the ACPI0011 device's GPIO descriptors it uses a value of 30 ms. This device being one of the few actually specifying a debounce time in the ACPI is ironic since it uses drivers/pinctrl/intel/pinctrl-cherryview.c which does not support PIN_CONFIG_INPUT_DEBOUNCE... </semi offtopic> Regards, Hans > > Or if you want me to do it, I'll need something to go on how to how to effectively detect BYT and CYT hardware. > >> >>> So that's where both patches in this series came from. >>> >>>> >>>> drivers/input/keyboard/gpio_keys.c first will call gpiod_set_debounce() >>>> it self with the 50 ms provided by soc_button_array and if that does >>>> not work it will fall back to software debouncing. So I don't see how >>>> the 50 ms debounce can cause problems, other then maybe making >>>> really really (impossible?) fast double-clicks register as a single >>>> click . >>>> >>>> These buttons (e.g. volume up/down) are almost always simply mechanical >>>> switches and these definitely will need debouncing, the 0 value from >>>> the DSDT is plainly just wrong. There is no such thing as a not bouncing >>>> mechanical switch. >>> >>> On one of these tablets can you check the GPIO in Windows to see if it's using any debounce? >> >> I'm afraid I don't have Windows installed on any of these. >> >> But based on your testing + the DSDT specifying no debounce >> for the GPIO I guess Windows just follows the DSDt when it >> comes to setting up the hw debounce-settings and then uses >> sw-debouncing on top to actually avoid very quick >> press-release-press event cycles caused by the bouncing. >> > > Yeah that sounds like a plausible hypothesis. > >