[PATCH 0/3] Fix IRQ storm with GPIO interrupts

[PATCH 0/3] Fix IRQ storm with GPIO interrupts

[Biju Das]

The following issues observed while adding IRQ support for RTC.
 * The irq_disable is not clearing interrupt source properly.
 * The driver is not following as per hardware manual for changing
   interrupt settings.
 * IRQ storm due to phantum interrupt, when we select the TINT source.
   Here IRQ handler disables  the interrupts using disable_irq_nosync()
   and scheduling a work queue and in the work queue, re-enabling the
   interrupt with enable_irq().

Biju Das (3):
  irqchip: renesas-rzg2l: Fix logic to clear TINT interrupt source
  irqchip: renesas-rzg2l: Mask interrupts for changing interrupt
    settings
  irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for
    TINT

 drivers/irqchip/irq-renesas-rzg2l.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH 1/3] irqchip: renesas-rzg2l: Fix logic to clear TINT interrupt source

[Biju Das]

The logic to clear the TINT interrupt source in rzg2l_irqc_irq_disable()
is wrong as the mask is correct only for LSB on the TSSR register.
This issue is found when testing with two TINT interrupt sources. So fix
the logic for all TINTs by using the macro TSSEL_SHIFT() to multiply
tssr_offset with 8.

Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 drivers/irqchip/irq-renesas-rzg2l.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index 4bbfa2b0a4df..2cee5477be6b 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -118,7 +118,7 @@ static void rzg2l_irqc_irq_disable(struct irq_data *d)
 
 		raw_spin_lock(&priv->lock);
 		reg = readl_relaxed(priv->base + TSSR(tssr_index));
-		reg &= ~(TSSEL_MASK << tssr_offset);
+		reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset));
 		writel_relaxed(reg, priv->base + TSSR(tssr_index));
 		raw_spin_unlock(&priv->lock);
 	}
-- 
2.25.1

Re: [PATCH 1/3] irqchip: renesas-rzg2l: Fix logic to clear TINT interrupt source

[Geert Uytterhoeven]

On Mon, Sep 18, 2023 at 2:24 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The logic to clear the TINT interrupt source in rzg2l_irqc_irq_disable()
> is wrong as the mask is correct only for LSB on the TSSR register.
> This issue is found when testing with two TINT interrupt sources. So fix
> the logic for all TINTs by using the macro TSSEL_SHIFT() to multiply
> tssr_offset with 8.
>
> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] irqchip: renesas-rzg2l: Fix logic to clear TINT interrupt source

[claudiu beznea]

On 18.09.2023 15:24, Biju Das wrote:
> The logic to clear the TINT interrupt source in rzg2l_irqc_irq_disable()
> is wrong as the mask is correct only for LSB on the TSSR register.
> This issue is found when testing with two TINT interrupt sources. So fix
> the logic for all TINTs by using the macro TSSEL_SHIFT() to multiply
> tssr_offset with 8.
> 
> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

> ---
>  drivers/irqchip/irq-renesas-rzg2l.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> index 4bbfa2b0a4df..2cee5477be6b 100644
> --- a/drivers/irqchip/irq-renesas-rzg2l.c
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -118,7 +118,7 @@ static void rzg2l_irqc_irq_disable(struct irq_data *d)
>  
>  		raw_spin_lock(&priv->lock);
>  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> -		reg &= ~(TSSEL_MASK << tssr_offset);
> +		reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset));
>  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
>  		raw_spin_unlock(&priv->lock);
>  	}

[irqchip: irq/irqchip-fixes] irqchip: renesas-rzg2l: Fix logic to clear TINT interrupt source

[irqchip-bot for Biju Das]

The following commit has been merged into the irq/irqchip-fixes branch of irqchip:

Commit-ID:     9b8df572ba3f4e544366196820a719a40774433e
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/9b8df572ba3f4e544366196820a719a40774433e
Author:        Biju Das <biju.das.jz@bp.renesas.com>
AuthorDate:    Mon, 18 Sep 2023 13:24:09 +01:00
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Sun, 24 Sep 2023 10:18:19 +01:00

irqchip: renesas-rzg2l: Fix logic to clear TINT interrupt source

The logic to clear the TINT interrupt source in rzg2l_irqc_irq_disable()
is wrong as the mask is correct only for LSB on the TSSR register.
This issue is found when testing with two TINT interrupt sources. So fix
the logic for all TINTs by using the macro TSSEL_SHIFT() to multiply
tssr_offset with 8.

Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20230918122411.237635-2-biju.das.jz@bp.renesas.com
---
 drivers/irqchip/irq-renesas-rzg2l.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index 4bbfa2b..2cee547 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -118,7 +118,7 @@ static void rzg2l_irqc_irq_disable(struct irq_data *d)
 
 		raw_spin_lock(&priv->lock);
 		reg = readl_relaxed(priv->base + TSSR(tssr_index));
