From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexandru Marginean Date: Tue, 17 Dec 2019 08:18:42 +0100 Subject: [PATCH v3 1/6] net: introduce DSA class for Ethernet switches In-Reply-To: References: <20191203145645.13361-1-alexandru.marginean@nxp.com> <20191203145645.13361-2-alexandru.marginean@nxp.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 12/15/2019 2:08 PM, Vladimir Oltean wrote: > On Tue, 3 Dec 2019 at 17:32, Alex Marginean 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 > 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 > > 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 >