diff --git a/drivers/net/mdio/mdio-i2c.c b/drivers/net/mdio/mdio-i2c.c index ed20352a589a..3afb6da3ca97 100644 --- a/drivers/net/mdio/mdio-i2c.c +++ b/drivers/net/mdio/mdio-i2c.c @@ -11,6 +11,7 @@ * of their settings. */ #include +#include #include #include #include @@ -195,6 +196,26 @@ 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 +/* Wall-clock budget for i2c_rollball_mii_poll(): generous enough to + * ride out the module's post-insertion wake window, during which the + * mailbox can stall for hundreds of ms. + */ +#define ROLLBALL_POLL_TIMEOUT_MS 1000 + +/* Completion-poll backoff: first sleep is short so modules that answer + * in a couple of ms return promptly; the interval then doubles up to + * the cap so modules that take tens of ms per command are not hammered + * with status reads (each poll iteration costs four I2C transactions + * for the page save/set/read/restore dance). + */ +#define ROLLBALL_POLL_SLEEP_MIN_US 2000 +#define ROLLBALL_POLL_SLEEP_MAX_US 32000 + +/* 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, @@ -298,7 +319,9 @@ static int i2c_rollball_mii_poll(struct mii_bus *bus, int bus_addr, u8 *buf, struct i2c_adapter *i2c = bus->priv; struct i2c_msg msgs[2]; u8 cmd_addr, tmp, *res; - int i, ret; + unsigned long deadline; + unsigned long sleep_us; + int ret; cmd_addr = ROLLBALL_CMD_ADDR; @@ -315,12 +338,22 @@ static int i2c_rollball_mii_poll(struct mii_bus *bus, int bus_addr, u8 *buf, msgs[1].len = len; 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. + /* Some of these SFPs answer within a couple of ms once running, + * others take tens of ms per command, and any of them can stall + * for hundreds of ms while the module is still waking after + * insertion. Poll with exponential backoff: the first iteration + * sleeps only a couple of ms so a fast module returns promptly + * instead of paying a fixed 20 ms per register access, while the + * backoff keeps the number of status-poll I2C transactions for a + * slow module close to the fixed-interval scheme's. The whole + * loop is bounded by a generous wall-clock budget for the wake + * window. Sleep as TASK_IDLE (usleep_range_idle): the CPU is free + * while we wait, so the poll must not inflate the load average. */ - i = 10; + deadline = jiffies + msecs_to_jiffies(ROLLBALL_POLL_TIMEOUT_MS); + sleep_us = ROLLBALL_POLL_SLEEP_MIN_US; do { - msleep(20); + usleep_range_idle(sleep_us, sleep_us * 2); ret = i2c_transfer_rollball(i2c, msgs, ARRAY_SIZE(msgs)); if (ret) @@ -328,7 +361,10 @@ static int i2c_rollball_mii_poll(struct mii_bus *bus, int bus_addr, u8 *buf, if (*res == ROLLBALL_CMD_DONE) return 0; - } while (i-- > 0); + + sleep_us = min_t(unsigned long, sleep_us * 2, + ROLLBALL_POLL_SLEEP_MAX_US); + } while (time_before(jiffies, deadline)); dev_dbg(&bus->dev, "poll timed out\n"); @@ -362,7 +398,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 +410,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 +440,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 +454,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; diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c index 2ae70dcf82ec..73ddfbb215c5 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 @@ -246,6 +260,32 @@ static int bcm84881_get_features(struct phy_device *phydev) return 0; } +static const int bcm8489x_features[] = { + ETHTOOL_LINK_MODE_Autoneg_BIT, + ETHTOOL_LINK_MODE_100baseT_Half_BIT, + ETHTOOL_LINK_MODE_100baseT_Full_BIT, + ETHTOOL_LINK_MODE_1000baseT_Full_BIT, + ETHTOOL_LINK_MODE_2500baseT_Full_BIT, + ETHTOOL_LINK_MODE_5000baseT_Full_BIT, + ETHTOOL_LINK_MODE_10000baseT_Full_BIT, +}; + +static int bcm8489x_get_features(struct phy_device *phydev) +{ + /* The PMA/PMD abilities of this family are fixed and known, and + * on some modules every MDIO access is expensive (RollBall + * MDIO-over-I2C mailbox, tens to hundreds of ms per register), + * so do not spend half a dozen reads at attach time discovering + * constants. EEE abilities may vary with firmware; keep reading + * those. + */ + linkmode_set_bit_array(bcm8489x_features, + ARRAY_SIZE(bcm8489x_features), + phydev->supported); + + return genphy_c45_read_eee_abilities(phydev); +} + static int bcm84881_config_aneg(struct phy_device *phydev) { bool changed = false; @@ -303,8 +343,45 @@ static int bcm84881_aneg_done(struct phy_device *phydev) static int bcm84881_read_status(struct phy_device *phydev) { + bool was_resolved; unsigned int mode; - int bmsr, val; + int bmsr, val, stat1; + + /* Whether the previous poll left a fully resolved link whose + * negotiated parameters (lp_advertising, speed, duplex, pause, + * mdix, interface) are still valid. They can only change through + * a renegotiation, which is observable below as a link drop, an + * autoneg restart or autoneg-complete deasserting. + */ + was_resolved = phydev->link && phydev->autoneg_complete && + phydev->speed != SPEED_UNKNOWN; + + stat1 = -1; + + /* In polling mode with the link previously up and resolved, a + * single read of the AN status register is normally sufficient + * to confirm nothing changed: the link status bit is latched + * low, so a momentary drop or a renegotiation since the last + * poll reads as 0 even if the link has already come back, and + * autoneg-complete deasserts across a renegotiation. This + * mirrors the polling-mode single-read logic in + * genphy_c45_read_link(). + */ + if (phy_polling_mode(phydev) && was_resolved) { + stat1 = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); + if (stat1 < 0) + return stat1; + + if ((stat1 & MDIO_STAT1_LSTATUS) && + (stat1 & MDIO_AN_STAT1_COMPLETE)) + return 0; + + /* Something changed. The latched status has now been + * consumed, so the full evaluation below must reuse + * this value rather than re-read the register, which + * would miss a momentary link drop. + */ + } val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1); if (val < 0) @@ -315,21 +392,32 @@ static int bcm84881_read_status(struct phy_device *phydev) return 0; } - val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); - if (val < 0) - return val; + if (stat1 < 0) { + stat1 = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); + if (stat1 < 0) + return stat1; + } bmsr = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_C22 + MII_BMSR); if (bmsr < 0) return bmsr; - phydev->autoneg_complete = !!(val & MDIO_AN_STAT1_COMPLETE) && + phydev->autoneg_complete = !!(stat1 & MDIO_AN_STAT1_COMPLETE) && !!(bmsr & BMSR_ANEGCOMPLETE); - phydev->link = !!(val & MDIO_STAT1_LSTATUS) && + phydev->link = !!(stat1 & MDIO_STAT1_LSTATUS) && !!(bmsr & BMSR_LSTATUS); if (phydev->autoneg == AUTONEG_ENABLE && !phydev->autoneg_complete) phydev->link = false; + /* On some modules every MDIO access is expensive (RollBall + * MDIO-over-I2C mailbox, tens to hundreds of ms per register). + * If the link was already up and resolved on the previous poll + * and still is, the negotiated parameters cannot have changed: + * skip re-reading them and keep the cached values. + */ + if (was_resolved && phydev->link && phydev->autoneg_complete) + return 0; + linkmode_zero(phydev->lp_advertising); phydev->speed = SPEED_UNKNOWN; phydev->duplex = DUPLEX_UNKNOWN; @@ -416,12 +504,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, @@ -432,9 +538,10 @@ 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, + .get_features = bcm8489x_get_features, .config_aneg = bcm84881_config_aneg, .aneg_done = bcm84881_aneg_done, .read_status = bcm84881_read_status, @@ -446,9 +553,10 @@ 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, + .get_features = bcm8489x_get_features, .config_aneg = bcm84881_config_aneg, .aneg_done = bcm84881_aneg_done, .read_status = bcm84881_read_status, diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index e1358a015442..defa07b21da1 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -1938,6 +1938,44 @@ 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_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++) { + u32 id = phy->c45_ids.device_ids[i]; + + if (id && (id & 0xffff) != 0xffff && + (id >> 16) != 0xffff) { + ids_valid = true; + break; + } + } + + if (!ids_valid) { + dev_info(sfp->dev, + "PHY identifiers unstable (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 +1989,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));