[PATCH net-next v2 2/5] of: mdio: Scan PHY address 0 last

[Jakub Vaněk]

Some PHY devices incorrectly treat address 0 as a broadcast address.
As a result, accesses to address 0 may cause multiple PHYs to respond,
making one or both PHYs work unreliably.

On several PHYs (e.g. Motorcomm YT8821 and Realtek RTL8221B), this
behavior can be disabled via a vendor-specific internal register.
However, for that to be useful, that register would have to be
programmed before address 0 is accessed for the first time.

Device Tree-based systems scan MDIO buses via of_mdiobus_register().
Modify the scanning order so that address 0 is scanned last. This
ensures PHY fixups for addresses 1-31 are applied before address 0
is accessed, allowing the collision to be prevented.

However, preserve the original probing order for one edge case: when
the Device Tree does not explicitly specify PHY addresses. In that
scenario, PHY DT nodes appear to be matched to devices based on a
sequential bus scan. Changing the scanning sequence could change
the association between DT nodes and PHY devices and potentially break
existing setups.

For example, with:

    mdio-bus {
        phy0: ethernet-phy {
            compatible = "ethernet-phy-id1234.5678";
        };
        phy1: ethernet-phy {
            compatible = "ethernet-phy-id90AB.CDEF";
        };
    };

scanning address 0 last could cause the PHY on address 0 to be associated
with the phy1 node.

Suggested-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
Signed-off-by: Jakub Vaněk <linuxtardis@xxxxxxxxx>
---
 drivers/net/mdio/of_mdio.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index b8d298c04d3f..a705991f6b04 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -27,6 +27,11 @@ MODULE_AUTHOR("Grant Likely <grant.likely@xxxxxxxxxxxx>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("OpenFirmware MDIO bus (Ethernet PHY) accessors");
 
+enum scan_phase {
+	SCAN_PHASE_NOT_PHYAD_0,
+	SCAN_PHASE_PHYAD_0,
+};
+
 /* Extract the clause 22 phy ID from the compatible string of the form
  * ethernet-phy-idAAAA.BBBB */
 static int of_get_phy_id(struct device_node *device, u32 *phy_id)
@@ -137,7 +142,7 @@ bool of_mdiobus_child_is_phy(struct device_node *child)
 EXPORT_SYMBOL(of_mdiobus_child_is_phy);
 
 static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
-				   bool *scanphys)
+				   bool *scanphys, enum scan_phase phase)
 {
 	struct device_node *child;
 	int addr, rc = 0;
@@ -149,7 +154,7 @@ static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
 			if (!of_property_present(child, "reg"))
 				continue;
 
-			rc = __of_mdiobus_parse_phys(mdio, child, NULL);
+			rc = __of_mdiobus_parse_phys(mdio, child, NULL, phase);
 			if (rc && rc != -ENODEV)
 				goto exit;
 
@@ -164,6 +169,12 @@ static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
 			continue;
 		}
 
+		if (phase == SCAN_PHASE_NOT_PHYAD_0 && addr == 0)
+			continue;
+
+		if (phase == SCAN_PHASE_PHYAD_0 && addr != 0)
+			continue;
+
 		if (of_mdiobus_child_is_phy(child))
 			rc = of_mdiobus_register_phy(mdio, child, addr);
 		else
@@ -223,8 +234,19 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
 	if (rc)
 		return rc;
 
-	/* Loop over the child nodes and register a phy_device for each phy */
-	rc = __of_mdiobus_parse_phys(mdio, np, &scanphys);
+	/* Loop over the child nodes and register a phy_device for each phy.
+	 * However, scan address 0 last. Some vendors consider it a broadcast
+	 * address and so their PHYs respond at it in addition to the actual PHY
+	 * address. Scanning addresses 1-31 first allows PHY fixups to stop
+	 * the potential collision at address 0 from occurring.
+	 */
+	rc = __of_mdiobus_parse_phys(mdio, np, &scanphys,
+				     SCAN_PHASE_NOT_PHYAD_0);
+	if (rc)
+		goto unregister;
+
+	rc = __of_mdiobus_parse_phys(mdio, np, &scanphys,
+				     SCAN_PHASE_PHYAD_0);
 	if (rc)
 		goto unregister;
 
@@ -238,6 +260,11 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
 		    of_node_name_eq(child, "ethernet-phy-package"))
 			continue;
 
+		/* Skip the SCAN_PHASE_NOT_PHYAD_0/SCAN_PHASE_PHYAD_0
+		 * stuff here. Some device tree setups may assume linear
+		 * assignment from address 0 onwards and the two-pass probing
+		 * is not worth breaking these setups.
+		 */
 		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
 			/* skip already registered PHYs */
 			if (mdiobus_is_registered_device(mdio, addr))
-- 
2.43.0