[PATCH 4/6] net: fec: add support for DM_MDIO

Tim Harvey tharvey at gateworks.com
Thu Mar 31 19:48:55 CEST 2022


On Thu, Mar 31, 2022 at 10:01 AM Vladimir Oltean
<vladimir.oltean at nxp.com> wrote:
>
> On Tue, Mar 29, 2022 at 03:52:38PM -0700, Tim Harvey wrote:
> > Add support for DM_MDIO by registering a UCLASS_MDIO driver and
> > attempting to use it. This is necessary if wanting to use a DSA
> > driver for example hanging off of the FEC MAC.
> >
> > Care is taken to fallback to non DM_MDIO as several boards define
> > DM_MDIO without having the proper device-tree configuration necessary
> > such as an mdio subnode, a phy-mode prop, and either a valid phy-handle
> > prop or fixed-phy subnode.
> >
> > Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> > ---
> >  drivers/net/fec_mxc.c | 113 ++++++++++++++++++++++++++++++++++++++----
> >  drivers/net/fec_mxc.h |   1 +
> >  2 files changed, 104 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> > index e8ebef09032a..374ef9d67d5e 100644
> > --- a/drivers/net/fec_mxc.c
> > +++ b/drivers/net/fec_mxc.c
> > @@ -30,6 +30,8 @@
> >  #include <asm/arch/imx-regs.h>
> >  #include <asm/mach-imx/sys_proto.h>
> >  #include <asm-generic/gpio.h>
> > +#include <dm/device_compat.h>
> > +#include <dm/lists.h>
> >
> >  #include "fec_mxc.h"
> >  #include <eth_phy.h>
> > @@ -1025,6 +1027,80 @@ struct mii_dev *fec_get_miibus(ulong base_addr, int dev_id)
> >       return bus;
> >  }
> >
> > +#ifdef CONFIG_DM_MDIO
> > +struct dm_fec_mdio_priv {
> > +     struct ethernet_regs *regs;
> > +};
> > +
> > +static int dm_fec_mdio_read(struct udevice *dev, int addr, int devad, int reg)
> > +{
> > +     struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> > +
> > +     return fec_mdio_read(priv->regs, addr, reg);
> > +}
> > +
> > +static int dm_fec_mdio_write(struct udevice *dev, int addr, int devad, int reg, u16 data)
> > +{
> > +     struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> > +
> > +     return fec_mdio_write(priv->regs, addr, reg, data);
> > +}
> > +
> > +static const struct mdio_ops dm_fec_mdio_ops = {
> > +     .read = dm_fec_mdio_read,
> > +     .write = dm_fec_mdio_write,
> > +};
> > +
> > +static int dm_fec_mdio_probe(struct udevice *dev)
> > +{
> > +     struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> > +
> > +     priv->regs = (struct ethernet_regs *)ofnode_get_addr(dev_ofnode(dev->parent));
> > +
> > +     return 0;
> > +}
> > +
> > +U_BOOT_DRIVER(fec_mdio) = {
> > +     .name           = "fec_mdio",
> > +     .id             = UCLASS_MDIO,
> > +     .probe          = dm_fec_mdio_probe,
> > +     .ops            = &dm_fec_mdio_ops,
> > +     .priv_auto      = sizeof(struct dm_fec_mdio_priv),
> > +};
> > +
> > +static int dm_fec_bind_mdio(struct udevice *dev)
> > +{
> > +     struct udevice *mdiodev;
> > +     const char *name;
> > +     ofnode mdio;
> > +     int ret;
> > +
> > +     /* for a UCLASS_MDIO driver we need to bind and probe manually
> > +      * for an internal MDIO bus that has no dt compatible of its own
> > +      */
> > +     ofnode_for_each_subnode(mdio, dev_ofnode(dev)) {
> > +             name = ofnode_get_name(mdio);
> > +
> > +             if (strcmp(name, "mdio"))
> > +                     continue;
> > +
> > +             ret = device_bind_driver_to_node(dev, "fec_mdio",
> > +                                              name, mdio, &mdiodev);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             /* need to probe it as there is no compatible to do so */
> > +             ret = uclass_get_device_by_ofnode(UCLASS_MDIO, mdio, &mdiodev);
> > +             if (!ret)
> > +                     return 0;
> > +
> > +             break;
> > +     }
> > +
> > +     return ret;
> > +}
> > +#endif
> > +
>
> On which branch does this apply? The context above fecmxc_read_rom_hwaddr()
> is different in the branches I've checked:
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L1276
> https://source.denx.de/u-boot/custodians/u-boot-net/-/blob/next/drivers/net/fec_mxc.c#L1276
>

Sorry, I should have specified in my cover letter that this is on top
of next which removes the non dm-eth support from the driver which is
quite the cleanup.