-		reg &= ~(TSSEL_MASK << tssr_offset);
+		reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset));
 		writel_relaxed(reg, priv->base + TSSR(tssr_index));
 		raw_spin_unlock(&priv->lock);
 	}

[PATCH 2/3] irqchip: renesas-rzg2l: Mask interrupts for changing interrupt settings

[Biju Das]

As per RZ/G2L hardware manual Rev.1.30 section 8.8.3 Precaution when
changing interrupt settings,  we need to mask the interrupts for
any changes in below settings:

 * When changing the noise filter settings.
 * When switching the GPIO pins to IRQ or GPIOINT.
 * When changing the source of TINT.
 * When changing the interrupt detection method.

This patch masks the interrupts when there is a change in the interrupt
detection method and changing the source of TINT.

Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index 2cee5477be6b..33a22bafedcd 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -116,11 +116,13 @@ static void rzg2l_irqc_irq_disable(struct irq_data *d)
 		u8 tssr_index = TSSR_INDEX(offset);
 		u32 reg;
 
+		irq_chip_mask_parent(d);
 		raw_spin_lock(&priv->lock);
 		reg = readl_relaxed(priv->base + TSSR(tssr_index));
 		reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset));
 		writel_relaxed(reg, priv->base + TSSR(tssr_index));
 		raw_spin_unlock(&priv->lock);
+		irq_chip_unmask_parent(d);
 	}
 	irq_chip_disable_parent(d);
 }
@@ -137,11 +139,13 @@ static void rzg2l_irqc_irq_enable(struct irq_data *d)
 		u8 tssr_index = TSSR_INDEX(offset);
 		u32 reg;
 
+		irq_chip_mask_parent(d);
 		raw_spin_lock(&priv->lock);
 		reg = readl_relaxed(priv->base + TSSR(tssr_index));
 		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
 		writel_relaxed(reg, priv->base + TSSR(tssr_index));
 		raw_spin_unlock(&priv->lock);
+		irq_chip_unmask_parent(d);
 	}
 	irq_chip_enable_parent(d);
 }
@@ -226,10 +230,12 @@ static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
 	unsigned int hw_irq = irqd_to_hwirq(d);
 	int ret = -EINVAL;
 
+	irq_chip_mask_parent(d);
 	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
 		ret = rzg2l_irq_set_type(d, type);
 	else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ)
 		ret = rzg2l_tint_set_edge(d, type);
+	irq_chip_unmask_parent(d);
 	if (ret)
 		return ret;
 
-- 
2.25.1

Re: [PATCH 2/3] irqchip: renesas-rzg2l: Mask interrupts for changing interrupt settings

[Marc Zyngier]

On Mon, 18 Sep 2023 13:24:10 +0100,
Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> As per RZ/G2L hardware manual Rev.1.30 section 8.8.3 Precaution when
> changing interrupt settings,  we need to mask the interrupts for
> any changes in below settings:
> 
>  * When changing the noise filter settings.
>  * When switching the GPIO pins to IRQ or GPIOINT.
>  * When changing the source of TINT.
>  * When changing the interrupt detection method.
> 
> This patch masks the interrupts when there is a change in the interrupt
> detection method and changing the source of TINT.
> 
> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>  drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> index 2cee5477be6b..33a22bafedcd 100644
> --- a/drivers/irqchip/irq-renesas-rzg2l.c
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -116,11 +116,13 @@ static void rzg2l_irqc_irq_disable(struct irq_data *d)
>  		u8 tssr_index = TSSR_INDEX(offset);
>  		u32 reg;
>  
> +		irq_chip_mask_parent(d);
>  		raw_spin_lock(&priv->lock);
>  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
>  		reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset));
>  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
>  		raw_spin_unlock(&priv->lock);
> +		irq_chip_unmask_parent(d);

What guarantees that the parent irqchip state has been correctly restored?
Nothing refcounts the nesting of mask/unmask.

>  	}
>  	irq_chip_disable_parent(d);

I'd rather you start by *disabling* the parent, and then none of that
matters at all.

>  }
> @@ -137,11 +139,13 @@ static void rzg2l_irqc_irq_enable(struct irq_data *d)
>  		u8 tssr_index = TSSR_INDEX(offset);
>  		u32 reg;
>  
> +		irq_chip_mask_parent(d);
>  		raw_spin_lock(&priv->lock);
>  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
>  		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
>  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
>  		raw_spin_unlock(&priv->lock);
> +		irq_chip_unmask_parent(d);
>  	}
>  	irq_chip_enable_parent(d);

