| Message ID | 20240212113712.71878-5-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Fix spurious TINT IRQ and enhancements | expand |
On Mon, Feb 12 2024 at 11:37, Biju Das wrote: > Use TIEN for enable/disable and avoid modifying TINT source selection > register. Why? Changelogs are supposed to explain the WHY and not just decribe the WHAT. Thanks, tglx
Hi Thomas Gleixner, > -----Original Message----- > From: Thomas Gleixner <tglx@linutronix.de> > Sent: Friday, March 1, 2024 2:16 PM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Biju Das <biju.das.jz@bp.renesas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev- > lad.rj@bp.renesas.com>; Marc Zyngier <maz@kernel.org>; Geert Uytterhoeven <geert+renesas@glider.be>; > biju.das.au <biju.das.au@gmail.com>; linux-renesas-soc@vger.kernel.org > Subject: Re: [PATCH 4/5] irqchip/renesas-rzg2l: Use TIEN for enable/disable > > On Mon, Feb 12 2024 at 11:37, Biju Das wrote: > > Use TIEN for enable/disable and avoid modifying TINT source selection > > register. > > Why? This will lead to conflict in TINT detection register and TINT source as we are modifying the source. This can also lead to spurious IRQ. > > Changelogs are supposed to explain the WHY and not just decribe the WHAT. OK, will do it in the next version. Cheers, Biju
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c index c48c8e836dd1..fbee400985a9 100644 --- a/drivers/irqchip/irq-renesas-rzg2l.c +++ b/drivers/irqchip/irq-renesas-rzg2l.c @@ -35,7 +35,6 @@ #define TSSR(n) (0x30 + ((n) * 4)) #define TIEN BIT(7) #define TSSEL_SHIFT(n) (8 * (n)) -#define TSSEL_MASK GENMASK(7, 0) #define IRQ_MASK 0x3 #define TSSR_OFFSET(n) ((n) % 4) @@ -178,8 +177,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 << TSSEL_SHIFT(tssr_offset)); - writel_relaxed(reg, priv->base + TSSR(tssr_index)); + rzg2l_tint_endisable(priv, reg, tssr_offset, tssr_index, false); raw_spin_unlock(&priv->lock); } irq_chip_disable_parent(d); @@ -190,7 +188,6 @@ static void rzg2l_irqc_irq_enable(struct irq_data *d) unsigned int hw_irq = irqd_to_hwirq(d); if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) { - unsigned long tint = (uintptr_t)irq_data_get_irq_chip_data(d); struct rzg2l_irqc_priv *priv = irq_data_to_priv(d); u32 offset = hw_irq - IRQC_TINT_START; u32 tssr_offset = TSSR_OFFSET(offset); @@ -199,8 +196,7 @@ static void rzg2l_irqc_irq_enable(struct irq_data *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)); + rzg2l_tint_endisable(priv, reg, tssr_offset, tssr_index, true); raw_spin_unlock(&priv->lock); } irq_chip_enable_parent(d);
Use TIEN for enable/disable and avoid modifying TINT source selection register. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/irqchip/irq-renesas-rzg2l.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)