Driver support for bcm84891 SFP+

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,
@@ -316,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);
 
@@ -362,7 +370,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 +382,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 +412,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 +426,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;
 

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
@@ -416,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,
@@ -432,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,
@@ -446,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,

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));