Same thing: if the parent was disabled, why do we need to do anything?


>  }
> @@ -226,10 +230,12 @@ static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
>  	unsigned int hw_irq = irqd_to_hwirq(d);
>  	int ret = -EINVAL;
>  
> +	irq_chip_mask_parent(d);
>  	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
>  		ret = rzg2l_irq_set_type(d, type);
>  	else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ)
>  		ret = rzg2l_tint_set_edge(d, type);
> +	irq_chip_unmask_parent(d);

This one is the only interesting one: why don't you mask the interrupt
at the local level rather than on the parent? And this should be
conditioned on the interrupt state itself (enabled or disabled), not
done unconditionally.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

RE: [PATCH 2/3] irqchip: renesas-rzg2l: Mask interrupts for changing interrupt settings

[Biju Das]

Hi Marc Zyngier,

Thanks for the feedback.

> Subject: Re: [PATCH 2/3] irqchip: renesas-rzg2l: Mask interrupts for
> changing interrupt settings
> 
> On Mon, 18 Sep 2023 13:24:10 +0100,
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > As per RZ/G2L hardware manual Rev.1.30 section 8.8.3 Precaution when
> > changing interrupt settings,  we need to mask the interrupts for any
> > changes in below settings:
> >
> >  * When changing the noise filter settings.
> >  * When switching the GPIO pins to IRQ or GPIOINT.
> >  * When changing the source of TINT.
> >  * When changing the interrupt detection method.
> >
> > This patch masks the interrupts when there is a change in the
> > interrupt detection method and changing the source of TINT.
> >
> > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller
> > driver")
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > ---
> >  drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> > b/drivers/irqchip/irq-renesas-rzg2l.c
> > index 2cee5477be6b..33a22bafedcd 100644
> > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > @@ -116,11 +116,13 @@ static void rzg2l_irqc_irq_disable(struct irq_data
> *d)
> >  		u8 tssr_index = TSSR_INDEX(offset);
> >  		u32 reg;
> >
> > +		irq_chip_mask_parent(d);
> >  		raw_spin_lock(&priv->lock);
> >  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> >  		reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset));
> >  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> >  		raw_spin_unlock(&priv->lock);
> > +		irq_chip_unmask_parent(d);
> 
> What guarantees that the parent irqchip state has been correctly restored?
> Nothing refcounts the nesting of mask/unmask.
> 
> >  	}
> >  	irq_chip_disable_parent(d);
> 
> I'd rather you start by *disabling* the parent, and then none of that
> matters at all.

Agreed. Will do this in next version.

> 
> >  }
> > @@ -137,11 +139,13 @@ static void rzg2l_irqc_irq_enable(struct irq_data
> *d)
> >  		u8 tssr_index = TSSR_INDEX(offset);
> >  		u32 reg;
> >
> > +		irq_chip_mask_parent(d);
> >  		raw_spin_lock(&priv->lock);
> >  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> >  		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
> >  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> >  		raw_spin_unlock(&priv->lock);
> > +		irq_chip_unmask_parent(d);
> >  	}
> >  	irq_chip_enable_parent(d);
> 
> Same thing: if the parent was disabled, why do we need to do anything?

OK. It is not required.

> 
> 
> >  }
> > @@ -226,10 +230,12 @@ static int rzg2l_irqc_set_type(struct irq_data *d,
> unsigned int type)
> >  	unsigned int hw_irq = irqd_to_hwirq(d);
> >  	int ret = -EINVAL;
> >
> > +	irq_chip_mask_parent(d);
> >  	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> >  		ret = rzg2l_irq_set_type(d, type);
> >  	else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ)
> >  		ret = rzg2l_tint_set_edge(d, type);
> > +	irq_chip_unmask_parent(d);
> 
> This one is the only interesting one: why don't you mask the interrupt at
> the local level rather than on the parent? And this should be conditioned
> on the interrupt state itself (enabled or disabled), not done
> unconditionally.

OK. Will do this locally by conditioned on the interrupt state.

Cheers,
Biju

[PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Biju Das]

In case of edge trigger detection, enabling the TINT source causes a
phantum interrupt that leads to irq storm. So clear the phantum interrupt
in rzg2l_irqc_irq_enable().

