[PATCH] gpio: dwapb: Add robust error handling in interrupt handler

[PATCH] gpio: dwapb: Add robust error handling in interrupt handler

[Liang Hao]

The current interrupt handler silently continues if an interrupt
handling fails, which may lead to interrupt storms. Add proper
error handling to gracefully recover from failed interrupt
handling.

When generic_handle_irq() fails, the following recovery actions are
taken:
  - Write EOI to clear the pending interrupt
  - Mask the interrupt to prevent immediate re-triggering
  - Disable the interrupt to stop further interrupts on this line

These measures prevent the system from being overwhelmed by repeated
unhandled interrupts while logging a rate-limited warning for
debugging purposes.

Signed-off-by: Liang Hao <haohlliang@gmail.com>
---
 drivers/gpio/gpio-dwapb.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 7b92b233fafe..dec700e3cfb0 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -209,8 +209,20 @@ static u32 dwapb_do_irq(struct dwapb_gpio *gpio)
 	for_each_set_bit(hwirq, &irq_status, DWAPB_MAX_GPIOS) {
 		int gpio_irq = irq_find_mapping(gen_gc->gc.irq.domain, hwirq);
 		u32 irq_type = irq_get_trigger_type(gpio_irq);
-
-		generic_handle_irq(gpio_irq);
+		int ret;
+		u32 val_intmask, val_inten;
+
+		ret = generic_handle_irq(gpio_irq);
+		if (ret) {
+			dev_warn_ratelimited(gpio->dev, "Failed to handle irq %d\n", gpio_irq);
+			/* Clear the interrupt */
+			dwapb_write(gpio, GPIO_PORTA_EOI, BIT(hwirq));
+			val_intmask = dwapb_read(gpio, GPIO_INTMASK);
+			dwapb_write(gpio, GPIO_INTMASK, val_intmask | BIT(hwirq));
+			val_inten = dwapb_read(gpio, GPIO_INTEN);
+			dwapb_write(gpio, GPIO_INTEN, val_inten & ~BIT(hwirq));
+			continue;
+		}
 
 		if ((irq_type & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH)
 			dwapb_toggle_trigger(gpio, hwirq);
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] gpio: dwapb: Add robust error handling in interrupt handler

[Linus Walleij]

Hi Liang,

thanks for your patch!

Top posting so tglx get the context...

On Fri, Jul 3, 2026 at 3:49 PM Liang Hao <haohlliang@gmail.com> wrote:

> The current interrupt handler silently continues if an interrupt
> handling fails, which may lead to interrupt storms. Add proper
> error handling to gracefully recover from failed interrupt
> handling.
>
> When generic_handle_irq() fails, the following recovery actions are
> taken:
>   - Write EOI to clear the pending interrupt
>   - Mask the interrupt to prevent immediate re-triggering
>   - Disable the interrupt to stop further interrupts on this line
>
> These measures prevent the system from being overwhelmed by repeated
> unhandled interrupts while logging a rate-limited warning for
> debugging purposes.
>
> Signed-off-by: Liang Hao <haohlliang@gmail.com>
> ---
>  drivers/gpio/gpio-dwapb.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 7b92b233fafe..dec700e3cfb0 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -209,8 +209,20 @@ static u32 dwapb_do_irq(struct dwapb_gpio *gpio)
>         for_each_set_bit(hwirq, &irq_status, DWAPB_MAX_GPIOS) {
>                 int gpio_irq = irq_find_mapping(gen_gc->gc.irq.domain, hwirq);
>                 u32 irq_type = irq_get_trigger_type(gpio_irq);
> -
> -               generic_handle_irq(gpio_irq);
> +               int ret;
> +               u32 val_intmask, val_inten;
> +
> +               ret = generic_handle_irq(gpio_irq);
> +               if (ret) {
> +                       dev_warn_ratelimited(gpio->dev, "Failed to handle irq %d\n", gpio_irq);
> +                       /* Clear the interrupt */
> +                       dwapb_write(gpio, GPIO_PORTA_EOI, BIT(hwirq));
> +                       val_intmask = dwapb_read(gpio, GPIO_INTMASK);
> +                       dwapb_write(gpio, GPIO_INTMASK, val_intmask | BIT(hwirq));
> +                       val_inten = dwapb_read(gpio, GPIO_INTEN);
> +                       dwapb_write(gpio, GPIO_INTEN, val_inten & ~BIT(hwirq));
> +                       continue;
> +               }
>
>                 if ((irq_type & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH)
>                         dwapb_toggle_trigger(gpio, hwirq);

Ugh I don't know if that is how you'r supposed to deal with the return value
from generic_handle_irq(), we better get tglx eyes on this.

Yours,
Linus Walleij

Re: [PATCH] gpio: dwapb: Add robust error handling in interrupt handler

[Thomas Gleixner]

On Fri, Jul 03 2026 at 23:21, Linus Walleij wrote:
> On Fri, Jul 3, 2026 at 3:49 PM Liang Hao <haohlliang@gmail.com> wrote:
>
>> The current interrupt handler silently continues if an interrupt
>> handling fails, which may lead to interrupt storms. Add proper
>> error handling to gracefully recover from failed interrupt
>> handling.
>>
>> When generic_handle_irq() fails, the following recovery actions are
>> taken:
>>   - Write EOI to clear the pending interrupt
>>   - Mask the interrupt to prevent immediate re-triggering
>>   - Disable the interrupt to stop further interrupts on this line

This completely fails to explain WHY generic_handle_irq() fails.

There are only two reasons for that:

      1) The interrupt descriptor is not available (EINVAL)

      2) The platform mandates that the interrupt has to be handled in
         hard interrupt context (EPERM). This also emits a warning.

There is also zero information whether the irq mapping lookup returns a
valid interrupt in this scenario, so it's hard to tell what's really
going on.

As there is no mention of the warning I assume that's #1. Which means
this is papering over some underlying problem in that driver. Looking at
the counter measures makes it entirely clear:

    EOI, mask, disable

If there is no interrupt descriptor then the driver failed to mask and
disable the interrupt line at some point.

So this is just a lazy debugging aid for something which is not supposed
to happen in a sane and production ready driver.

Thanks,

        tglx

Re: [PATCH] gpio: dwapb: Add robust error handling in interrupt handler

[haohlliang]

Hi tglx,

Thank you for the thorough review. You're absolutely right that my
previous approach was limited to handling the symptom at the crash
site without investigating the root cause.

After your explanation, I realized the real problem is that the GPIO
controller does not properly initialize its interrupt state, and stale
interrupt configuration can persist across warm reboots when
peripherals remain powered.

I'll send a v4 that masks all interrupts at hardware initialization
time via the init_hw() callback, which addresses the root cause
rather than the symptom.

Thanks,
Liang

[PATCH v4] gpio: dwapb: Mask interrupts at hardware initialization

[Liang Hao]

GPIO interrupts may retain stale state across warm reboots when
peripherals remain powered. If a GPIO line is not explicitly
configured for interrupts, this can result in interrupt storms
due to missing handlers.

Fix this by ensuring all interrupts are masked and disabled at
hardware initialization time via the init_hw() callback. Pending
interrupts are also cleared to start from a known-safe state.

Interrupts will be unmasked only when explicitly configured by
userspace or kernel drivers.

Signed-off-by: Liang Hao <haohlliang@gmail.com>
---
v3 -> v4:
- Instead of handling errors in the interrupt handler, properly
  initialize hardware interrupt state at probe time via init_hw()
- This addresses the root cause identified by tglx rather than
  papering over the symptom

v2 -> v3:
- Remove duplicate comment

v1 -> v2:
- Add spinlock protection for register access in error path
- Protect against race with irq_chip callbacks accessing shared registers
---
 drivers/gpio/gpio-dwapb.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 7b92b233fafe..3189a0269efc 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -199,6 +199,22 @@ static void dwapb_toggle_trigger(struct dwapb_gpio *gpio, unsigned int offs)
 	dwapb_write(gpio, GPIO_INT_POLARITY, pol);
 }
 
