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

[Thomas Gleixner]

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.

> +	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()

> +		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.

> +	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?

> +		inttsel_reg &= ~TINTSEL(offset);
> +		writel_relaxed(inttsel_reg, priv->base + INTTSEL);
> +		raw_spin_unlock(&priv->lock);

Thanks,

        tglx