This issue is observed when the irq handler disables the interrupts using
disable_irq_nosync() and scheduling a work queue and in the work queue,
re-enabling the interrupt with enable_irq().

Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index 33a22bafedcd..78a9e90512a6 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -144,6 +144,12 @@ static void rzg2l_irqc_irq_enable(struct irq_data *d)
 		reg = readl_relaxed(priv->base + TSSR(tssr_index));
 		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
 		writel_relaxed(reg, priv->base + TSSR(tssr_index));
+		/*
+		 * In case of edge trigger detection, enabling the TINT source
+		 * cause a phantum interrupt that leads to irq storm. So clear
+		 * the phantum interrupt.
+		 */
+		rzg2l_tint_eoi(d);
 		raw_spin_unlock(&priv->lock);
 		irq_chip_unmask_parent(d);
 	}
-- 
2.25.1

Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Marc Zyngier]

On Mon, 18 Sep 2023 13:24:11 +0100,
Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> In case of edge trigger detection, enabling the TINT source causes a
> phantum interrupt that leads to irq storm. So clear the phantum interrupt
> in rzg2l_irqc_irq_enable().
> 
> This issue is observed when the irq handler disables the interrupts using
> disable_irq_nosync() and scheduling a work queue and in the work queue,
> re-enabling the interrupt with enable_irq().
> 
> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>  drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> index 33a22bafedcd..78a9e90512a6 100644
> --- a/drivers/irqchip/irq-renesas-rzg2l.c
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -144,6 +144,12 @@ static void rzg2l_irqc_irq_enable(struct irq_data *d)
>  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
>  		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
>  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> +		/*
> +		 * In case of edge trigger detection, enabling the TINT source
> +		 * cause a phantum interrupt that leads to irq storm. So clear
> +		 * the phantum interrupt.
> +		 */
> +		rzg2l_tint_eoi(d);

This looks incredibly unsafe. disable_irq()+enable_irq() with an
interrupt being made pending in the middle, and you've lost that
interrupt.

What prevents this scenario?

	M.

-- 
Without deviation from the norm, progress is not possible.

RE: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Biju Das]

Hi Marc Zyngier,

Thanks for the feedback.

> Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
> trigger detection for TINT
> 
> On Mon, 18 Sep 2023 13:24:11 +0100,
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > In case of edge trigger detection, enabling the TINT source causes a
> > phantum interrupt that leads to irq storm. So clear the phantum
> > interrupt in rzg2l_irqc_irq_enable().
> >
> > This issue is observed when the irq handler disables the interrupts
> > using
> > disable_irq_nosync() and scheduling a work queue and in the work
> > queue, re-enabling the interrupt with enable_irq().
> >
> > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller
> > driver")
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > ---
> >  drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> > b/drivers/irqchip/irq-renesas-rzg2l.c
> > index 33a22bafedcd..78a9e90512a6 100644
> > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > @@ -144,6 +144,12 @@ static void rzg2l_irqc_irq_enable(struct irq_data
> *d)
> >  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> >  		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
> >  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > +		/*
> > +		 * In case of edge trigger detection, enabling the TINT source
> > +		 * cause a phantum interrupt that leads to irq storm. So clear
> > +		 * the phantum interrupt.
> > +		 */
> > +		rzg2l_tint_eoi(d);
> 
> This looks incredibly unsafe. disable_irq()+enable_irq() with an interrupt
> being made pending in the middle, and you've lost that interrupt.

In this driver that will never happen as it clears the TINT source
during disable(), so there won't be any TINT source for interrupt detection after disable().

Cheers,
Biju

> What prevents this scenario?

Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Marc Zyngier]

On Tue, 19 Sep 2023 16:24:53 +0100,
Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> Hi Marc Zyngier,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
> > trigger detection for TINT
> > 
> > On Mon, 18 Sep 2023 13:24:11 +0100,
> > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > >
> > > In case of edge trigger detection, enabling the TINT source causes a
> > > phantum interrupt that leads to irq storm. So clear the phantum
> > > interrupt in rzg2l_irqc_irq_enable().
> > >
> > > This issue is observed when the irq handler disables the interrupts
> > > using
> > > disable_irq_nosync() and scheduling a work queue and in the work
> > > queue, re-enabling the interrupt with enable_irq().
> > >
> > > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller
> > > driver")
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > > ---
> > >  drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> > > b/drivers/irqchip/irq-renesas-rzg2l.c
> > > index 33a22bafedcd..78a9e90512a6 100644
> > > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > @@ -144,6 +144,12 @@ static void rzg2l_irqc_irq_enable(struct irq_data
> > *d)
> > >  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > >  		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
> > >  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > > +		/*
> > > +		 * In case of edge trigger detection, enabling the TINT source
> > > +		 * cause a phantum interrupt that leads to irq storm. So clear
> > > +		 * the phantum interrupt.
> > > +		 */
> > > +		rzg2l_tint_eoi(d);
> > 
> > This looks incredibly unsafe. disable_irq()+enable_irq() with an interrupt
> > being made pending in the middle, and you've lost that interrupt.
> 
> In this driver that will never happen as it clears the TINT source
> during disable(), so there won't be any TINT source for interrupt
> detection after disable().

