Hi Thomas Gleixner,
Thanks for the feedback.
> -----Original Message-----
> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Sent: Friday, March 1, 2024 2:39 PM
> Subject: Re: [PATCH 1/5] irqchip/renesas-rzg2l: Prevent IRQ HW race
>
> On Mon, Feb 12 2024 at 11:37, Biju Das wrote:
> > As per section "8.8.2 Clear Timing of Interrupt Cause" of the RZ/G2L
> > hardware manual (Rev.1.45 Jan, 2024), it is mentioned that we need to
> > clear the interrupt cause flag in the isr.
>
> We need to clear? Please write changelogs in neutral tone. Also use proper words instead of acronyms,
> this is not twatter.
OK.
>
> I'm also failing to see the value of above sentence other that it occupies space. The code already
> does that, no?
Ya, that sentence is not required.
>
> > It takes some time for the cpu to clear the interrupt cause flag.
> > Therefore, to prevent another occurrence of interrupt due to this
> > delay, the interrupt cause flag is read after clearing.
>
> You really want to explain explicitely what the problem is. The above is a novel
>
> Something like this:
>
> The irq_eoi() callback of the RX/G2L interrupt chip clears the
> relevant interrupt cause bit in the TSCR register.
>
> This write is not sufficient because the write is posted and therefore
> not guaranteed to immediately clear the bit. Due to that delay the CPU
> can raise the just handled interrupt again.
>
> Prevent this by reading the register back which causes the posted
> write to be flushed to the hardware before the read completes.
>
> This uses the proper technical term 'posted write' which is well defined and exactly the cause of the
> problem, no?
You are correct. The 'Posted write' is new technological term for me.
>
> > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller
> > driver")
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > drivers/irqchip/irq-renesas-rzg2l.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> > b/drivers/irqchip/irq-renesas-rzg2l.c
> > index 9494fc26259c..46f9b07e0e8a 100644
> > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > @@ -111,8 +111,11 @@ static void rzg2l_tint_eoi(struct irq_data *d)
> > u32 reg;
> >
> > reg = readl_relaxed(priv->base + TSCR);
> > - if (reg & bit)
> > + if (reg & bit) {
> > writel_relaxed(reg & ~bit, priv->base + TSCR);
> > + /* Read to avoid irq generation due to irq clearing delay */
>
> /*
> * Enforce that the posted write is flushed to prevent
> * that the just handled interrupt is raised again.
> */
>
> Hmm?
Agreed, Will update accordingly.
Cheers,
Biju
>
> > + readl_relaxed(priv->base + TSCR);
