mx6sabresd: Trying to use qca,clk-out-frequency

[Michael Walle]

Am 2020-06-17 21:25, schrieb Vladimir Oltean:
> On Wed, 17 Jun 2020 at 22:22, Tom Rini <trini at konsulko.com> wrote:
>> 
>> On Wed, Jun 17, 2020 at 10:12:37PM +0300, Vladimir Oltean wrote:
>> > On Wed, 17 Jun 2020 at 22:01, Vladimir Oltean <olteanv at gmail.com> wrote:
>> > >
>> > > Hi Fabio,
>> > >
>> > > On Wed, 17 Jun 2020 at 21:30, Fabio Estevam <festevam at gmail.com> 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 at 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 <grygorii.strashko at ti.com>
>> > > 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 <grygorii.strashko at ti.com>
>> > >     Acked-by: Joe Hershberger <joe.hershberger at ni.com>
>> > >
>> > > Didn't yet figure out what to do, though.
>> > >
>> > > -Vladimir
>> >
>> > Update:
>> > I upgraded tsec to DM_MDIO:
>> > https://patchwork.ozlabs.org/project/uboot/patch/20200612151735.49048-4-Zhiqiang.Hou@nxp.com/
>> > and it now prints the correct phy node:
>> > ar803x_of_init: found PHY node: ethernet-phy at 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
> 
> Not Grygorii's fault, I'm sure. He just noticed that there wasn't an
> of node for the PHY, and created one.
> 
> Today I learned, with the phy_connect API, you're supposed to populate
> the phydev node _yourself_.
> Like zynq_gem does:
> 
> https://gitlab.denx.de/u-boot/u-boot/-/blob/master/drivers/net/zynq_gem.c#L330
> 
> Too bad almost nobody else does......

Almost nobody ;)
https://lists.denx.de/pipermail/u-boot/2019-October/388360.html

-michael