So you mean that you *already* lose interrupts across a disable
followed by an enable? I'm slightly puzzled...

	M.

-- 
Without deviation from the norm, progress is not possible.

RE: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Biju Das]

Hi Marc Zyngier,

> Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
> trigger detection for TINT
> 
> On Tue, 19 Sep 2023 16:24:53 +0100,
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Marc Zyngier,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with
> > > edge trigger detection for TINT
> > >
> > > On Mon, 18 Sep 2023 13:24:11 +0100,
> > > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > >
> > > > In case of edge trigger detection, enabling the TINT source causes
> > > > a phantum interrupt that leads to irq storm. So clear the phantum
> > > > interrupt in rzg2l_irqc_irq_enable().
> > > >
> > > > This issue is observed when the irq handler disables the
> > > > interrupts using
> > > > disable_irq_nosync() and scheduling a work queue and in the work
> > > > queue, re-enabling the interrupt with enable_irq().
> > > >
> > > > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt
> > > > Controller
> > > > driver")
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > > > ---
> > > >  drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> > > > b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > index 33a22bafedcd..78a9e90512a6 100644
> > > > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > @@ -144,6 +144,12 @@ static void rzg2l_irqc_irq_enable(struct
> > > > irq_data
> > > *d)
> > > >  		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > > >  		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
> > > >  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > > > +		/*
> > > > +		 * In case of edge trigger detection, enabling the TINT
> source
> > > > +		 * cause a phantum interrupt that leads to irq storm. So
> clear
> > > > +		 * the phantum interrupt.
> > > > +		 */
> > > > +		rzg2l_tint_eoi(d);
> > >
> > > This looks incredibly unsafe. disable_irq()+enable_irq() with an
> > > interrupt being made pending in the middle, and you've lost that
> interrupt.
> >
> > In this driver that will never happen as it clears the TINT source
> > during disable(), so there won't be any TINT source for interrupt
> > detection after disable().
> 
> So you mean that you *already* lose interrupts across a disable followed by
> an enable? I'm slightly puzzled...

There is no interrupt lost at all. 

Currently this patch addresses 2 issues.

Scenario 1: Extra interrupt when we select TINT source on enable_irq()

Getting an extra interrupt, when client drivers calls enable_irq() during probe()/resume(). In this case, the irq handler on the
Client driver just clear the interrupt status bit.

Issue 2: IRQ storm when we select TINT source on enable_irq()

Here as well, we are getting an extra interrupt, when client drivers calls enable_irq() during probe() and this Interrupts getting generated infinitely, when the client driver calls disable_irq() in irq handler and in in work queue calling enable_irq().

Currently we are not loosing interrupts, but we are getting additional
Interrupt(phantom) which is causing the issue.

Cheers,
Biju

Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Marc Zyngier]

On Tue, 19 Sep 2023 17:32:05 +0100,
Biju Das <biju.das.jz@bp.renesas.com> wrote:

[...]

> > So you mean that you *already* lose interrupts across a disable followed by
> > an enable? I'm slightly puzzled...
> 
> There is no interrupt lost at all. 
> 
> Currently this patch addresses 2 issues.
> 
> Scenario 1: Extra interrupt when we select TINT source on enable_irq()
> 
> Getting an extra interrupt, when client drivers calls enable_irq()
> during probe()/resume(). In this case, the irq handler on the Client
> driver just clear the interrupt status bit.
>
> Issue 2: IRQ storm when we select TINT source on enable_irq()
> 
> Here as well, we are getting an extra interrupt, when client drivers
> calls enable_irq() during probe() and this Interrupts getting
> generated infinitely, when the client driver calls disable_irq() in
> irq handler and in in work queue calling enable_irq().

How do you know this is a spurious interrupt? For all you can tell,
you are just consuming an edge. I absolutely don't buy this
workaround, because you have no context that allows you to
discriminate between a real spurious interrupt and a normal interrupt
that lands while the interrupt line was masked.

> Currently we are not loosing interrupts, but we are getting additional
> Interrupt(phantom) which is causing the issue.

If you get an interrupt at probe time in the endpoint driver, that's
probably because the device is not in a quiescent state when the
interrupt is requested. And it is probably this that needs addressing.

	M.

-- 
Without deviation from the norm, progress is not possible.

