On Thursday, June 20, 2019 8:10:18 PM CEST David Bauer wrote: > Hello Christian, > On 20.06.19 17:21, Christian Lamparter wrote: > > On Tuesday, June 18, 2019 1:06:12 PM CEST David Bauer wrote: > >> This patch implements consistent handling of the debounce interval set > >> for the GPIO buttons. Hotplug events will only be fired if > >> > >> 1. It's the initial stable state (no state-change for duration of the > >> debounce interval) for a switch. Buttons will not trigger an event for > >> the initial stable state. > >> > >> 2. The input changes it's state and remains stable for the debounce > >> interval. > >> > >> Prior to this patch, this was handled inconsistently for interrupt-based > >> an polled gpio-keys. We unify the shared logic in button_hotplug_event > >> and modify both implementations to read the initial state. > >> > >> Run-tested for 'gpio-keys' and 'gpio-keys-polled' on > >> > >> - devolo WiFi pro 1200e > >> - devolo WiFi pro 1750c > >> - devolo WiFi pro 1750x > >> > >> Signed-off-by: David Bauer <[email protected]> > >> --- > >> .../src/gpio-button-hotplug.c | 42 +++++++++---------- > >> 1 file changed, 20 insertions(+), 22 deletions(-) > >> > >> diff --git a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c > >> b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c > >> index e63d414284..25150344e0 100644 > >> --- a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c > >> +++ b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c > >> @@ -351,8 +352,8 @@ static irqreturn_t button_handle_irq(int irq, void > >> *_bdata) > >> struct gpio_keys_button_data *bdata = > >> (struct gpio_keys_button_data *) _bdata; > >> > >> - schedule_delayed_work(&bdata->work, > >> - msecs_to_jiffies(bdata->software_debounce)); > >> + mod_delayed_work(system_wq, &bdata->work, > >> + msecs_to_jiffies(bdata->software_debounce)); > >> > >> return IRQ_HANDLED; > >> } > >> @@ -608,6 +609,9 @@ static int gpio_keys_probe(struct platform_device > >> *pdev) > >> > >> INIT_DELAYED_WORK(&bdata->work, gpio_keys_irq_work_func); > >> > >> + schedule_delayed_work(&bdata->work, > >> + > >> msecs_to_jiffies(bdata->software_debounce)); > >> + > > Hm, well since the first state is -1 we could just as well schedule the work > > immediately here... > > Hmm, i have a bit trouble grasping your intention here. > > Do you mean we can unify the scheduling for polled and > interrupt-based keys?
I was gunning for making the polled and interrupt behave in the same way. Currently the gpio_polled variant does | for (i = 0; i < pdata->nbuttons; i++) | gpio_keys_polled_check_state(&bdev->data[i]); as part of it's probe function which immediately initializes the button's state from -1 to either 0 or 1. But with the | + schedule_delayed_work(&bdata->work, | + msecs_to_jiffies(bdata->software_debounce)); the irq-path would wait software_debounce (5ms by default) time until the -1 is cleared. This is not really a problem, other than a symmetry mismatch when targets switch from gpio-keys to gpio-keys-polled or vice versa and expect the implementation to behave in the same manner. Best Regards, Christian _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
