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

[Christian Lamparter]

On Thursday, June 13, 2019 11:00:36 PM CEST David Bauer wrote:
> This commit reads the initial state for interrupt triggered gpio-keys.
> Without this commit, the switch to the initial stable input-state
> triggers a button-event. Button events are now only triggered when the
> button state differs fromt the initial state.
> 
> Effectively, this reverts commit 6c5bfaac84b0 ("gpio-button-hotplug:
> gpio-keys: fix always missing first event"), but in addition, we read
> the initial button state on driver probe. This commit caused some devices
> to enter failsafe mode every time when booting.
> 
> Run-tested on devolo WiFi pro 1200e & 1200i.
> 
> Signed-off-by: David Bauer <mail at david-bauer.net>

Oh boy, I think the problem is in the failsafe button handler instead.

|
|#!/bin/sh
|
|[ "${TYPE}" = "switch" ] || echo ${BUTTON} > /tmp/failsafe_button
|
|return 0
|

It's not checking the button state, so even a single "released" message
can do something. If this turns out to be even more troublesome I think
we should enlist Linus Walleij... he might be interested too, since he's
the upstream linux-gpio maintainer and gemini target contributor.

Maybe he can point to a specifc reason why the interrupt gets triggered
when the module is loaded and how to handle it. Because this behavior
seems to be common between different platforms now and the upstream
gpio-keys (which does work differently!) seems to handle it just fine.

@Linus: Do you have any inside knowledge about the issue? That when
gpio-keys is loaded (in OpenWrt it's a module due to kernel size
constraint on various routers) the associated interrupt fires and
this results in a ghost key event. I have to add that OpenWrt's
gpio-button-hotplug.c (which registers the gpio-keys and
gpio-keys-polled) is a special, out-of-tree module that sends broadcast
events (netlink) rather than using the input-subsystem (again due
to space issues). 

the OpenWrt's gpio-button-hotplug.c source is right here:
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c

Regards,
Christian

> ---
>  .../src/gpio-button-hotplug.c                  | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 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 daa4b2a4f7..5bc783e015 100644
> --- a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
> +++ b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c
> @@ -341,8 +341,16 @@ static void gpio_keys_irq_work_func(struct work_struct *work)
>  	struct gpio_keys_button_data *bdata = container_of(work,
>  		struct gpio_keys_button_data, work.work);
>  
> -	button_hotplug_event(bdata, bdata->b->type ?: EV_KEY,
> -			     gpio_button_get_value(bdata));
> +	int state = gpio_button_get_value(bdata);
> +
> +	if (state != bdata->last_state) {
> +		unsigned int type = bdata->b->type ?: EV_KEY;
> +
> +		if (bdata->last_state != -1 || type == EV_SW)
> +			button_hotplug_event(bdata, type, state);
> +
> +		bdata->last_state = state;
> +	}
>  }
>  
>  static irqreturn_t button_handle_irq(int irq, void *_bdata)
> @@ -607,6 +615,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));
> +
>  		ret = devm_request_threaded_irq(&pdev->dev,
>  			bdata->irq, NULL, button_handle_irq,
>  			irqflags, dev_name(&pdev->dev), bdata);
> @@ -620,9 +631,6 @@ static int gpio_keys_probe(struct platform_device *pdev)
>  			dev_dbg(&pdev->dev, "gpio:%d has irq:%d\n",
>  				button->gpio, bdata->irq);
>  		}
> -
> -		if (bdata->b->type == EV_SW)
> -			button_hotplug_event(bdata, EV_SW, gpio_button_get_value(bdata));
>  	}
>  
>  	return 0;
> 





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