RE: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Biju Das]

Hi Marc Zyngier,

> Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
> trigger detection for TINT
> 
> On Tue, 19 Sep 2023 17:32:05 +0100,
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> [...]
> 
> > > So you mean that you *already* lose interrupts across a disable
> > > followed by an enable? I'm slightly puzzled...
> >
> > There is no interrupt lost at all.
> >
> > Currently this patch addresses 2 issues.
> >
> > Scenario 1: Extra interrupt when we select TINT source on enable_irq()
> >
> > Getting an extra interrupt, when client drivers calls enable_irq()
> > during probe()/resume(). In this case, the irq handler on the Client
> > driver just clear the interrupt status bit.
> >
> > Issue 2: IRQ storm when we select TINT source on enable_irq()
> >
> > Here as well, we are getting an extra interrupt, when client drivers
> > calls enable_irq() during probe() and this Interrupts getting
> > generated infinitely, when the client driver calls disable_irq() in
> > irq handler and in in work queue calling enable_irq().
> 
> How do you know this is a spurious interrupt? 

We have PMOD on RZ/G2L SMARC EVK. So I connected it to GPIO pin
and other end to ground. During the boot, I get an interrupt
even though there is no high to low transition, when the IRQ is setup
in the probe(). From this it is a spurious interrupt.

> For all you can tell, you are
> just consuming an edge. I absolutely don't buy this workaround, because you
> have no context that allows you to discriminate between a real spurious
> interrupt and a normal interrupt that lands while the interrupt line was
> masked.
> 
> > Currently we are not loosing interrupts, but we are getting additional
> > Interrupt(phantom) which is causing the issue.
> 
> If you get an interrupt at probe time in the endpoint driver, that's
> probably because the device is not in a quiescent state when the interrupt
> is requested. And it is probably this that needs addressing.

Any pointer for addressing this issue? 

Thanks for your help.

Cheers,
Biju

Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Marc Zyngier]

On Tue, 19 Sep 2023 18:06:54 +0100,
Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> Hi Marc Zyngier,
> 
> > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
> > trigger detection for TINT
> > 
> > On Tue, 19 Sep 2023 17:32:05 +0100,
> > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > 
> > [...]
> > 
> > > > So you mean that you *already* lose interrupts across a disable
> > > > followed by an enable? I'm slightly puzzled...
> > >
> > > There is no interrupt lost at all.
> > >
> > > Currently this patch addresses 2 issues.
> > >
> > > Scenario 1: Extra interrupt when we select TINT source on enable_irq()
> > >
> > > Getting an extra interrupt, when client drivers calls enable_irq()
> > > during probe()/resume(). In this case, the irq handler on the Client
> > > driver just clear the interrupt status bit.
> > >
> > > Issue 2: IRQ storm when we select TINT source on enable_irq()
> > >
> > > Here as well, we are getting an extra interrupt, when client drivers
> > > calls enable_irq() during probe() and this Interrupts getting
> > > generated infinitely, when the client driver calls disable_irq() in
> > > irq handler and in in work queue calling enable_irq().
> > 
> > How do you know this is a spurious interrupt? 
> 
> We have PMOD on RZ/G2L SMARC EVK. So I connected it to GPIO pin
> and other end to ground. During the boot, I get an interrupt
> even though there is no high to low transition, when the IRQ is setup
> in the probe(). From this it is a spurious interrupt.

That doesn't really handle my question. At the point of enabling the
interrupt and consuming the edge (which is what this patch does), how
do you know you can readily discard this signal? This is a genuine
question.

Spurious interrupts at boot are common. The HW resets in a funky,
unspecified state, and it's SW's job to initialise it before letting
other agents in the system use interrupts.

> 
> > For all you can tell, you are
> > just consuming an edge. I absolutely don't buy this workaround, because you
> > have no context that allows you to discriminate between a real spurious
> > interrupt and a normal interrupt that lands while the interrupt line was
> > masked.
> > 
> > > Currently we are not loosing interrupts, but we are getting additional
> > > Interrupt(phantom) which is causing the issue.
> > 
> > If you get an interrupt at probe time in the endpoint driver, that's
> > probably because the device is not in a quiescent state when the interrupt
> > is requested. And it is probably this that needs addressing.
> 
> Any pointer for addressing this issue? 

Nothing but the most basic stuff: you should make sure that the
interrupt isn't enabled before you can actually handle it, and triage
it as spurious.

	M.

-- 
Without deviation from the norm, progress is not possible.

RE: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Biju Das]

Hi Marc Zyngier,

Thanks for the feedback.

> Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
> trigger detection for TINT
> 
> On Tue, 19 Sep 2023 18:06:54 +0100,
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Marc Zyngier,
> >
> > > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with
> > > edge trigger detection for TINT
> > >
> > > On Tue, 19 Sep 2023 17:32:05 +0100,
> > > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > >
> > > [...]
> > >
> > > > > So you mean that you *already* lose interrupts across a disable
> > > > > followed by an enable? I'm slightly puzzled...
> > > >
> > > > There is no interrupt lost at all.
> > > >
> > > > Currently this patch addresses 2 issues.
> > > >
> > > > Scenario 1: Extra interrupt when we select TINT source on
> > > > enable_irq()
> > > >
> > > > Getting an extra interrupt, when client drivers calls enable_irq()
> > > > during probe()/resume(). In this case, the irq handler on the
> > > > Client driver just clear the interrupt status bit.
> > > >
> > > > Issue 2: IRQ storm when we select TINT source on enable_irq()
> > > >
> > > > Here as well, we are getting an extra interrupt, when client
> > > > drivers calls enable_irq() during probe() and this Interrupts
> > > > getting generated infinitely, when the client driver calls
> > > > disable_irq() in irq handler and in in work queue calling
> enable_irq().
> > >
> > > How do you know this is a spurious interrupt?
> >
> > We have PMOD on RZ/G2L SMARC EVK. So I connected it to GPIO pin and
> > other end to ground. During the boot, I get an interrupt even though
> > there is no high to low transition, when the IRQ is setup in the
> > probe(). From this it is a spurious interrupt.
> 
> That doesn't really handle my question. At the point of enabling the
> interrupt and consuming the edge (which is what this patch does), how do
> you know you can readily discard this signal? This is a genuine question.
> 
> Spurious interrupts at boot are common. The HW resets in a funky,
> unspecified state, and it's SW's job to initialise it before letting other
> agents in the system use interrupts.

I got your point related to loosing interrupts.

Now I can detect spurious interrupts for edge trigger.

Pin controller driver has a read-only register to monitor input values of GPIO input pins, use that register values before/after rzg2l_irq_enable() with TINT Status Control Register (TSCR)
in IRQ controller to detect the spurious interrupt.

Eg:
1) Check PIN_43_0 value (ex: low)in pinctrl driver
2) Enable the IRQ using rzg2l_irq_enable()/ irq_chip_enable_parent()in pinctrl driver
3) Check PIN_43_0 value (ex: low) in pinctrl driver
4) Check the TINT Status Control Register(TSCR) in IRQ controller driver

     If the values in 1 and 3 are same and the status in 4 is set, then there is a spurious interrupt.

> 
> >
> > > For all you can tell, you are
> > > just consuming an edge. I absolutely don't buy this workaround,
> > > because you have no context that allows you to discriminate between
> > > a real spurious interrupt and a normal interrupt that lands while
> > > the interrupt line was masked.
> > >
> > > > Currently we are not loosing interrupts, but we are getting
> > > > additional
> > > > Interrupt(phantom) which is causing the issue.
> > >
> > > If you get an interrupt at probe time in the endpoint driver, that's
> > > probably because the device is not in a quiescent state when the
> > > interrupt is requested. And it is probably this that needs addressing.
> >
> > Any pointer for addressing this issue?
> 
> Nothing but the most basic stuff: you should make sure that the interrupt
> isn't enabled before you can actually handle it, and triage it as spurious.

For the GPIO interrupt case I have,

RTC driver(endpoint)--> Pin controller driver -->IRQ controller driver-->GIC controller.

1) I have configured the pin as GPIO interrupts in pin controller driver
2) Set the IRQ detection in IRQ controller for edge trigger
3) The moment I set the IRQ source in IRQ controller 
   I get an interrupt, even though there is no voltage transition.

Here the system is setup properly, but there is a spurious interrupt. Currently don't know how to handle it? 

Any pointers for handling this issue?

Note:
 Currently the pin controller driver is not configuring GPIO as GPIO input in Port Mode Register for the GPIO interrupts instead it is using reset value which is "Hi-Z". I will send a patch to fix it.

Cheers,
Biju

RE: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge trigger detection for TINT

[Biju Das]

Hi Marc,

