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

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

[Yongzhao Chen]

After a QCA8337 SmartSpeed downshift, genphy_read_status() can report
1000 Mbit/s from the advertised modes when the PHY is running at
100 Mbit/s. On a Redmi AX5400 running downstream OpenWrt Linux 6.18.52,
this caused qca8k to program the user-port MAC for 1000 Mbit/s and
traffic failed. An earlier prototype that read the resolved speed
from the PHY restored traffic.

Wrap genphy_read_status() and use at803x_read_specific_status() to
replace its speed and duplex result when an autonegotiated link comes
up. Resolve pause for the resulting duplex. Until the PHY reports a
resolved speed and duplex, report the link as down so that the next
poll reads them again, as marvell_read_status_page_an() does. In that
case also clear the duplex, master/slave state and link partner modes
that genphy_read_status() read for an up link. QCA8337's register
layout matches the helper (data sheet 80-Y0619-3 Rev. D, pages
327-328).

Keep the generic master/slave read order and the early return for a
steady autonegotiated link. This avoids additional reads of the
clear-on-read idle error count in MII_STAT1000. Forced mode continues
to use BMCR. Report the MDI-X state read at link-up and invalidate it
when the link goes down or autonegotiation is disabled. Clear
mdix_ctrl because this driver does not implement MDI-X configuration.

The QCA8327 entries keep genphy_read_status(). The AR8327 data sheet
(MKG-15791 Rev. 1.0) lists the QCA8327-A PHY ID and documents the same
PHY-specific status layout, with SmartSpeed enabled after a hardware
reset, so those PHYs may have the same problem. That is untested on
QCA8327 hardware, so limit this change to QCA8337.

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
---
v5:
 - When the link is reported down at link-up, also clear duplex,
   master_slave_state and lp_advertising. v4 left the values that
   genphy_read_status() had read for an up link, and a reserved speed
   value left a concrete duplex.
 - State in the commit message why the QCA8327 entries are unchanged.
 - Rebased onto net 513444cc62e2. The files involved are unchanged.
 Both points were raised in the Sashiko review of v4.

v4: https://lore.kernel.org/netdev/20261008193515.14880-1-yongzhao.derek@gmail.com/
v3: https://lore.kernel.org/netdev/20261006160316.1356-1-yongzhao.derek@gmail.com/
v2: https://lore.kernel.org/netdev/20261003173210.1235-1-yongzhao.derek@gmail.com/
v1: https://lore.kernel.org/netdev/20260928220749.857-1-yongzhao.derek@gmail.com/

Testing: the source-extraction model passes 28 checks covering speed
and duplex at the phylib notification boundary, latch-low transitions,
pause, forced mode, MDIO errors, the idle-error and MSFAIL read order,
the unresolved and reserved status cases, and mdix. v4 fails only the
new check for the state left after an unresolved or reserved status.
An arm64 W=1 build of qca83xx.o, at803x.o and qcom-phy-lib.o is clean.

v5 has not been tested on hardware. The hardware result above used
v2's at803x_read_status() plus genphy_read_master_slave() on one board,
with an Intel igb peer, a two-pair cable and default autonegotiation.
Ping changed from 0/20 to 20/20 in each direction at resolved 100 Full.
Whether QCA8337 can report link up before the resolved bit is set is
unknown.
---
 drivers/net/phy/qcom/qca83xx.c | 58 ++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/net/phy/qcom/qca83xx.c b/drivers/net/phy/qcom/qca83xx.c
index bc70ed8efd8..2c4d085bacd 100644
--- a/drivers/net/phy/qcom/qca83xx.c
+++ b/drivers/net/phy/qcom/qca83xx.c
@@ -92,6 +92,63 @@ static int qca83xx_probe(struct phy_device *phydev)
 	return 0;
 }
 
+static int qca8337_read_status(struct phy_device *phydev)
+{
+	struct at803x_ss_mask ss_mask = {
+		.speed_mask = AT803X_SS_SPEED_MASK,
+		.speed_shift = __bf_shf(AT803X_SS_SPEED_MASK),
+	};
+	int ret, old_link = phydev->link;
+
+	ret = genphy_read_status(phydev);
+	if (ret)
+		return ret;
+
+	/* Keep the state resolved at link-up while the link stays up */
+	if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link)
+		goto out;
+
+	phydev->mdix = ETH_TP_MDI_INVALID;
+
+	/* genphy_read_status() derives speed and duplex from the
+	 * advertisements, which is wrong after a SmartSpeed downshift.
+	 * Use what the PHY resolved instead.
+	 */
+	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete &&
+	    phydev->link) {
+		phydev->speed = SPEED_UNKNOWN;
+		phydev->duplex = DUPLEX_UNKNOWN;
+		phydev->pause = false;
+		phydev->asym_pause = false;
+
+		ret = at803x_read_specific_status(phydev, ss_mask);
+		if (ret)
+			return ret;
+
+		/* Report the link down until speed and duplex are resolved,
+		 * so that the next poll reads them again. Also clear what
+		 * genphy_read_status() read for an up link, as the
+		 * PHY_HALTED path of the state machine does.
+		 */
+		if (phydev->speed == SPEED_UNKNOWN ||
+		    phydev->duplex == DUPLEX_UNKNOWN) {
+			phydev->link = 0;
+			phydev->duplex = DUPLEX_UNKNOWN;
+			phydev->mdix = ETH_TP_MDI_INVALID;
+			phydev->master_slave_state = MASTER_SLAVE_STATE_UNKNOWN;
+			linkmode_zero(phydev->lp_advertising);
+		} else {
+			phy_resolve_aneg_pause(phydev);
+		}
+	}
+
+out:
+	/* MDI-X configuration is not implemented */
+	phydev->mdix_ctrl = ETH_TP_MDI_INVALID;
+
+	return 0;
+}
+
 static int qca83xx_config_init(struct phy_device *phydev)
 {
 	u8 switch_revision;
@@ -220,6 +277,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: 513444cc62e231a11c9b15ea88de0890da9b2b4f
-- 
2.43.0