From d7edc2c4a1cfd49a2fb3bcacb3463b2b01cd12f9 Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:21:11 +0100 Subject: [PATCH 1/6] 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 154369f60f59be63eb795ea9c3c6fc4d6d26210a Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:22:23 +0100 Subject: [PATCH 2/6] The RollBall MDIO-over-I2C protocol issues a command and polls the command register for ROLLBALL_CMD_DONE, with a budget of 10 x 20 ms. When the poll times out, i2c_mii_read_rollball() returns 0xffff -- indistinguishable from register data. Some modules (observed: "OEM SFP-10G-T" with a Broadcom BCM84891) intermittently take longer than the ~200 ms budget to complete a command, well past the initial 25 s access-denial window. Each timed-out read then injects a spurious 0xffff word into whatever the caller is reading. During PHY probe this corrupts the C45 identifiers (e.g. devs_in_pkg 0xc000ffff, PMA ID 0xffff5081 where the true values are 0xc000009b and 0x35905081), so no PHY driver matches and sfp_add_phy() fails with -EINVAL, leaving the SFP state machine in SFP_S_FAIL until the module is re-seated. The same mechanism can corrupt reads and silently drop writes at runtime after a successful probe. Re-issue the command up to 3 times when the completion poll times out, in both the read and write paths. Re-issuing a read of the same devad/reg is idempotent, so a late completion of a previous attempt still yields data for the correct register. The 0xffff-on-timeout semantic is preserved once retries are exhausted, keeping the existing behaviour for the initial access-denial window (where get_phy_c45_ids() maps all-ones to -ENODEV and the SFP state machine retries the whole probe). --- drivers/net/mdio/mdio-i2c.c | 45 +++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/drivers/net/mdio/mdio-i2c.c b/drivers/net/mdio/mdio-i2c.c index ed20352a589a..3ea150b9a814 100644 --- a/drivers/net/mdio/mdio-i2c.c +++ b/drivers/net/mdio/mdio-i2c.c @@ -195,6 +195,11 @@ static int smbus_byte_mii_write_default_c22(struct mii_bus *bus, int phy_id, #define ROLLBALL_CMD_READ 0x02 #define ROLLBALL_CMD_DONE 0x04 +/* Number of times a command is re-issued if the module does not + * signal ROLLBALL_CMD_DONE within the polling budget. + */ +#define ROLLBALL_CMD_RETRIES 3 + #define SFP_PAGE_ROLLBALL_MDIO 3 static int __i2c_transfer_err(struct i2c_adapter *i2c, struct i2c_msg *msgs, @@ -362,7 +367,7 @@ static int i2c_mii_read_rollball(struct mii_bus *bus, int phy_id, int devad, int reg) { u8 buf[4], res[6]; - int bus_addr, ret; + int bus_addr, ret, i; u16 val; bus_addr = i2c_mii_phy_addr(phy_id); @@ -374,12 +379,23 @@ static int i2c_mii_read_rollball(struct mii_bus *bus, int phy_id, int devad, buf[2] = (reg >> 8) & 0xff; buf[3] = reg & 0xff; - ret = i2c_rollball_mii_cmd(bus, bus_addr, ROLLBALL_CMD_READ, buf, - sizeof(buf)); - if (ret < 0) - return ret; + /* Some modules (e.g. OEM SFP-10G-T with a BCM84891) occasionally + * take longer than the polling budget to execute a command. Since + * returning 0xffff for a timed-out read is indistinguishable from + * register data, retry the command a few times first; re-issuing + * the same read is idempotent. + */ + for (i = 0; i < ROLLBALL_CMD_RETRIES; i++) { + ret = i2c_rollball_mii_cmd(bus, bus_addr, ROLLBALL_CMD_READ, + buf, sizeof(buf)); + if (ret < 0) + return ret; + + ret = i2c_rollball_mii_poll(bus, bus_addr, res, sizeof(res)); + if (ret != -ETIMEDOUT) + break; + } - ret = i2c_rollball_mii_poll(bus, bus_addr, res, sizeof(res)); if (ret == -ETIMEDOUT) return 0xffff; else if (ret < 0) @@ -393,7 +409,7 @@ static int i2c_mii_read_rollball(struct mii_bus *bus, int phy_id, int devad, static int i2c_mii_write_rollball(struct mii_bus *bus, int phy_id, int devad, int reg, u16 val) { - int bus_addr, ret; + int bus_addr, ret, i; u8 buf[6]; bus_addr = i2c_mii_phy_addr(phy_id); @@ -407,12 +423,17 @@ static int i2c_mii_write_rollball(struct mii_bus *bus, int phy_id, int devad, buf[4] = val >> 8; buf[5] = val & 0xff; - ret = i2c_rollball_mii_cmd(bus, bus_addr, ROLLBALL_CMD_WRITE, buf, - sizeof(buf)); - if (ret < 0) - return ret; + for (i = 0; i < ROLLBALL_CMD_RETRIES; i++) { + ret = i2c_rollball_mii_cmd(bus, bus_addr, ROLLBALL_CMD_WRITE, + buf, sizeof(buf)); + if (ret < 0) + return ret; + + ret = i2c_rollball_mii_poll(bus, bus_addr, NULL, 0); + if (ret != -ETIMEDOUT) + break; + } - ret = i2c_rollball_mii_poll(bus, bus_addr, NULL, 0); if (ret < 0) return ret; From 074107fd60865408db8aa1fc3e3ce74728273bf6 Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:23:37 +0100 Subject: [PATCH 3/6] The zero-identifier retry added previously still accepts a phydev whose identifiers are partially corrupt: a RollBall read that times out returns 0xffff for one 16-bit half of an identifier while the other half reads real data (observed: PMA ID 0xffff5081 for a BCM84891 whose true ID is 0x35905081). Such an identifier matches no driver and still leads to a hard sfp_add_phy() -EINVAL failure. Tighten the check: an identifier is only considered plausible if it is non-zero and neither 16-bit half reads 0xffff. If no MMD carries a plausible identifier, treat the PHY as not ready and return -ENODEV so the state machine's retry logic applies. A legitimate PHY ID with a 16-bit half of exactly 0xffff is not known to exist; if one ever appears, it would only cause probe retries for that module, diagnosable via the retry log message. Depends on: "net: phy: sfp: retry PHY probe when c45 identifiers read as zero" --- drivers/net/phy/sfp.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 15c06ec950ba..defa07b21da1 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -1948,20 +1948,28 @@ 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; + 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) { + if (!ids_valid) { 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 ec05e59d2071055f7ad0bcbb4d0b590b8174704f Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:24:13 +0100 Subject: [PATCH 4/6] 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 db57e0547b8919fdac060c561ac683952d1efd4a Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:25:07 +0100 Subject: [PATCH 5/6] The RollBall command completion poll allows 10 x 20 ms per attempt. An OEM SFP-10G-T module with a BCM84891 is observed to stall its I2C mailbox for long stretches while the PHY firmware is busy: under back-to-back register access, the first command after an idle period completes but subsequent commands routinely exceed the 200 ms budget, even with command re-issue retries in place. dmesg shows the probe reading one register successfully and then timing out on ~30 consecutive reads. Extend the budget to 50 iterations (~1 s per attempt). Healthy modules complete in <= 70 ms and are unaffected; the extra budget only spends time when the module is actually stalling, where the alternative is returning fabricated 0xffff data. --- drivers/net/mdio/mdio-i2c.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/mdio/mdio-i2c.c b/drivers/net/mdio/mdio-i2c.c index 3ea150b9a814..b5139eec8873 100644 --- a/drivers/net/mdio/mdio-i2c.c +++ b/drivers/net/mdio/mdio-i2c.c @@ -321,9 +321,12 @@ static int i2c_rollball_mii_poll(struct mii_bus *bus, int bus_addr, u8 *buf, msgs[1].buf = res; /* By experiment it takes up to 70 ms to access a register for these - * SFPs. Sleep 20ms between iterations and try 10 times. + * SFPs in the common case, but some modules stall for hundreds of + * milliseconds when their firmware is busy (e.g. an OEM SFP-10G-T + * with a BCM84891 running internal autonegotiation). Sleep 20 ms + * between iterations and allow up to ~1 s per command. */ - i = 10; + i = 50; do { msleep(20); From 442be472436b2a1112b636884afd0423584c868a Mon Sep 17 00:00:00 2001 From: meehien Date: Tue, 7 Jul 2026 14:26:16 +0100 Subject: [PATCH 6/6] 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,