Re: [PATCH net-next v6 1/9] net: dsa: microchip: Add support for KSZ8463 global irq

[Vladimir Oltean <olteanv@gmail.com>]

On Wed, Mar 04, 2026 at 11:18:52AM +0100, Bastien Curutchet (Schneider Electric) wrote:
> @@ -2890,14 +2899,18 @@ static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id)
>  	unsigned int nhandled = 0;
>  	struct ksz_device *dev;
>  	unsigned int sub_irq;
> -	u8 data;
> +	u16 data;
>  	int ret;
>  	u8 n;
>  
>  	dev = kirq->dev;
>  
> -	/* Read interrupt status register */
> -	ret = ksz_read8(dev, kirq->reg_status, &data);
> +	/*
> +	 * Most of the KSZ switches have a 8-bits long interrupt status
> +	 * register, but the KSZ8463 has a 16-bits long one. The overread here
> +	 * is safe because we only iterate over kirq->nirqs in the below loop.

FWIW, this isn't the only thing making an overread "safe".
If the adjacent register also has "clear on read" semantics, that's not
good.

I can't tell whether that's the case here, though. There are just too
many hardware variations to check for. I just wanted to point out that
the reasoning is incomplete.

> +	 */
> +	ret = ksz_read16(dev, kirq->reg_status, &data);

I guess I don't fully understand the KSZ_REGMAP_TABLE() layer, but I
have a concern here.

Take the normal ksz_girq_setup() case for example, where we set
kirq->reg_status = REG_SW_PORT_INT_STATUS__1.

With ksz_read8(), we read u8 data from address REG_SW_PORT_INT_STATUS__1.
With ksz_read16(), how do we read u16 data? Don't we read
data[15:8] from REG_SW_PORT_INT_STATUS__1 and
data[7:0] from REG_SW_PORT_INT_STATUS__1 + 1?
But then we still look at the 0..kirq->nirq bits, effectively from address
REG_SW_PORT_INT_STATUS__1 + 1 now?

Or are the registers at address i 16-bits wide without spilling over
into address i+1? Because if that's the case, I don't understand the
comment/concern about overreading.

>  	if (ret)
>  		goto out;