Thread (3 messages) 3 messages, 2 authors, 2025-07-12

Re: [PATCH] irqchip/imx-irqsteer: Fix irq handling if an error occurs in imx_irqsteer_irq_handler()

From: Marc Zyngier <maz@kernel.org>
Date: 2025-07-12 11:02:54
Also in: imx, kernel-janitors, lkml

On Sat, 12 Jul 2025 10:58:35 +0100,
Christophe JAILLET [off-list ref] wrote:
Le 17/11/2024 à 12:21, Christophe JAILLET a écrit :
quoted
chained_irq_enter(() should be paired with a corresponding
chained_irq_exit().

Here, if (hwirq < 0), a early return occurs and chained_irq_exit() is not
called.
After several month without any feedback, this is a polite ping.
Is this patch correct?
An untested patch is unlikely to make a lot of forward progress, to
be honest.
CJ

quoted
Add a new label and a goto for fix it.

Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output interrupts support")
Signed-off-by: Christophe JAILLET <redacted>
---
Compile tested only.

Review with care, irq handling is sometimes tricky...
---
  drivers/irqchip/irq-imx-irqsteer.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 75a0e980ff35..59abe5a8beb8 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -135,7 +135,7 @@ static void imx_irqsteer_irq_handler(struct irq_desc *desc)
  	if (hwirq < 0) {
  		pr_warn("%s: unable to get hwirq base for irq %d\n",
  			__func__, irq);
-		return;
+		goto out;
  	}
    	for (i = 0; i < 2; i++, hwirq += 32) {
@@ -153,6 +153,7 @@ static void imx_irqsteer_irq_handler(struct irq_desc *desc)
  			generic_handle_domain_irq(data->domain, pos + hwirq);
  	}
  +out:
  	chained_irq_exit(irq_desc_get_chip(desc), desc);
  }
The real question is *how* do you end-up in this situation.

To trigger this case, you need a mux interrupt that is handled by
imx_irqsteer_irq_handler(), but for which you haven't got a
translation from DT the first place. Do you see the chicken and egg
problem?

In summary, this driver is checking for conditions that can't possibly
happen, and this check should simply be deleted instead of being
blindly "fixed".

	M.

-- 
Without deviation from the norm, progress is not possible.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help