RE: [PATCH v5 15/15] irqchip/renesas-rzg2l: Add shared interrupt support

[Biju Das]

Hi Thomas,

Thanks for the feedback.

> -----Original Message-----
> From: Thomas Gleixner <tglx@xxxxxxxxxx>
> Sent: 20 March 2026 09:01
> Subject: Re: [PATCH v5 15/15] irqchip/renesas-rzg2l: Add shared interrupt support
> 
> On Wed, Mar 11 2026 at 19:24, Biju wrote:
> > +static int rzg2l_irqc_irq_request_resources(struct irq_data *d) {
> > +	unsigned int hw_irq = irqd_to_hwirq(d);
> > +	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +	u32 offset, tssr_offset;
> > +	u8 tssr_index, tssel_shift;
> > +	u32 reg, inttsel_reg;
> > +	u8 value;
> 
> Once again: Proper variable declaration ordering please. Do I have to repeat that every other week?
> 
> Again the same type salad.

Sorry, Will fix it in next version.

> 
> > +	if (!priv->info.shared_irq_cnt)
> > +		return 0;
> > +
> > +	if (rzg2l_irqc_is_shared_irqc(priv->info, hw_irq)) {
> > +		offset = hw_irq + IRQC_TINT_COUNT - priv->info.tint_start;
> > +		tssr_offset = TSSR_OFFSET(offset);
> > +		tssr_index = TSSR_INDEX(offset);
> > +		tssel_shift = TSSEL_SHIFT(tssr_offset);
> > +
> > +		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > +		value = (reg & (TIEN << tssel_shift)) >> tssel_shift;
> > +		if (value)
> > +			goto err_conflict;
> > +
> > +		raw_spin_lock(&priv->lock);
> 
> scoped_guard()

Agreed.

> 
> > +		inttsel_reg = readl_relaxed(priv->base + INTTSEL);
> > +		inttsel_reg |= TINTSEL(offset);
> > +		writel_relaxed(inttsel_reg, priv->base + INTTSEL);
> > +		raw_spin_unlock(&priv->lock);
> > +	} else if (rzg2l_irqc_is_shared_tint(priv->info, hw_irq)) {
> > +		offset = hw_irq - priv->info.tint_start;
> > +		tssr_offset = TSSR_OFFSET(offset);
> > +		tssr_index = TSSR_INDEX(offset);
> > +
> > +		inttsel_reg = readl_relaxed(priv->base + INTTSEL);
> > +		value = (inttsel_reg & TINTSEL(offset)) >> offset;
> > +		if (value)
> > +			goto err_conflict;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_conflict:
> > +	pr_err("%s: Shared SPI conflict!\n", __func__);
> > +	return -EBUSY;
> > +}
> > +
> > +static void rzg2l_irqc_irq_release_resources(struct irq_data *d) {
> > +	unsigned int hw_irq = irqd_to_hwirq(d);
> > +	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +	u32 offset;
> > +	u8 inttsel_reg;
> 
> Your type choices are really interresting and both variables are not used in the outer scope. Declare
> them in the scope where they are used.

OK.
> 
> > +	if (!priv->info.shared_irq_cnt)
> > +		return;
> > +
> > +	if (rzg2l_irqc_is_shared_irqc(priv->info, hw_irq)) {
> > +		offset = hw_irq + IRQC_TINT_COUNT - priv->info.tint_start;
> > +
> > +		raw_spin_lock(&priv->lock);
> > +		inttsel_reg = readl_relaxed(priv->base + INTTSEL);
>                 ^^^^          ^^^
>                 u8            u32
> 
> Seriously?

Oops, will fix this.

Cheers,
Biju