+static int dwapb_irq_init_hw(struct gpio_chip *gc)
+{
+	struct dwapb_gpio *gpio = to_dwapb_gpio(gc);
+
+	/*
+	 * GPIO interrupts may retain stale state across warm reboots when
+	 * peripherals stay powered. Force a known-safe state before the GPIO
+	 * irqchip and irq domain are set up.
+	 */
+	dwapb_write(gpio, GPIO_INTEN, 0);
+	dwapb_write(gpio, GPIO_INTMASK, 0xffffffff);
+	dwapb_write(gpio, GPIO_PORTA_EOI, 0xffffffff);
+
+	return 0;
+}
+
 static u32 dwapb_do_irq(struct dwapb_gpio *gpio)
 {
 	struct gpio_generic_chip *gen_gc = &gpio->ports[0].chip;
@@ -457,6 +473,7 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
 	girq = &gc->irq;
 	girq->handler = handle_bad_irq;
 	girq->default_type = IRQ_TYPE_NONE;
+	girq->init_hw = dwapb_irq_init_hw;
 
 	port->pirq = pirq;
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v4] gpio: dwapb: Mask interrupts at hardware initialization

[Thomas Gleixner]

On Sun, Jul 05 2026 at 15:47, Liang Hao wrote:

> GPIO interrupts may retain stale state across warm reboots when
> peripherals remain powered. If a GPIO line is not explicitly
> configured for interrupts, this can result in interrupt storms
> due to missing handlers.
>
> Fix this by ensuring all interrupts are masked and disabled at
> hardware initialization time via the init_hw() callback. Pending
> interrupts are also cleared to start from a known-safe state.
>
> Interrupts will be unmasked only when explicitly configured by
> userspace or kernel drivers.

That makes way more sense. :)