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 Signed-off-by: Jakub Vaněk --- 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 "); 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