Also, after more testing I believe the dm-mdio driver code 'above' is
correct but the business of trying to use it and fallback 'below' is
wrong and needs work. It doesn't break current users of fec_mxc from
what I can tell but it also doesn't properly connect the dm_mdio
driver.

> >  static int fecmxc_read_rom_hwaddr(struct udevice *dev)
> >  {
> >       struct fec_priv *priv = dev_get_priv(dev);
> > @@ -1088,7 +1164,7 @@ static int device_get_phy_addr(struct fec_priv *priv, struct udevice *dev)
> >
> >  static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
> >  {
> > -     struct phy_device *phydev;
> > +     struct phy_device *phydev = NULL;
> >       int addr;
> >
> >       addr = device_get_phy_addr(priv, dev);
> > @@ -1096,7 +1172,12 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
> >       addr = CONFIG_FEC_MXC_PHYADDR;
> >  #endif
> >
> > -     phydev = phy_connect(priv->bus, addr, dev, priv->interface);
> > +#ifdef CONFIG_DM_MDIO
> > +     if (priv->dm_mdio)
> > +             phydev = dm_eth_phy_connect(dev);
> > +#endif
> > +     if (!phydev)
> > +             phydev = phy_connect(priv->bus, addr, dev, priv->interface);
> >       if (!phydev)
> >               return -ENODEV;
> >
> > @@ -1227,11 +1308,19 @@ static int fecmxc_probe(struct udevice *dev)
> >
> >       priv->dev_id = dev_seq(dev);
> >
> > +#ifdef CONFIG_DM_MDIO
> > +     ret = dm_fec_bind_mdio(dev);
> > +     if (!ret) {
> > +             ret = fec_phy_init(priv, dev);
> > +             if (!ret)
> > +                     priv->dm_mdio = true;
> > +     }
> > +#endif
> >  #ifdef CONFIG_DM_ETH_PHY
> >       bus = eth_phy_get_mdio_bus(dev);
> >  #endif
> >
> > -     if (!bus) {
> > +     if (!bus && !priv->dm_mdio) {
> >               dm_mii_bus = false;
> >  #ifdef CONFIG_FEC_MXC_MDIO_BASE
> >               bus = fec_get_miibus((ulong)CONFIG_FEC_MXC_MDIO_BASE,
> > @@ -1240,7 +1329,7 @@ static int fecmxc_probe(struct udevice *dev)
> >               bus = fec_get_miibus((ulong)priv->eth, dev_seq(dev));
> >  #endif
> >       }
> > -     if (!bus) {
> > +     if (!bus && !priv->dm_mdio) {
> >               ret = -ENOMEM;
> >               goto err_mii;
> >       }
> > @@ -1271,14 +1360,16 @@ static int fecmxc_probe(struct udevice *dev)
> >               break;
> >       }
> >
> > -     ret = fec_phy_init(priv, dev);
> > -     if (ret)
> > -             goto err_phy;
> > +     if (!priv->dm_mdio) {
> > +             ret = fec_phy_init(priv, dev);
> > +             if (ret)
> > +                     goto err_phy;
> > +     }
> >
> >       return 0;
> >
> >  err_phy:
> > -     if (!dm_mii_bus) {
> > +     if (!dm_mii_bus && !priv->dm_mdio) {
> >               mdio_unregister(bus);
> >               free(bus);
> >       }
> > @@ -1294,8 +1385,10 @@ static int fecmxc_remove(struct udevice *dev)
> >
> >       free(priv->phydev);
> >       fec_free_descs(priv);
> > -     mdio_unregister(priv->bus);
> > -     mdio_free(priv->bus);
> > +     if (priv->bus) {
> > +             mdio_unregister(priv->bus);
> > +             mdio_free(priv->bus);
> > +     }
> >
> >  #ifdef CONFIG_DM_REGULATOR
> >       if (priv->phy_supply)
> > diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
> > index 48faa33d66ec..c297880ccc54 100644
> > --- a/drivers/net/fec_mxc.h
> > +++ b/drivers/net/fec_mxc.h
> > @@ -248,6 +248,7 @@ struct fec_priv {
> >       uint8_t *tdb_ptr;
> >       int dev_id;
> >       struct mii_dev *bus;
> > +     bool dm_mdio;
> >  #ifdef CONFIG_PHYLIB
> >       struct phy_device *phydev;
> >       ofnode phy_of_node;
> > --
> > 2.17.1
> >

What I'm trying to accomplish here vs just simply using
dm_fec_bind_mdio(dev) and dm_eth_phy_connect(dev) is to provide a
fallback for the many users of fec_mxc that do not have a dt that
works with dm-mdio yet still have defconfig's that enable DM_MDIO.
Most of the users would not currently have an mdio subnode in the fec
node, nor have the required phy-mode 'and' phy-handle prop or
fixed-link subnode which would cause the dm_eth_phy_connect to fail.

Best Regards,

Tim


More information about the U-Boot mailing list