| Message ID | 20240212113712.71878-3-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 |
Hi Biju, On Mon, Feb 12, 2024 at 12:37 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Rename rzg2l_tint_eoi->rzg2l_clear_tint_int and simplify the code by > removing redundant priv and hw_irq local variables. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/drivers/irqchip/irq-renesas-rzg2l.c > +++ b/drivers/irqchip/irq-renesas-rzg2l.c > @@ -103,11 +103,10 @@ static void rzg2l_irq_eoi(struct irq_data *d) > writel_relaxed(iscr & ~bit, priv->base + ISCR); > } > > -static void rzg2l_tint_eoi(struct irq_data *d) > +static void rzg2l_clear_tint_int(struct rzg2l_irqc_priv *priv, > + unsigned int hwirq) > { > - unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START; > - struct rzg2l_irqc_priv *priv = irq_data_to_priv(d); > - u32 bit = BIT(hw_irq); > + u32 bit = BIT(hwirq - IRQC_TINT_START); > u32 reg; > > reg = readl_relaxed(priv->base + TSCR); > @@ -127,7 +126,7 @@ static void rzg2l_irqc_eoi(struct irq_data *d) > if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT) > rzg2l_irq_eoi(d); > else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) > - rzg2l_tint_eoi(d); > + rzg2l_clear_tint_int(priv, hw_irq); Perhaps pass the tint number (i.e. "hw_irq - IRQC_TINT_START") instead? > raw_spin_unlock(&priv->lock); > irq_chip_eoi_parent(d); I think it makes sense to make a similar change to rzg2l_irq_eoi(). 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
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Monday, February 12, 2024 4:38 PM > Subject: Re: [PATCH 2/5] irqchip/renesas-rzg2l: Rename rzg2l_tint_eoi() > > Hi Biju, > > On Mon, Feb 12, 2024 at 12:37 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > Rename rzg2l_tint_eoi->rzg2l_clear_tint_int and simplify the code by > > removing redundant priv and hw_irq local variables. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/drivers/irqchip/irq-renesas-rzg2l.c > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c > > @@ -103,11 +103,10 @@ static void rzg2l_irq_eoi(struct irq_data *d) > > writel_relaxed(iscr & ~bit, priv->base + ISCR); } > > > > -static void rzg2l_tint_eoi(struct irq_data *d) > > +static void rzg2l_clear_tint_int(struct rzg2l_irqc_priv *priv, > > + unsigned int hwirq) > > { > > - unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START; > > - struct rzg2l_irqc_priv *priv = irq_data_to_priv(d); > > - u32 bit = BIT(hw_irq); > > + u32 bit = BIT(hwirq - IRQC_TINT_START); > > u32 reg; > > > > reg = readl_relaxed(priv->base + TSCR); @@ -127,7 +126,7 @@ > > static void rzg2l_irqc_eoi(struct irq_data *d) > > if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT) > > rzg2l_irq_eoi(d); > > else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) > > - rzg2l_tint_eoi(d); > > + rzg2l_clear_tint_int(priv, hw_irq); > > Perhaps pass the tint number (i.e. "hw_irq - IRQC_TINT_START") instead? Agreed. > > > raw_spin_unlock(&priv->lock); > > irq_chip_eoi_parent(d); > > I think it makes sense to make a similar change to rzg2l_irq_eoi(). OK will do similar change. Cheers, Biju
On Mon, Feb 12 2024 at 17:38, Geert Uytterhoeven wrote: >> -static void rzg2l_tint_eoi(struct irq_data *d) >> +static void rzg2l_clear_tint_int(struct rzg2l_irqc_priv *priv, >> + unsigned int hwirq) >> { >> - unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START; >> - struct rzg2l_irqc_priv *priv = irq_data_to_priv(d); >> - u32 bit = BIT(hw_irq); >> + u32 bit = BIT(hwirq - IRQC_TINT_START); >> u32 reg; >> >> reg = readl_relaxed(priv->base + TSCR); >> @@ -127,7 +126,7 @@ static void rzg2l_irqc_eoi(struct irq_data *d) >> if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT) >> rzg2l_irq_eoi(d); >> else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) >> - rzg2l_tint_eoi(d); >> + rzg2l_clear_tint_int(priv, hw_irq); > > Perhaps pass the tint number (i.e. "hw_irq - IRQC_TINT_START") > instead? No. You have to do that on all call sites then. There is another coming in the next patch AFAICT. Thanks, tglx
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c index 46f9b07e0e8a..74c8cbb790e9 100644 --- a/drivers/irqchip/irq-renesas-rzg2l.c +++ b/drivers/irqchip/irq-renesas-rzg2l.c @@ -103,11 +103,10 @@ static void rzg2l_irq_eoi(struct irq_data *d) writel_relaxed(iscr & ~bit, priv->base + ISCR); } -static void rzg2l_tint_eoi(struct irq_data *d) +static void rzg2l_clear_tint_int(struct rzg2l_irqc_priv *priv, + unsigned int hwirq) { - unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START; - struct rzg2l_irqc_priv *priv = irq_data_to_priv(d); - u32 bit = BIT(hw_irq); + u32 bit = BIT(hwirq - IRQC_TINT_START); u32 reg; reg = readl_relaxed(priv->base + TSCR); @@ -127,7 +126,7 @@ static void rzg2l_irqc_eoi(struct irq_data *d) if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT) rzg2l_irq_eoi(d); else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) - rzg2l_tint_eoi(d); + rzg2l_clear_tint_int(priv, hw_irq); raw_spin_unlock(&priv->lock); irq_chip_eoi_parent(d); }
Rename rzg2l_tint_eoi->rzg2l_clear_tint_int and simplify the code by removing redundant priv and hw_irq local variables. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/irqchip/irq-renesas-rzg2l.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)