diff mbox series

[PATCHv3] gpio: gpio-omap: Fix lost edge wake-up interrupts

Message ID 20190612063352.5760-1-tony@atomide.com (mailing list archive)
State Mainlined
Commit a522f1d0c381c42f3ace13b8bbeeccabdd6d2e5c
Headers show
Series [PATCHv3] gpio: gpio-omap: Fix lost edge wake-up interrupts | expand

Commit Message

Tony Lindgren June 12, 2019, 6:33 a.m. UTC
If an edge interrupt triggers while entering idle just before we save
GPIO datain register to saved_datain, the triggered GPIO will not be
noticed on wake-up. This is because the saved_datain and GPIO datain
are the same on wake-up in omap_gpio_unidle(). Let's fix this by
ignoring any pending edge interrupts for saved_datain.

This issue affects only idle states where the GPIO module internal
wake-up path is operational. For deeper idle states where the GPIO
module gets powered off, Linux generic wakeirqs must be used for
the padconf wake-up events with pinctrl-single driver. For examples,
please see "interrupts-extended" dts usage in many drivers.

This issue can be somewhat easily reproduced by pinging an idle system
with smsc911x Ethernet interface configured IRQ_TYPE_EDGE_FALLING. At
some point the smsc911x interrupts will just stop triggering. Also if
WLCORE WLAN is used with EDGE interrupt like it's documentation specifies,
we can see lost interrupts without this patch.

Note that in the long run we may be able to cancel entering idle by
returning an error in gpio_omap_cpu_notifier() on pending interrupts.
But let's fix the bug first.

Also note that because of the recent clean-up efforts this patch does
not apply directly to older kernels. This does fix a long term issue
though, and can be backported as needed.

Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

This patch is against v5.2-rc series. FYI, it does not conflict with
the fixes (or the clean-up) in Russell's patch series.

Changes since v2:
- Add comments for what we're checking, drop comments for saved_datain

Changes since v1:
- Add handling to ignore EDGE_BOTH

---
 drivers/gpio/gpio-omap.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Linus Walleij June 12, 2019, 12:32 p.m. UTC | #1
On Wed, Jun 12, 2019 at 8:34 AM Tony Lindgren <tony@atomide.com> wrote:

> If an edge interrupt triggers while entering idle just before we save
> GPIO datain register to saved_datain, the triggered GPIO will not be
> noticed on wake-up. This is because the saved_datain and GPIO datain
> are the same on wake-up in omap_gpio_unidle(). Let's fix this by
> ignoring any pending edge interrupts for saved_datain.
>
> This issue affects only idle states where the GPIO module internal
> wake-up path is operational. For deeper idle states where the GPIO
> module gets powered off, Linux generic wakeirqs must be used for
> the padconf wake-up events with pinctrl-single driver. For examples,
> please see "interrupts-extended" dts usage in many drivers.
>
> This issue can be somewhat easily reproduced by pinging an idle system
> with smsc911x Ethernet interface configured IRQ_TYPE_EDGE_FALLING. At
> some point the smsc911x interrupts will just stop triggering. Also if
> WLCORE WLAN is used with EDGE interrupt like it's documentation specifies,
> we can see lost interrupts without this patch.
>
> Note that in the long run we may be able to cancel entering idle by
> returning an error in gpio_omap_cpu_notifier() on pending interrupts.
> But let's fix the bug first.
>
> Also note that because of the recent clean-up efforts this patch does
> not apply directly to older kernels. This does fix a long term issue
> though, and can be backported as needed.
>
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Ladislav Michl <ladis@linux-mips.org>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Patch applied.

Let's see if this nails it.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1277,13 +1277,23 @@  static void omap_gpio_idle(struct gpio_bank *bank, bool may_lose_context)
 {
 	struct device *dev = bank->chip.parent;
 	void __iomem *base = bank->base;
-	u32 nowake;
+	u32 mask, nowake;
 
 	bank->saved_datain = readl_relaxed(base + bank->regs->datain);
 
 	if (!bank->enabled_non_wakeup_gpios)
 		goto update_gpio_context_count;
 
+	/* Check for pending EDGE_FALLING, ignore EDGE_BOTH */
+	mask = bank->enabled_non_wakeup_gpios & bank->context.fallingdetect;
+	mask &= ~bank->context.risingdetect;
+	bank->saved_datain |= mask;
+
+	/* Check for pending EDGE_RISING, ignore EDGE_BOTH */
+	mask = bank->enabled_non_wakeup_gpios & bank->context.risingdetect;
+	mask &= ~bank->context.fallingdetect;
+	bank->saved_datain &= ~mask;
+
 	if (!may_lose_context)
 		goto update_gpio_context_count;