Re: [PATCH net-next v10 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] Hardware initialization failure: polling PHY soft reset on MAC-only CPU port may cause timeout.
--

commit e3efbaeb1bd18a981cfc0bd58667bbdba4d7252e
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 includes a custom reset routine that iterates over the switch
ports to issue a self-clearing PHY soft reset.

> diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
> index 941ae9f66f70b..40d7d0eaf1cee 100644
> --- a/drivers/net/dsa/microchip/ksz8.c
> +++ b/drivers/net/dsa/microchip/ksz8.c
[ ... ]
> @@ -253,7 +255,31 @@ 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++)
> +			ksz_port_cfg(dev, port, REG_PORT_STATUS_3,
> +				     PORT_PHY_SOFT_RESET, true);
> +
> +		/* 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]
Could this polling loop timeout and abort the switch probe when checking the
CPU port?

In ksz8_reset_switch(), the loop iterates up to dev->info->port_cnt, which
includes the MAC-only CPU port. Since the CPU port does not have an internal
PHY, writing the PORT_PHY_SOFT_RESET bit to it might not self-clear depending
on the hardware register implementation.

If the bit acts as a regular R/W bit on the CPU port rather than being
hardwired to 0, regmap_read_poll_timeout() will poll for 600ms and ultimately
return -ETIMEDOUT. Should this loop iterate up to dev->phy_port_cnt instead,
or explicitly skip the CPU port?

> +
> +		/* Some PHYs need additional settling time after reset. */
> +		usleep_range(1000, 2000);
> +	} else if (ksz_is_ksz88x3(dev)) {
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-ks8995-to-ksz8-v10-0-7379239eb65f@kernel.org?part=3