Re: [PATCH net-next v13 3/4] net: dsa: microchip: Support Microchip KSZ8995XA / KS8995XA
From: sashiko-bot@kernel.org
Date: 2026-09-14 19:57:26
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The PHY soft reset loop incorrectly iterates over all ports including the CPU port, writing to and polling a reserved register which can lead to a 600ms timeout and probe failure. -- commit f211090124449a019250a45d2be289b7aa2d1e62 Author: Linus Walleij [off-list ref] net: dsa: microchip: Support Microchip KSZ8995XA / KS8995XA This commit adds support for the Microchip KSZ8995XA switch to the KSZ driver, along with handling its specific PHY reset requirements, DSCP mapping, and legal packet size semantics. It also transitions the legacy KS8995 driver exclusives.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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);
[Severity: High] Does this code inadvertently write to a reserved register for the CPU port? The PHY soft reset loop iterates over all ports up to dev->info->port_cnt, which includes the CPU port (port 4). However, the CPU port lacks an internal PHY (.internal_phy = false). Initiating a PHY reset here means writing to REG_PORT_STATUS_3, which maps to a reserved register (offset 0x5D) for the CPU port.
+ 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] Can this polling loop cause a 600ms probe timeout? Since this loop also polls the reserved register on the CPU port, if the written bit does not self-clear as it would on actual PHYs, this could block for the entire 600ms duration and cause the initialization to fail. Should these loops iterate up to dev->phy_port_cnt or explicitly check .internal_phy[port] before interacting with the PHY reset register?
+ if (ret) + return ret; + } + + /* Some PHYs need additional settling time after reset. */ + usleep_range(1000, 2000);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260913-ks8995-to-ksz8-v13-0-327b16d309eb@kernel.org?part=3