On 12/15/2019 2:08 PM, Vladimir Oltean wrote:
On Tue, 3 Dec 2019 at 17:32, Alex Marginean <alexandru.margin...@nxp.com> wrote:
+/**
+ * This function deals with additional devices around the switch as these 
should
+ * have been bound to drivers by now.
+ * TODO: pick up references to other switch devices here, if we're cascaded.
+ */
+static int dm_dsa_pre_probe(struct udevice *dev)
+{
+       struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+       int i;
+
+       if (!platdata)
+               return -EINVAL;
+
+       if (ofnode_valid(platdata->master_node))
+               uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
+                                            &platdata->master_dev);
+
+       for (i = 0; i < platdata->num_ports; i++) {
+               struct dsa_port_platdata *port = &platdata->port[i];
+
+               if (port->dev) {
+                       port->dev->priv = port;
+                       port->phy = dm_eth_phy_connect(port->dev);

Fixed-link interfaces don't work with DM_MDIO. That is somewhat
natural as there is no MDIO bus for a fixed-link. However the legacy
phy_connect function can be made rather easily to work with
fixed-link, since it has the necessary code for dealing with it
already. I am not, however, sure how it ever worked in the absence of
an MDIO bus.

commit 1b7e23cc7e6d0dc3fe7ae9c55390b40717ff3c3a
Author: Vladimir Oltean <olte...@gmail.com>
Date:   Sat Dec 14 23:25:40 2019 +0200

     phy: make phy_connect_fixed work with a null mdio bus

     It is utterly pointless to require an MDIO bus pointer for a fixed PHY
     device. The fixed.c implementation does not require it, only
     phy_device_create. Fix that.

     Signed-off-by: Vladimir Oltean <olte...@gmail.com>

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 80a7664e4978..8ea5c9005291 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -658,7 +658,7 @@ static struct phy_device *phy_device_create(struct
mii_dev *bus, int addr,
         dev = malloc(sizeof(*dev));
         if (!dev) {
                 printf("Failed to allocate PHY device for %s:%d\n",
-                      bus->name, addr);
+                      bus ? bus->name : "(null bus)", addr);
                 return NULL;
         }

@@ -686,7 +686,7 @@ static struct phy_device *phy_device_create(struct
mii_dev *bus, int addr,
                 return NULL;
         }

-       if (addr >= 0 && addr < PHY_MAX_ADDR)
+       if (addr >= 0 && addr < PHY_MAX_ADDR && phy_id != PHY_FIXED_ID)
                 bus->phymap[addr] = dev;

         return dev;

With the patch above in place, fixed-link can also be made to work
with some logic similar to what can be seen below:

     if (ofnode_valid(ofnode_find_subnode(port->dev->node, "fixed-link")))
         port->phy = phy_connect(NULL, 0, port->dev, phy_mode); //
phy_mode needs to be pre-parsed somewhere else as well
     else
         port->phy = dm_eth_phy_connect(port->dev);

How would you see fixed-link interfaces being treated? My question so
far is in the context of front-panel ports but I am interested in your
view of the CPU port situation as well.

I was thinking turning dm_eth_phy_connect into a more generic helper that also deals with fixed links, which it does not yet. That would move the "fixed-link" if out of the driver code. Ideally the driver should be able to call a single helper and, if the device has a DT node, it would get back a PHY handle to either a proper PHY or to a fixed link (from phy_connect_fixed).

Alex


+               }
+       }
+
+       return 0;
+}

Thanks,
-Vladimir

Reply via email to