Re: [PATCH net-next v12 3/4] net: dsa: microchip: Support Microchip KSZ8995XA / KS8995XA

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The PHY soft reset loop in `ksz8_reset_switch` unconditionally issues a soft reset and polls for it to self-clear on all ports, including the CPU port which lacks an internal PHY. This risks causing a probe timeout on the KSZ8995XA if the reserved register latches the bit.
--

commit d45e85fe8d15053b235725e1e75c154fee3c35c3
Author: Linus Walleij <linusw@kernel.org>

net: dsa: microchip: Support Microchip KSZ8995XA / KS8995XA

This commit adds support for the Microchip KSZ8995XA switch to the KSZ driver.
It introduces hardware detection, a dedicated PHY soft reset workaround for
this older switch, and configures the switch for its specific legacy features.

> diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
> index 941ae9f66f70b..4acce01ca0631 100644
> --- a/drivers/net/dsa/microchip/ksz8.c
> +++ b/drivers/net/dsa/microchip/ksz8.c
[ ... ]
> @@ -253,7 +255,35 @@ static int ksz8463_reset_switch(struct ksz_device *dev)
>  
>  static int ksz8_reset_switch(struct ksz_device *dev)
>  {
> -	if (ksz_is_ksz88x3(dev)) {
> +	if (ksz_is_ksz8995xa(dev)) {
> +		unsigned int port, reg, val;
> +		int ret;
> +
> +		/* The KSZ8995XA switch itself cannot be reset by software, but
> +		 * often boot loaders have fun with ports, so reset them. This
> +		 * reset bit is self-clearing.
> +		 */
> +		for (port = 0; port < dev->info->port_cnt; port++) {
> +			ret = ksz_prmw8(dev, port, REG_PORT_STATUS_3,
> +					PORT_PHY_SOFT_RESET,
> +					PORT_PHY_SOFT_RESET);
> +			if (ret)
> +				return ret;
> +		}

[Severity: Medium]
Does this loop incorrectly include the MAC-only CPU port? 

Since dev->info->port_cnt is 5 for the KSZ8995XA, this iterates over ports
0 through 4. Port 4 is the CPU port, which does not have an internal PHY.
Does writing PORT_PHY_SOFT_RESET to REG_PORT_STATUS_3 on the CPU port write
to a reserved register?

> +
> +		/* IEEE 802.3 allows up to 500 ms for a PHY reset. */
> +		for (port = 0; port < dev->info->port_cnt; port++) {
> +			reg = dev->dev_ops->get_port_addr(port, REG_PORT_STATUS_3);
> +			ret = regmap_read_poll_timeout(ksz_regmap_8(dev), reg, val,
> +						       !(val & PORT_PHY_SOFT_RESET),
> +						       1000, 600000);
> +			if (ret)
> +				return ret;
> +		}

[Severity: Medium]
Will this polling loop timeout on the CPU port if the hardware latches the
written bit?

Since the CPU port lacks the PHY logic to execute the reset and self-clear
the bit, it might hang indefinitely and fail the probe with -ETIMEDOUT.
Should this loop (and the write loop above) be restricted to
dev->phy_port_cnt instead of dev->info->port_cnt?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-ks8995-to-ksz8-v12-0-5c3422d0d72e@kernel.org?part=3