On Fri, Jun 14, 2019 at 9:28 PM Christian Lamparter <[email protected]> wrote:

> 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.

Without any detailed knowledge I'd say the most common cause
is the underlying GPIO chip implementation. There are often transient
line events when the system is powered up and initialized and it
is often necessary for the gpio_chip driver to clear any interrupt
flags in hardware before setting up the gpio chip, especially the
irqchip portions of it.

I tried to half-guess what gpio chip this is using and since it
is WiFi pro 1200e I guess ths gpio driver is
drivers/gpio-ath79.c which does indeed initialize an
irqchip without clearing the interrupts first.

Can you try this patch, if this solves the problem I will commit
it upstream as well:

>From ce4b6db51658e0954f97837095393c5fd1416db2 Mon Sep 17 00:00:00 2001
From: Linus Walleij <[email protected]>
Date: Sat, 15 Jun 2019 10:18:48 +0200
Subject: [PATCH] gpio: ath79: Clear pending IRQs

The ath79 gpio driver may emit "ghost interrupts" because
pending IRQs are sitting in the latches when we probe the
driver. It appears this GPIO block clears interrupts by
reading the status register, so read that and toss the
result before adding the gpio irqchip.

Reported-by: David Bauer <[email protected]>
Reported-by: Christian Lamparter <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
---
 drivers/gpio/gpio-ath79.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-ath79.c b/drivers/gpio/gpio-ath79.c
index 0a553d676042..39ccb48c03d5 100644
--- a/drivers/gpio/gpio-ath79.c
+++ b/drivers/gpio/gpio-ath79.c
@@ -290,6 +290,8 @@ static int ath79_gpio_probe(struct platform_device *pdev)
     if (np && !of_property_read_bool(np, "interrupt-controller"))
         return 0;

+    /* Clear any pending IRQs so we have a clean slate */
+    ath79_gpio_read(ctrl, AR71XX_GPIO_REG_INT_PENDING);
     err = gpiochip_irqchip_add(&ctrl->gc, &ath79_gpio_irqchip, 0,
                 handle_simple_irq, IRQ_TYPE_NONE);
     if (err) {
-- 
2.21.0

> 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).

This seems like a valid usecase. I guess it may be hard to drive
that solution home upstream but at some point it should at least
be discussed with Dmitry (the input maintainer) so he can give
his view on how resource constrained systems should handle
this.

I suspect the root cause is the "footprint problem" that hits IOT
devices like OpenWrt and I know Nicolas Pitre tried to drive
a few approaches upstream to get footprint down, but the
experience was somewhat discouraging.

I think it's a worthy cause though! Small memory systems
should be able to run Linux proper IMO, we just lack the
manpower to make it happen.

Yours,
Linus Walleij

_______________________________________________
openwrt-devel mailing list
[email protected]
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to