> Subject: RE: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
> trigger detection for TINT
> 
> Hi Marc Zyngier,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with
> > edge trigger detection for TINT
> >
> > On Tue, 19 Sep 2023 18:06:54 +0100,
> > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > >
> > > Hi Marc Zyngier,
> > >
> > > > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm
> > > > with edge trigger detection for TINT
> > > >
> > > > On Tue, 19 Sep 2023 17:32:05 +0100, Biju Das
> > > > <biju.das.jz@bp.renesas.com> wrote:
> > > >
> > > > [...]
> > > >
> > > > > > So you mean that you *already* lose interrupts across a
> > > > > > disable followed by an enable? I'm slightly puzzled...
> > > > >
> > > > > There is no interrupt lost at all.
> > > > >
> > > > > Currently this patch addresses 2 issues.
> > > > >
> > > > > Scenario 1: Extra interrupt when we select TINT source on
> > > > > enable_irq()
> > > > >
> > > > > Getting an extra interrupt, when client drivers calls
> > > > > enable_irq() during probe()/resume(). In this case, the irq
> > > > > handler on the Client driver just clear the interrupt status bit.
> > > > >
> > > > > Issue 2: IRQ storm when we select TINT source on enable_irq()
> > > > >
> > > > > Here as well, we are getting an extra interrupt, when client
> > > > > drivers calls enable_irq() during probe() and this Interrupts
> > > > > getting generated infinitely, when the client driver calls
> > > > > disable_irq() in irq handler and in in work queue calling
> > enable_irq().
> > > >
> > > > How do you know this is a spurious interrupt?
> > >
> > > We have PMOD on RZ/G2L SMARC EVK. So I connected it to GPIO pin and
> > > other end to ground. During the boot, I get an interrupt even though
> > > there is no high to low transition, when the IRQ is setup in the
> > > probe(). From this it is a spurious interrupt.
> >
> > That doesn't really handle my question. At the point of enabling the
> > interrupt and consuming the edge (which is what this patch does), how
> > do you know you can readily discard this signal? This is a genuine
> question.
> >
> > Spurious interrupts at boot are common. The HW resets in a funky,
> > unspecified state, and it's SW's job to initialise it before letting
> > other agents in the system use interrupts.
> 
> I got your point related to loosing interrupts.
> 
> Now I can detect spurious interrupts for edge trigger.
> 
> Pin controller driver has a read-only register to monitor input values of
> GPIO input pins, use that register values before/after rzg2l_irq_enable()
> with TINT Status Control Register (TSCR) in IRQ controller to detect the
> spurious interrupt.
> 
> Eg:
> 1) Check PIN_43_0 value (ex: low)in pinctrl driver
> 2) Enable the IRQ using rzg2l_irq_enable()/ irq_chip_enable_parent()in
> pinctrl driver
> 3) Check PIN_43_0 value (ex: low) in pinctrl driver
> 4) Check the TINT Status Control Register(TSCR) in IRQ controller driver
> 
>      If the values in 1 and 3 are same and the status in 4 is set, then
> there is a spurious interrupt.
> 
> >
> > >
> > > > For all you can tell, you are
> > > > just consuming an edge. I absolutely don't buy this workaround,
> > > > because you have no context that allows you to discriminate
> > > > between a real spurious interrupt and a normal interrupt that
> > > > lands while the interrupt line was masked.
> > > >
> > > > > Currently we are not loosing interrupts, but we are getting
> > > > > additional
> > > > > Interrupt(phantom) which is causing the issue.
> > > >
> > > > If you get an interrupt at probe time in the endpoint driver,
> > > > that's probably because the device is not in a quiescent state
> > > > when the interrupt is requested. And it is probably this that needs
> addressing.
> > >
> > > Any pointer for addressing this issue?
> >
> > Nothing but the most basic stuff: you should make sure that the
> > interrupt isn't enabled before you can actually handle it, and triage it
> as spurious.
> 
> For the GPIO interrupt case I have,
> 
> RTC driver(endpoint)--> Pin controller driver -->IRQ controller driver--
> >GIC controller.
> 
> 1) I have configured the pin as GPIO interrupts in pin controller driver
> 2) Set the IRQ detection in IRQ controller for edge trigger
> 3) The moment I set the IRQ source in IRQ controller
>    I get an interrupt, even though there is no voltage transition.
> 
> Here the system is setup properly, but there is a spurious interrupt.
> Currently don't know how to handle it?
> 
> Any pointers for handling this issue?
> 
> Note:
>  Currently the pin controller driver is not configuring GPIO as GPIO input
> in Port Mode Register for the GPIO interrupts instead it is using reset
> value which is "Hi-Z". I will send a patch to fix it.

An update, I have found a way to fix the spurious interrupt issue.

Spurious interrupt is generated if we do simultaneous writing of
TINT Source selection and TINT Source enable in TSSRx register.

If we write the register in correct order, then there is no issue.
i.e., first set the TINT Source selection and after that enable it.

Looks like it is a HW race condition. I am checking this issue with HW team.

Cheers,
Biju