Re: [PATCH v3 4/6] irqchip/renesas-rzv2h: Make IRQ type handling range-aware

[Lad, Prabhakar]

Hi Thomas,

Thank you for the review.

On Tue, Feb 24, 2026 at 7:30 AM Thomas Gleixner <tglx@xxxxxxxxxx> wrote:
>
> On Mon, Feb 09 2026 at 10:41, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Refine IRQ type handling to explicitly bound IRQ and TINT ranges and
> > dispatch based on the hardware IRQ number.
>
> Changelog should not start with an explanation of the what. See
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog
>
> > This restructures the logic to clearly separate NMI, IRQ, and TINT
>
> Restructure the ...
>
Sure, I will update it.

>
> > @@ -175,18 +177,27 @@ static void rzv2h_icu_eoi(struct irq_data *d)
> >       u32 bit;
> >
> >       scoped_guard(raw_spinlock, &priv->lock) {
> > -             if (hw_irq >= ICU_TINT_START) {
> > -                     tintirq_nr = hw_irq - ICU_TINT_START;
> > -                     bit = BIT(tintirq_nr);
> > -                     if (!irqd_is_level_type(d))
> > -                             writel_relaxed(bit, priv->base + priv->info->t_offs + ICU_TSCLR);
> > -             } else if (hw_irq >= ICU_IRQ_START) {
> > +             switch (hw_irq) {
> > +             case 0:
> > +                     /* Clear NMI */
> > +                     writel_relaxed(ICU_NSCLR_NCLR, priv->base + ICU_NSCLR);
> > +                     break;
> > +             case ICU_IRQ_START ... ICU_IRQ_LAST:
> > +                     /* Clear IRQ */
> >                       tintirq_nr = hw_irq - ICU_IRQ_START;
> >                       bit = BIT(tintirq_nr);
> >                       if (!irqd_is_level_type(d))
> >                               writel_relaxed(bit, priv->base + ICU_ISCLR);
> > -             } else {
> > -                     writel_relaxed(ICU_NSCLR_NCLR, priv->base + ICU_NSCLR);
> > +                     break;
> > +             case ICU_TINT_START ... ICU_TINT_LAST:
> > +                     /* Clear TINT */
> > +                     tintirq_nr = hw_irq - ICU_TINT_START;
> > +                     bit = BIT(tintirq_nr);
> > +                     if (!irqd_is_level_type(d))
> > +                             writel_relaxed(bit, priv->base + priv->info->t_offs + ICU_TSCLR);
> > +                     break;
> > +             default:
> > +                     break;
> >               }
> >       }
>
> TBH, I personally do not care about the performance of your platform at
> all, but are you really serious about having a switch case like that in
> a hotpath function?
>
> Instead of sprinkling this switch case gunk all over the place you can
> simply have separate interrupt chips for each region and install the
> proper one at setup time. Then the functions are clearly separated and
> just handling the type they are written for and nothing else. No?
>
Agreed, I will split them up in separate irq chips, thank you for the
suggestion.

Cheers,
Prabhakar