[PATCH net v2] net: phy: qca83xx: read resolved QCA8337 link status

[PATCH net v2] net: phy: qca83xx: read resolved QCA8337 link status

[Yongzhao Chen]

After SmartSpeed lowers the speed of a link, the generic status code
still reports the advertised speed, because it derives speed from the
advertised and link partner modes.

This breaks traffic on a QCA8337 user port. With a downstream OpenWrt
6.18.52 kernel, a port connected to an Intel igb NIC over a two-pair
cable downshifted to 100 Mbit/s, but phylib reported 1000 Mbit/s. qca8k
then programmed the MAC for 1000 Mbit/s and no traffic passed. With
at803x_read_status() and genphy_read_master_slave(), as in this change,
the port reported 100 Mbit/s and traffic passed.

Use at803x_read_status() for QCA8337, so speed and duplex come from the
PHY-Specific Status register. The register layout matches what the
helper expects: speed encoding 3 is reserved and bits 9:7 read as zero
(QCA8337 data sheet 80-Y0619-3 Rev. D, page 328), and the MDI crossover
field of the function control register is the same as on AT803X (page
327).

Compared with genphy_read_status(), MDI-X is now reported. A reserved
speed encoding or an unresolved status leaves the speed at SPEED_UNKNOWN
with the link up. The new wrapper also calls genphy_read_master_slave(),
to keep the master/slave status that genphy_read_status() provides.
This refreshes master/slave status on each poll, including while the
link stays up. In forced mode, speed and duplex also come from the
PHY-Specific Status register instead of BMCR.

Fixes: b3591c2a3661 ("net: dsa: qca8k: Switch to PHYLINK instead of PHYLIB")
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Yongzhao Chen <yongzhao.derek@gmail.com>
Assisted-by: LLM
---
v2:
 - Target net as Andrew requested and describe the observed user-port
   failure. The Fixes tag identifies the switch to phylink, after which
   the reported speed is used to program the user-port MAC.
 - Use at803x_read_status() instead of decoding the vendor status here. It
   reads that status only when the link changes, so the code that forced the
   link down for an unresolved status or a reserved speed encoding, and the
   code that cleared the state again when there is no link, is gone. That
   also answers Andrew's two questions on v1: the helper needs no special
   handling of those cases, and page 328 of the data sheet lists speed
   encoding 3 as reserved.
 - Keep the master/slave status of genphy_read_status() with a small wrapper
   around the helper.
 - Cite the data sheet pages in the commit message.
 - v1 could change speed and duplex while the link stayed up, without
   PHYLIB telling its consumers. The helper does not.

v1: https://lore.kernel.org/netdev/20260928220749.857-1-yongzhao.derek@gmail.com/

Testing: a model test compiles the real genphy_update_link(),
at803x_read_status(), at803x_read_specific_status(),
genphy_read_master_slave(), phy_resolve_aneg_pause() and
phy_check_link_status() against an MDIO stub. It covers speed and duplex on
link-up, no rereads of the vendor status while the link stays up, a BMSR
latch-low renegotiation, unresolved and reserved speed encodings, MDIO
errors, pause, forced mode and master/slave status. An arm64 W=1 build of
qca83xx.o, at803x.o and qcom-phy-lib.o is clean.

The hardware comparison in the commit message used one board, QCA8337
lan2 and default autonegotiation; the downshift resolved to 100 Mbit/s
full duplex. Ping was 0/20 in both directions with genphy_read_status()
and 20/20 each way with the new path. Each path ran two warm boots and
three port down/up cycles. I have not tested a cold boot, other boards,
forced mode, or an unresolved status on hardware. The latter two cases
are only modelled; whether QCA8337 sets the resolved bit on a forced
link has not been verified.

 drivers/net/phy/qcom/qca83xx.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/phy/qcom/qca83xx.c b/drivers/net/phy/qcom/qca83xx.c
index bc70ed8efd8..b8da6a41788 100644
--- a/drivers/net/phy/qcom/qca83xx.c
+++ b/drivers/net/phy/qcom/qca83xx.c
@@ -92,6 +92,17 @@ static int qca83xx_probe(struct phy_device *phydev)
 	return 0;
 }
 
+static int qca8337_read_status(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = at803x_read_status(phydev);
+	if (ret)
+		return ret;
+
+	return genphy_read_master_slave(phydev);
+}
+
 static int qca83xx_config_init(struct phy_device *phydev)
 {
 	u8 switch_revision;
@@ -220,6 +231,7 @@ static struct phy_driver qca83xx_driver[] = {
 	.flags			= PHY_IS_INTERNAL,
 	.config_init		= qca83xx_config_init,
 	.soft_reset		= genphy_soft_reset,
+	.read_status		= qca8337_read_status,
 	.get_sset_count		= qca83xx_get_sset_count,
 	.get_strings		= qca83xx_get_strings,
 	.get_stats		= qca83xx_get_stats,

base-commit: 6dc989ea46b96ce170840174b4a38c4a387fb005
-- 
2.43.0