On Wed, Jun 17, 2020 at 10:12:37PM +0300, Vladimir Oltean wrote: > On Wed, 17 Jun 2020 at 22:01, Vladimir Oltean <[email protected]> wrote: > > > > Hi Fabio, > > > > On Wed, 17 Jun 2020 at 21:30, Fabio Estevam <[email protected]> wrote: > > > > > > Hi, > > > > > > Now that mx6sabresd board has Ethernet working again with this fix: > > > https://lists.denx.de/pipermail/u-boot/2020-June/416623.html > > > > > > ,I would like to remove the ar8031_phy_fixup() from > > > board/freescale/mx6sabresd/mx6sabresd.c. > > > > > > Here is what I tested so far: https://pastebin.com/raw/cQW5RmXW > > > > > > However, I see that in drivers/net/phy/atheros.c the > > > > > > if (!ofnode_read_u32(node, "qca,clk-out-frequency", &freq)) { > > > > > > condition is never executed, so qca,clk-out-frequency configuration > > > does not take effect. > > > > > > Any ideas why this happens? > > > > > > Thanks > > > > I just tried it out on a board I have. > > This: > > printf("%s: found PHY node: %s\n", __func__, ofnode_get_name(node)); > > prints this: > > ar803x_of_init: found PHY node: ethernet@2d10000 > > > > which is very odd, because it's not the ethernet-phy node, but the > > node of the parent. > > It happens because ofnode_valid() is false here: > > > > static inline ofnode phy_get_ofnode(struct phy_device *phydev) > > { > > if (ofnode_valid(phydev->node)) > > return phydev->node; > > else > > return dev_ofnode(phydev->dev); > > } > > > > which again seems to be caused by this commit: > > > > commit eef0b8a930d1a8799b8ebd26e67e27401df6a9f7 > > Author: Grygorii Strashko <[email protected]> > > Date: Thu Jul 5 12:02:48 2018 -0500 > > > > net: phy: add ofnode node to struct phy_device > > > > Now the UCLASS_ETH device "node" field is owerwritten by some > > network drivers in > > case of Ethernet PHYs which are linked to UCLASS_ETH device using > > "phy-handle" DT property and when Ethernet PHY driver needs to read some > > additional information from DT. In such cases following happens (in > > general): > > > > - network drivers > > priv->phydev = phy_connect(priv->bus, priv->phyaddr, dev, > > priv->interface); > > <-- phydev is connected to dev which is UCLASS_ETH device > > > > if (priv->phy_of_handle > 0) > > dev_set_of_offset(priv->phydev->dev, > > priv->phy_of_handle); > > <-- phydev->dev->node is overwritten by phy-handle DT node > > > > - PHY driver in .config() callback > > int node = dev_of_offset(dev); > > <-- PHY driver uses overwritten dev->node > > const void *fdt = gd->fdt_blob; > > > > if (fdtdec_get_bool(fdt, node, "property")) > > ... > > > > As result, UCLASS_ETH device can't be used any more for DT accessing. > > > > This patch adds additional ofnode node field to struct phy_device which > > can > > be set explicitly by network drivers and used by PHY drivers, so > > overwriting can be avoided. Also add helper function phy_get_ofnode() > > which will check and return phy_device->node or dev_ofnode(phydev->dev) > > for > > backward compatibility with existing drivers. > > > > Signed-off-by: Grygorii Strashko <[email protected]> > > Acked-by: Joe Hershberger <[email protected]> > > > > Didn't yet figure out what to do, though. > > > > -Vladimir > > Update: > I upgraded tsec to DM_MDIO: > https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ > and it now prints the correct phy node: > ar803x_of_init: found PHY node: ethernet-phy@2 > > Pretty sure that DM_MDIO is also how Michael tested this. > What I'm pretty fuzzy about is: do we _need_ DM_MDIO for this to work? > Who's more knowledgeable about this stuff around here?
Grygorii can you chime in more on your thinking here? Thanks! -- Tom
signature.asc
Description: PGP signature
