net: mdio: of_mdio: check for already registered phy before creating new instances

In of_mdiobus_register_phy(), check if the phy with the given address is already registered within the mii bus before calling phy_device_create() or get_phy_device().

This allows us to augment auto-probed phy devices with extra information via DT. Without this patch, a second instance of the phydev is created unnecessarily.

Signed-off-by Daniel Mack <[email protected]>

---
 drivers/of/of_mdio.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

drivers/of/of_mdio.c +9 -5

index 9a95831..264ea3f 100644
@@ -72,10 +72,15 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
        is_c45 = of_device_is_compatible(child,
                                         "ethernet-phy-ieee802.3-c45");
 
-       if (!is_c45 && !of_get_phy_id(child, &phy_id))
-               phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
-       else
-               phy = get_phy_device(mdio, addr, is_c45);
+       /* Check if the phy we're looking for is already registered */
+       phy = mdio->phy_map[addr];
+       if (!phy) {
+               if (!is_c45 && !of_get_phy_id(child, &phy_id))
+                       phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
+               else
+                       phy = get_phy_device(mdio, addr, is_c45);
+       }
+
        if (!phy || IS_ERR(phy))
                return 1;
 
--

1.9.0

2014-05-06 8:21 GMT-07:00 Daniel Mack <[email protected]>:

Quoted 3 lines

In of_mdiobus_register_phy(), check if the phy with the given address is already registered within the mii bus before calling phy_device_create() or get_phy_device().

I am not exactly sure how you could be in that sort of situation. of_mdiobus_register() handles two different cases at the moment:

1) PHY child nodes have a valid 'reg' property, and this property is used to register the PHY at the given address
2) if a PHY child node does not have a valid 'reg' property, which will trigger an auto-scan and then we iterate through all 32 addresses of the bus, we skip over PHYs that are already registered

Quoted 1 line

Quoted 3 lines

This allows us to augment auto-probed phy devices with extra information via DT. Without this patch, a second instance of the phydev is created unnecessarily.

Which piece of code is doing the auto-probing in your case? One of the very first things that of_mdiobus_register() does is set the PHY mask to 0xffffffff to prevent the default PHY probing method to trigger, since we are using information from the Device Tree right after that.

(+ Mugunthan V N)

Hi Florian,

Quoted 15 lines · Florian Fainelli

2014-05-06 8:21 GMT-07:00 Daniel Mack <[email protected]>:

Quoted 3 lines

In of_mdiobus_register_phy(), check if the phy with the given address is already registered within the mii bus before calling phy_device_create() or get_phy_device().

I am not exactly sure how you could be in that sort of situation. of_mdiobus_register() handles two different cases at the moment:

1) PHY child nodes have a valid 'reg' property, and this property is used to register the PHY at the given address
2) if a PHY child node does not have a valid 'reg' property, which will trigger an auto-scan and then we iterate through all 32 addresses of the bus, we skip over PHYs that are already registered

Quoted 1 line

Quoted 8 lines

Quoted 3 lines

This allows us to augment auto-probed phy devices with extra information via DT. Without this patch, a second instance of the phydev is created unnecessarily.

Which piece of code is doing the auto-probing in your case? One of the very first things that of_mdiobus_register() does is set the PHY mask to 0xffffffff to prevent the default PHY probing method to trigger, since we are using information from the Device Tree right after that.

Ah, ok. So what happens here is that of_mdiobus_register() sets the phy_mask to ~0, but mdiobus_register() calls bus->reset(), which resets the mask to ffffffef in my case. bus->reset is davinci_mdio_reset() in my case.

Is the davinci mdio driver doing anything wrong by touching bus->phy_mask?

Another solution would be to split mdiobus_register() and create a mdiobus_register_noscan() or something, and then call that from of_mdiobus_register().

2014-05-07 9:01 GMT-07:00 Daniel Mack <[email protected]>:

Quoted 3 lines

Another solution would be to split mdiobus_register() and create a mdiobus_register_noscan() or something, and then call that from of_mdiobus_register().

That, or have a specific MDIO bus controller node boolean property such as "mdio-bus-autoscan" or something similar which will tell of_mdiobus_register() not to override the phy_mask since the MDIO bus controller is capable of auto-detecting the PHYs present.

This should not be too controversial as we should really be describing a feature of the hardware here.

From a DT user point of view, I believe that the behaviour with my patch