On 12/17/2019 10:41 AM, Vladimir Oltean wrote:
On Tue, 17 Dec 2019 at 09:18, Alexandru Marginean alexm.osslist@gmail.com wrote:
On 12/15/2019 2:08 PM, Vladimir Oltean wrote:
On Tue, 3 Dec 2019 at 17:32, Alex Marginean alexandru.marginean@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 olteanv@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 <olteanv@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
How would you do that? Just put my snippet above in a higher-level wrapper over dm_eth_phy_connect and phy_connect? Otherwise I think it's pretty difficult and hacky to pass a null mdiodev pointer through dm_mdio_phy_connect.
It wouldn't go through mdio_phy_connect, dm_eth_phy_connect would need to figure out if it's a fixed link or PHY and then call the proper function, like in your snippet. The MDIO NULL topic is a bit more complicated I think. Some of the Eth drivers just deal with fixed link internally and don't create a phy device at all. If there is a PHY device I think we need to have a MDIO bus for it, and we shouldn't piggy-back on real MDIO buses. I'm thinking we could have a dummy MDIO bus created automatically by ETH/MDIO uclass code for fixed link PHY devices.
Thoughts? Alex
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