RE: [PATCH v3 7/9] irqchip/renesas-rzg2l: Add RZ/G3L support

[Biju Das]

Hi Thomas,

Thanks for the feedback.

> -----Original Message-----
> From: Thomas Gleixner <tglx@xxxxxxxxxx>
> Sent: 06 February 2026 11:59
> Subject: Re: [PATCH v3 7/9] irqchip/renesas-rzg2l: Add RZ/G3L support
> 
> On Fri, Feb 06 2026 at 11:16, Biju wrote:
> >  /**
> >   * struct rzg2l_hw_info - Interrupt Control Unit controller hardware info structure.
> > + * @tssel_lut:		TINT lookup table
> >   * @irq_count:		Number of IRQC interrupts
> >   * @tint_start:		Start of TINT interrupts
> >   * @num_irq:		Total Number of interrupts
> >   */
> >  struct rzg2l_hw_info {
> > -	u8	irq_count;
> > -	u8	tint_start;
> > -	u8	num_irq;
> > +	const u8	*tssel_lut;
> 
> You can spare that churn by indenting this correctly from the beginning.

OK.

> 
> > +	u8		irq_count;
> > +	u8		tint_start;
> > +	u8		num_irq;
> >  };
> 
> > @@ -343,6 +345,9 @@ static u32 rzg2l_disable_tint_and_set_tint_source(struct irq_data *d, struct rzg
> >  	u32 tint = (u32)(uintptr_t)irq_data_get_irq_chip_data(d);
> >  	u32 tien = reg & (TIEN << TSSEL_SHIFT(tssr_offset));
> >
> > +	if (priv->info->tssel_lut)
> > +		tint = priv->info->tssel_lut[tint];
> 
> I'd rather make it very clear in the code:
> 
> 	if (priv->info->tssel_lut)
> 		tint = priv->info->tssel_lut[tint];
>         else
>         	tint = (u32)(uintptr_t)irq_data_get_irq_chip_data(d);
> 
> rather than read first and overwrite somewhere else.

Agreed.

> 
> > +/* Mapping based on port index on Table 4.2-1 and GPIOINT on Table
> > +4.6-7 */ static const u8 rzg3l_tssel_lut[] = {
> > +	83, 84,					/* P20-P21 */
> > +	7, 8, 9, 10, 11, 12, 13,		/* P30-P36 */
> > +	85, 86, 87, 88, 89, 90, 91,		/* P50-P56 */
> > +	92, 93, 94, 95, 96, 97, 98,		/* P60-P66 */
> > +	99, 100, 101, 102, 103, 104, 105, 106,	/* P70-P77 */
> 
> Please format it so it looks like a table:
> 
> 	 83,  84,				/* P20-P21 */
> 	  7,   8,   9,  10,  11,  12,  13,	/* P30-P36 */
> 	 85,  86,  87,  88,  89,  90,  91,	/* P50-P56 */
> 	 92,  93,  94,  95,  96,  97,  98,	/* P60-P66 */
> 	 99, 100, 101, 102, 103, 104, 105, 106,	/* P70-P77 */
> 
> That's makes it easy to read and to identify the number of entries for a particular port. The
> condensed format does not. No?

OK, will fix this in next version.

Cheers,
Biju