[OpenWrt-Devel] [PATCH] gpio-button-hotplug: gpio-keys: read initial state

[David Bauer]

Hello Linus,

On 17.06.19 22:26, Linus Walleij wrote:
> Hm this sounds like something that would be solved by debouncing.
> It might even be a bounce effect of sorts, it can be a capacitance
> or something in the mechanics causing this.
> 
> If you look in:
> drivers/input/keyboard/gpio_keys.c
> you will see that GPIO keys in the input subsystem has debouncing
> support. I guess something like this needs to be copied over to
> the OpenWrt netlink thingie.

Thanks for your evaluation. So the underlying GPIO driver does not seem to be the
culprit here.

> If the GPIO driver supports debounce (some do, it doesn't look like
> the ath79 does) that can be utilized. If someone can double-check
> the ath79 datasheet to check if it can do debounce that'd be great
> because it would solve this in hardware.

I've had a short look on the QCA9558 datasheet and it doesn't seem
like the driver supports debouncing.

>> If delays are allowed there, we could add a 20ms delay when configuring the GPIO line
>> direction as input. This way we could also assure the line is stable for "normal" value
>> reads. We would need to override the direction_input method provided by gpio-mmio for this.
> 
> That's like an initial debounce.

I've had a deeper look on

drivers/input/keyboard/gpio_keys.c

and i think I've understood the problem now as we finally found the "right level" of
the problem ;)

Upstream gpio_keys reports current state of all buttons on driver probe in
gpio_keys_open()

	/* Report current state of buttons that are connected to GPIOs */
	gpio_keys_report_state(ddata);

When an interrupt is handled, a job is created (or it's delay modified).
So the job is not executed if a GPIO is unstable for the debounce interval.

The job executes gpio_keys_gpio_work_func() as soon as it's state is stable,
which reports an input_event() in gpio_keys_gpio_report_event().

This event however is not handed down in the input subsystem if the value equals
the last state. As we are not using the input subsystem, we need to keep track of
the last button state ourselves.

Correct me if I'm wrong with anything above. :)

@Christian
If I'm not mistaken is kinda what I've implemented with my initial patch.
However, i think the logic is better placed elsewhere. I will send
a reworked patch shortly.


> 
>> Second:
>> We could store the kernel uptime together with the GPIO line and for how long interrupts
>> should be ignored within the driver. This could be checked when an interrupt is fired.
> 
> The third alternative is common software debounce. I.e. wait for any
> value to stabilize before reporting keys. Some extra interrupts more
> or less doesn't matter, we just frame it with some timer.
> 
>> I do not really like the second idea as dropping interrupts without feedback is probably not
>> what we want.
> 
> I think it makes a lot of sense on mechanical pushbuttons to
> implement generic debounce.

This is, however, not the job of the GPIO driver or is it?


Best wishes
David

> Yours,
> Linus Walleij
> 



_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel