[PATCH v2] irqchip/renesas-rzg2l: Fix loss of interrupt

Biju posted 1 patch 1 month, 1 week ago
drivers/irqchip/irq-renesas-rzg2l.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] irqchip/renesas-rzg2l: Fix loss of interrupt
Posted by Biju 1 month, 1 week ago
From: Biju Das <biju.das.jz@bp.renesas.com>

rzg2l_clear_irq_int() and rzg2l_clear_tint_int() perform a
read-modify-write on the ISCR/TSCR status registers to clear the bit
for the interrupt just handled. Since these registers are
write-0-to-clear per bit, this is racy: if another interrupt's status
bit gets set between the read and the write, that bit is written back
as 0 by the software-constructed value, clearing an interrupt that
hasn't been serviced yet and losing it. This can be reproduced by
triggering multiple interrupts at once, e.g.:

  gpioset -c gpiochip0 355=0 353=0 328=0 352=0

Fix this by writing back only the bit being cleared, with all other
bits set to 1, instead of read-modify-writing the whole register.
Since 1-bits are left unchanged by hardware, concurrently-set status
bits for other interrupts are preserved.

Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Updated the commit description.
---
 drivers/irqchip/irq-renesas-rzg2l.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index f6b2e69a2f4e..b3774f82855a 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -161,7 +161,7 @@ static void rzg2l_clear_irq_int(struct rzg2l_irqc_priv *priv, unsigned int hwirq
 	 * falling/rising-edge.
 	 */
 	if ((iscr & bit) && (iitsr & IITSR_IITSEL_MASK(hw_irq))) {
-		writel_relaxed(iscr & ~bit, priv->base + ISCR);
+		writel_relaxed(~bit, priv->base + ISCR);
 		/*
 		 * Enforce that the posted write is flushed to prevent that the
 		 * just handled interrupt is raised again.
@@ -177,7 +177,7 @@ static void rzg2l_clear_tint_int(struct rzg2l_irqc_priv *priv, unsigned int hwir
 
 	reg = readl_relaxed(priv->base + TSCR);
 	if (reg & bit) {
-		writel_relaxed(reg & ~bit, priv->base + TSCR);
+		writel_relaxed(~bit, priv->base + TSCR);
 		/*
 		 * Enforce that the posted write is flushed to prevent that the
 		 * just handled interrupt is raised again.
-- 
2.43.0
Re: [PATCH v2] irqchip/renesas-rzg2l: Fix loss of interrupt
Posted by Radu Rendec 1 month, 1 week ago
On Tue, 2026-08-18 at 12:09 +0100, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> rzg2l_clear_irq_int() and rzg2l_clear_tint_int() perform a
> read-modify-write on the ISCR/TSCR status registers to clear the bit
> for the interrupt just handled. Since these registers are
> write-0-to-clear per bit, this is racy: if another interrupt's status
> bit gets set between the read and the write, that bit is written back
> as 0 by the software-constructed value, clearing an interrupt that
> hasn't been serviced yet and losing it. This can be reproduced by
> triggering multiple interrupts at once, e.g.:
> 
>   gpioset -c gpiochip0 355=0 353=0 328=0 352=0
> 
> Fix this by writing back only the bit being cleared, with all other
> bits set to 1, instead of read-modify-writing the whole register.
> Since 1-bits are left unchanged by hardware, concurrently-set status
> bits for other interrupts are preserved.
> 
> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
>  * Updated the commit description.
> ---
>  drivers/irqchip/irq-renesas-rzg2l.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> index f6b2e69a2f4e..b3774f82855a 100644
> --- a/drivers/irqchip/irq-renesas-rzg2l.c
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -161,7 +161,7 @@ static void rzg2l_clear_irq_int(struct rzg2l_irqc_priv *priv, unsigned int hwirq
>  	 * falling/rising-edge.
>  	 */
>  	if ((iscr & bit) && (iitsr & IITSR_IITSEL_MASK(hw_irq))) {
> -		writel_relaxed(iscr & ~bit, priv->base + ISCR);
> +		writel_relaxed(~bit, priv->base + ISCR);
>  		/*
>  		 * Enforce that the posted write is flushed to prevent that the
>  		 * just handled interrupt is raised again.
> @@ -177,7 +177,7 @@ static void rzg2l_clear_tint_int(struct rzg2l_irqc_priv *priv, unsigned int hwir
>  
>  	reg = readl_relaxed(priv->base + TSCR);
>  	if (reg & bit) {
> -		writel_relaxed(reg & ~bit, priv->base + TSCR);
> +		writel_relaxed(~bit, priv->base + TSCR);
>  		/*
>  		 * Enforce that the posted write is flushed to prevent that the
>  		 * just handled interrupt is raised again.

Reviewed-by: Radu Rendec <radu@rendec.net>
[tip: irq/urgent] irqchip/renesas-rzg2l: Fix loss of interrupt
Posted by tip-bot2 for Biju Das 1 month, 1 week ago
The following commit has been merged into the irq/urgent branch of tip:

Commit-ID:     50b10bd0c2d721ad38abd1abe3acdefb6caa0944
Gitweb:        https://git.kernel.org/tip/50b10bd0c2d721ad38abd1abe3acdefb6caa0944
Author:        Biju Das <biju.das.jz@bp.renesas.com>
AuthorDate:    Tue, 18 Aug 2026 12:09:33 +01:00
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 19 Aug 2026 21:40:13 +02:00

irqchip/renesas-rzg2l: Fix loss of interrupt

rzg2l_clear_irq_int() and rzg2l_clear_tint_int() perform a
read-modify-write on the ISCR/TSCR status registers to clear the bit
for the interrupt just handled. Since these registers are
write-0-to-clear per bit, this is racy:

If another interrupt's status bit gets set between the read and the write,
that bit is written back as 0 by the software-constructed value, clearing
an interrupt that hasn't been serviced yet and losing it.

This can be reproduced by triggering multiple interrupts at once, e.g.:

  gpioset -c gpiochip0 355=0 353=0 328=0 352=0

Fix this by writing back only the bit being cleared, with all other bits
set to 1, instead of read-modify-writing the whole register.  Since 1-bits
are left unchanged by hardware, concurrently-set status bits for other
interrupts are preserved.

Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Link: https://patch.msgid.link/20260818110937.5816-1-biju.das.jz@bp.renesas.com
---
 drivers/irqchip/irq-renesas-rzg2l.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index f6b2e69..b3774f8 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -161,7 +161,7 @@ static void rzg2l_clear_irq_int(struct rzg2l_irqc_priv *priv, unsigned int hwirq
 	 * falling/rising-edge.
 	 */
 	if ((iscr & bit) && (iitsr & IITSR_IITSEL_MASK(hw_irq))) {
-		writel_relaxed(iscr & ~bit, priv->base + ISCR);
+		writel_relaxed(~bit, priv->base + ISCR);
 		/*
 		 * Enforce that the posted write is flushed to prevent that the
 		 * just handled interrupt is raised again.
@@ -177,7 +177,7 @@ static void rzg2l_clear_tint_int(struct rzg2l_irqc_priv *priv, unsigned int hwir
 
 	reg = readl_relaxed(priv->base + TSCR);
 	if (reg & bit) {
-		writel_relaxed(reg & ~bit, priv->base + TSCR);
+		writel_relaxed(~bit, priv->base + TSCR);
 		/*
 		 * Enforce that the posted write is flushed to prevent that the
 		 * just handled interrupt is raised again.