Re: [PATCH net-next v15 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:
- [High] net: dsa: ksz8995xa: PHY soft reset writes to reserved MAC register on CPU port
--

commit f841f88763071d08ce03b9faac5dca678f1c4f1f
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, 
handling legacy compatibles and implementing specialized DCB and MTU logic. It 
also provides a customized DSA switch operations struct for the older hardware.

> diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
> index 941ae9f66f70b..d7498132064e8 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++) {

[Severity: High]
Does this loop in ksz8_reset_switch() unconditionally include the CPU port?

Since dev->info->port_cnt for KSZ8995XA is 5, it looks like this will also
write PORT_PHY_SOFT_RESET to port 4 (the CPU port). However, port 4 lacks an
internal PHY (dev->info->internal_phy[4] is false) and writing to its reserved
MAC register address space could trigger unintended behavior.

> +			ret = ksz_prmw8(dev, port, REG_PORT_STATUS_3,
> +					PORT_PHY_SOFT_RESET,
> +					PORT_PHY_SOFT_RESET);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		/* 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);

[Severity: High]
Could polling this reserved register lead to a permanent timeout?

If the hardware retains the written value on the reserved register (meaning it
does not self-clear like a real PHY would), regmap_read_poll_timeout() will
timeout after 600ms and return -ETIMEDOUT, causing the entire switch probe to
fail.

Should this loop check dev->info->internal_phy[port] before proceeding, or
alternatively iterate only up to dev->phy_port_cnt to avoid touching the CPU
port?

> +			if (ret)
> +				return ret;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-ks8995-to-ksz8-v15-0-8825ff5fd4be@kernel.org?part=3