* [PATCH] irqchip/imx-irqsteer: Fix irq handling if an error occurs in imx_irqsteer_irq_handler()
@ 2024-11-17 11:21 Christophe JAILLET
2025-07-12 9:58 ` Christophe JAILLET
0 siblings, 1 reply; 3+ messages in thread
From: Christophe JAILLET @ 2024-11-17 11:21 UTC (permalink / raw)
To: Thomas Gleixner, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Lucas Stach, Marc Zyngier, Aisheng Dong
Cc: linux-kernel, kernel-janitors, Christophe JAILLET, imx,
linux-arm-kernel
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.
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 <christophe.jaillet@wanadoo.fr>
---
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);
}
--
2.47.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] irqchip/imx-irqsteer: Fix irq handling if an error occurs in imx_irqsteer_irq_handler()
2024-11-17 11:21 [PATCH] irqchip/imx-irqsteer: Fix irq handling if an error occurs in imx_irqsteer_irq_handler() Christophe JAILLET
@ 2025-07-12 9:58 ` Christophe JAILLET
2025-07-12 11:02 ` Marc Zyngier
0 siblings, 1 reply; 3+ messages in thread
From: Christophe JAILLET @ 2025-07-12 9:58 UTC (permalink / raw)
To: Thomas Gleixner, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Lucas Stach, Marc Zyngier, Aisheng Dong
Cc: linux-kernel, kernel-janitors, imx, linux-arm-kernel
Le 17/11/2024 à 12:21, Christophe JAILLET a écrit :
> 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?
CJ
>
> 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 <christophe.jaillet@wanadoo.fr>
> ---
> 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);
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] irqchip/imx-irqsteer: Fix irq handling if an error occurs in imx_irqsteer_irq_handler()
2025-07-12 9:58 ` Christophe JAILLET
@ 2025-07-12 11:02 ` Marc Zyngier
0 siblings, 0 replies; 3+ messages in thread
From: Marc Zyngier @ 2025-07-12 11:02 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Thomas Gleixner, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Lucas Stach, Aisheng Dong, linux-kernel,
kernel-janitors, imx, linux-arm-kernel
On Sat, 12 Jul 2025 10:58:35 +0100,
Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
>
> Le 17/11/2024 à 12:21, Christophe JAILLET a écrit :
> > 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
>
>
> >
> > 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 <christophe.jaillet@wanadoo.fr>
> > ---
> > 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-12 11:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-17 11:21 [PATCH] irqchip/imx-irqsteer: Fix irq handling if an error occurs in imx_irqsteer_irq_handler() Christophe JAILLET
2025-07-12 9:58 ` Christophe JAILLET
2025-07-12 11:02 ` Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).