From d7edc2c4a1cfd49a2fb3bcacb3463b2b01cd12f9 Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:21:11 +0100 Subject: [PATCH 1/5] RollBall SFP modules gate PHY register access for up to 25 seconds after insertion, returning 0xffff for all reads. get_phy_c45_ids() detects this (devices-in-package reads as all-ones) and returns -ENODEV, which the SFP state machine handles with its 25 x 1s retry loop (R_PHY_RETRY / phy_t_retry set by sfp_fixup_rollball()). However, some of these modules (observed with an "OEM SFP-10G-T" containing a Broadcom BCM84891) exit the gated window slightly before the PHY's identifier registers are stable: devices-in-package reads a plausible value while MII_PHYSID1/2 in all MMDs still read zero. get_phy_c45_ids() has no validity check for this case, so a phydev is created with all-zero c45 device_ids. No PHY driver can match a zero ID (phy_bus_match() only skips 0xffffffff), so phy->drv is NULL, phylink_sfp_connect_phy() rejects the PHY, and sfp_add_phy() fails with -EINVAL: mtk_soc_eth 15100000.ethernet lsfp: PHY i2c:sfp2:11 (id 0x00000000) has no driver loaded sfp sfp2: sfp_add_phy failed: -EINVAL Unlike -ENODEV, this error path sends the state machine to SFP_S_FAIL, which is terminal until the module is physically re-seated. Detect the all-zero identifier case in sfp_sm_probe_phy() and return -ENODEV so the existing retry logic applies. Also log the c45 identifiers when sfp_add_phy() fails, so a genuinely unsupported PHY model is diagnosable from dmesg. --- drivers/net/phy/sfp.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index e1358a015442..15c06ec950ba 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -1938,6 +1938,36 @@ static int sfp_sm_probe_phy(struct sfp *sfp, int addr, bool is_c45) return PTR_ERR(phy); } + /* Some RollBall modules stop gating PHY register access (reads + * returning 0xffff) slightly before the PHY's identifier registers + * are stable. A probe landing in this window reads a plausible + * devices-in-package value but all-zero device identifiers, which + * no PHY driver can match. sfp_add_phy() then fails hard with + * -EINVAL and the state machine enters SFP_S_FAIL, requiring a + * physical re-plug. Treat all-zero identifiers as "PHY not ready" + * instead, so the existing R_PHY_RETRY/phy_t_retry logic applies. + */ + if (is_c45) { + bool ids_zero = true; + int i; + + for (i = 1; i < ARRAY_SIZE(phy->c45_ids.device_ids); i++) { + if (phy->c45_ids.device_ids[i] && + phy->c45_ids.device_ids[i] != 0xffffffff) { + ids_zero = false; + break; + } + } + + if (ids_zero) { + dev_info(sfp->dev, + "PHY identifiers read as zero (devs_in_pkg 0x%08x), retrying\n", + phy->c45_ids.devices_in_package); + phy_device_free(phy); + return -ENODEV; + } + } + /* Mark this PHY as being on a SFP module */ phy->is_on_sfp_module = true; @@ -1951,6 +1981,13 @@ static int sfp_sm_probe_phy(struct sfp *sfp, int addr, bool is_c45) err = sfp_add_phy(sfp->sfp_bus, phy); if (err) { + if (is_c45) + dev_err(sfp->dev, + "PHY c45 ids: PMA 0x%08x PCS 0x%08x AN 0x%08x devs_in_pkg 0x%08x\n", + phy->c45_ids.device_ids[MDIO_MMD_PMAPMD], + phy->c45_ids.device_ids[MDIO_MMD_PCS], + phy->c45_ids.device_ids[MDIO_MMD_AN], + phy->c45_ids.devices_in_package); phy_device_remove(phy); phy_device_free(phy); dev_err(sfp->dev, "sfp_add_phy failed: %pe\n", ERR_PTR(err)); From 25ef5348dda78203778522e8e08263b0afda4f56 Mon Sep 17 00:00:00 2001 From: meehien Date: Wed, 8 Jul 2026 18:39:19 +0100 Subject: [PATCH 2/5] The zero-identifier retry ("retry PHY probe when c45 identifiers read as zero") and its 0xffff-half tightening accept a phydev as soon as one MMD carries a plausible identifier. That is not sufficient: a RollBall module can leave the gated window with a single MMD answering while the rest of the mailbox still returns 0xffff, including the devices-in-package map that selects which MMDs are enumerated in the first place. Observed on an OEM SFP-10G-T (Broadcom BCM84891): PMA answered cleanly (0x35905081) while PCS (0xffffffff), AN (0x0362ffff) and devices-in- package (0xffff009b) were still gated. The id-only check passed on the clean PMA identifier, the BCM driver matched on it, and the probe went on to attach. phy_init_hw()'s reset poll during attach then read the still -gated mailbox, saw the reset bit permanently set, and returned -ETIMEDOUT from sfp_add_phy(). Worse, a concurrent interface-down racing that failed attach could fault the PHY state machine. Require, in addition to at least one plausible identifier, that neither 16-bit half of devices_in_package reads 0xffff (all MMDs 0-15 or 16-31 "present" is not a real device map, only a timed-out read). If either is corrupt, treat the PHY as not ready and return -ENODEV so the existing R_PHY_RETRY/phy_t_retry logic re-probes once the module is fully awake. Depends on: "net: phy: sfp: retry PHY probe when c45 identifiers read as zero" --- drivers/net/phy/sfp.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 15c06ec950ba..4a0107f3aafc 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -1948,20 +1948,39 @@ static int sfp_sm_probe_phy(struct sfp *sfp, int addr, bool is_c45) * instead, so the existing R_PHY_RETRY/phy_t_retry logic applies. */ if (is_c45) { - bool ids_zero = true; + u32 devs = phy->c45_ids.devices_in_package; + bool ids_valid = false; int i; + /* Absent MMDs leave their identifier zero; a gated or + * timed-out RollBall access reads 0xffff, so an identifier + * with either 16-bit half at 0xffff is corrupt. Require at + * least one fully plausible identifier before accepting the + * PHY. + */ for (i = 1; i < ARRAY_SIZE(phy->c45_ids.device_ids); i++) { - if (phy->c45_ids.device_ids[i] && - phy->c45_ids.device_ids[i] != 0xffffffff) { - ids_zero = false; + u32 id = phy->c45_ids.device_ids[i]; + + if (id && (id & 0xffff) != 0xffff && + (id >> 16) != 0xffff) { + ids_valid = true; break; } } - if (ids_zero) { + /* devices_in_package selects which MMDs are enumerated, so a + * timed-out read leaving either 16-bit half at 0xffff makes + * the map itself untrustworthy -- one clean identifier is not + * enough to accept it. Seen on a BCM84891 that answered PMA + * (0x35905081) while the rest of the mailbox was still gated + * (devs_in_pkg 0xffff009b): the id-only check passed on the + * clean PMA, attached a half-awake PHY, and its reset poll + * then failed -ETIMEDOUT during attach. + */ + if (!ids_valid || + (devs & 0xffff) == 0xffff || (devs >> 16) == 0xffff) { dev_info(sfp->dev, - "PHY identifiers read as zero (devs_in_pkg 0x%08x), retrying\n", + "PHY identifiers unstable (devs_in_pkg 0x%08x), retrying\n", phy->c45_ids.devices_in_package); phy_device_free(phy); return -ENODEV; From ab01d3d5cd835ee2ce7c66c1e7a380dd5032e8c7 Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:24:13 +0100 Subject: [PATCH 3/5] bcm8489x_config_init() rejects any attachment other than USXGMII with -ENODEV, reflecting the board-soldered configuration it was written for. The BCM84891 is however also the PHY inside various RollBall "OEM SFP-10G-T" copper SFP+ modules, where phylink selects 10GBASE-R as the host interface (with SGMII/2500BASE-X rate switching), exactly as for the BCM84881. In that configuration a successful PHY probe still fails at attach time: sfp sfp2: PHY c45 ids: PMA 0x35905081 PCS ... devs_in_pkg ... sfp sfp2: sfp_add_phy failed: -ENODEV with the -ENODEV originating from phy_init_hw() -> bcm8489x_config_init(). Accept the same interface trio as the BCM84881 in addition to USXGMII, and populate possible_interfaces with all four so phylink can rate-switch on copper linkdowns. The LPOWER clearing is kept unconditional; it is harmless on module-hosted PHYs. --- drivers/net/phy/bcm84881.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c index 2ae70dcf82ec..6c62c5dc01f7 100644 --- a/drivers/net/phy/bcm84881.c +++ b/drivers/net/phy/bcm84881.c @@ -83,10 +83,24 @@ static int bcm84881_config_init(struct phy_device *phydev) static int bcm8489x_config_init(struct phy_device *phydev) { + bcm84881_fill_possible_interfaces(phydev); __set_bit(PHY_INTERFACE_MODE_USXGMII, phydev->possible_interfaces); - if (phydev->interface != PHY_INTERFACE_MODE_USXGMII) + /* The BCM84891 is found both soldered down and attached over + * USXGMII, and inside SFP+ copper modules (e.g. various + * "OEM SFP-10G-T" RollBall modules), where the host-side + * interface is 10GBASE-R with SGMII/2500BASE-X rate switching, + * as with the BCM84881. Accept both attachments. + */ + switch (phydev->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_2500BASEX: + case PHY_INTERFACE_MODE_10GBASER: + case PHY_INTERFACE_MODE_USXGMII: + break; + default: return -ENODEV; + } /* MDIO_CTRL1_LPOWER is set at boot on the tested platform. Does not * recur on ifdown/ifup, cable events, or link-partner advertisement From b7999edbf19a41acab67c79d0db9bc80b69d215a Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:26:16 +0100 Subject: [PATCH 4/5] The bcm84881 driver declares .inband_caps, unconditionally returning LINK_INBAND_DISABLE, but does not implement .config_inband. When a BCM84881-family PHY is used with a phylink MAC in inband mode (e.g. any SFP module PHY, or managed = "in-band-status" in DT), phylink's negotiation resolves to outband operation and sets phy_ib_mode = LINK_INBAND_DISABLE, then calls phy_config_inband() to apply it. With no .config_inband method this returns -EOPNOTSUPP, which phylink_major_config() treats as a fatal configuration error: mtk_soc_eth 15100000.ethernet lsfp: phy_config_inband: -EOPNOTSUPP major_config_failed then forces the link down in phylink_resolve() and holds it down across reconfiguration attempts. The result is a permanently dead link with a perfectly negotiated copper side (observed with a BCM84891-based "OEM SFP-10G-T" module on mt7988: copper AN completes at 10G, carrier never comes up). Implement config_inband: the PHY does not generate inband signalling in any mode, so it is permanently in the "disabled" state and a request to disable inband succeeds as a no-op. Any other request cannot be satisfied and returns -EINVAL (phylink will not request one, given the capabilities this driver reports). --- drivers/net/phy/bcm84881.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c index 6c62c5dc01f7..391f32d25a0b 100644 --- a/drivers/net/phy/bcm84881.c +++ b/drivers/net/phy/bcm84881.c @@ -430,12 +430,30 @@ static unsigned int bcm84881_inband_caps(struct phy_device *phydev, return LINK_INBAND_DISABLE; } +static int bcm84881_config_inband(struct phy_device *phydev, + unsigned int modes) +{ + /* This PHY does not generate inband signalling in any mode (see + * bcm84881_inband_caps()); inband is permanently disabled in + * hardware. A request to disable inband therefore requires no + * action, but must succeed: phylink treats any error from + * phy_config_inband() (including -EOPNOTSUPP from a missing + * config_inband method) as a major configuration failure which + * forces and holds the link down. + */ + if (modes == LINK_INBAND_DISABLE) + return 0; + + return -EINVAL; +} + static struct phy_driver bcm84881_drivers[] = { { .phy_id = 0xae025150, .phy_id_mask = 0xfffffff0, .name = "Broadcom BCM84881", .inband_caps = bcm84881_inband_caps, + .config_inband = bcm84881_config_inband, .config_init = bcm84881_config_init, .probe = bcm84881_probe, .get_features = bcm84881_get_features, @@ -446,6 +464,7 @@ static struct phy_driver bcm84881_drivers[] = { PHY_ID_MATCH_MODEL(0x35905080), .name = "Broadcom BCM84891", .inband_caps = bcm84881_inband_caps, + .config_inband = bcm84881_config_inband, .config_init = bcm8489x_config_init, .probe = bcm84881_probe, .get_features = bcm84881_get_features, @@ -460,6 +479,7 @@ static struct phy_driver bcm84881_drivers[] = { PHY_ID_MATCH_MODEL(0x359050a0), .name = "Broadcom BCM84892", .inband_caps = bcm84881_inband_caps, + .config_inband = bcm84881_config_inband, .config_init = bcm8489x_config_init, .probe = bcm84881_probe, .get_features = bcm84881_get_features, From 2406221a9e06d563d7a7dba094afe4ba8187d5f7 Mon Sep 17 00:00:00 2001 From: meehien Date: Wed, 8 Jul 2026 18:41:39 +0100 Subject: [PATCH 5/5] A RollBall module can leave the gated window with valid identifiers while its PMA/PMD control register is still deaf. The identifier checks added earlier ("retry PHY probe when c45 identifiers read as zero" and its 0xffff-half / devices-in-package tightening) therefore still let such a probe through to sfp_add_phy(). phy_attach_direct() -> phy_init_hw() then runs a soft reset: it writes MDIO_CTRL1 and polls the reset bit to self-clear. On a still-gated read the register returns 0xffff, the reset bit never appears to clear, and sfp_add_phy() fails with -ETIMEDOUT. Observed on an OEM SFP-10G-T (BCM84891) that answered its identifiers (PMA 0x35905081, clean devices-in-package 0xc000009b) yet still timed out in attach. The failed attach has also been observed to fault the PHY state machine when the interface is subsequently brought down. Read MDIO_CTRL1 in PMAPMD before attaching and treat 0xffff, a read error, or a set reset bit as "not ready", returning -ENODEV so the existing R_PHY_RETRY/phy_t_retry loop re-probes. This gates on the exact register the attach-time reset polls, so the PHY is attached once, after the module is genuinely awake, instead of faulting a half-awake reset. This narrows the -ETIMEDOUT window rather than closing it (the mailbox can still stall between this read and the reset a few ms later); the underlying attach-failure teardown path remains worth hardening separately. Depends on: "net: phy: sfp: retry PHY probe when c45 identifiers read as zero" --- drivers/net/phy/sfp.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 4a0107f3aafc..93bb411240b8 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -1950,6 +1950,7 @@ static int sfp_sm_probe_phy(struct sfp *sfp, int addr, bool is_c45) if (is_c45) { u32 devs = phy->c45_ids.devices_in_package; bool ids_valid = false; + int ret; int i; /* Absent MMDs leave their identifier zero; a gated or @@ -1985,6 +1986,25 @@ static int sfp_sm_probe_phy(struct sfp *sfp, int addr, bool is_c45) phy_device_free(phy); return -ENODEV; } + + /* Plausible identifiers do not guarantee the PMA/PMD control + * register is out of the gated window. phy_init_hw()'s soft + * reset during attach writes MDIO_CTRL1 and polls the reset + * bit to self-clear; a gated read returns 0xffff so the bit + * never clears and sfp_add_phy() fails -ETIMEDOUT (which has + * been seen to later fault the PHY state machine on interface + * teardown). Require that same register to read back live + * before attaching, so the retry logic waits out the wake + * window and the PHY is attached exactly once, cleanly. + */ + ret = phy_read_mmd(phy, MDIO_MMD_PMAPMD, MDIO_CTRL1); + if (ret < 0 || ret == 0xffff || (ret & MDIO_CTRL1_RESET)) { + dev_info(sfp->dev, + "PHY control register not ready (0x%04x), retrying\n", + ret); + phy_device_free(phy); + return -ENODEV; + } } /* Mark this PHY as being on a SFP module */