[U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices

Joe Hershberger joe.hershberger at ni.com
Sat Jun 1 18:42:06 UTC 2019


Hi Bin,

Thanks for the review. I shouldn't have tried to look at this while
falling asleep in the airport. :(

-Joe

On Sat, Jun 1, 2019 at 12:16 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Alex,
>
> On Sat, Jun 1, 2019 at 12:27 AM Alex Marginean <alexm.osslist at gmail.com> wrote:
> >
> > Adds UCLASS_MDIO DM class supporting MDIO buses that are probed as
> > stand-alone devices.  Useful in particular for systems that support
> > DM_ETH and have a stand-alone MDIO hardware block shared by multiple
> > Ethernet interfaces.
> >
>
> Please add a test case for testing mdio (see test/dm/*)
>
> > Signed-off-by: Alex Marginean <alexm.osslist at gmail.com>
> > ---
> >  cmd/mdio.c             |   5 ++
> >  drivers/net/Kconfig    |  13 +++++
> >  include/dm/uclass-id.h |   1 +
> >  include/miiphy.h       |  50 ++++++++++++++++++++
> >  net/Makefile           |   1 +
> >  net/mdio-uclass.c      | 105 +++++++++++++++++++++++++++++++++++++++++
> >  6 files changed, 175 insertions(+)
> >  create mode 100644 net/mdio-uclass.c
> >
> > diff --git a/cmd/mdio.c b/cmd/mdio.c
> > index 5e219f699d..a6fa9266d0 100644
> > --- a/cmd/mdio.c
> > +++ b/cmd/mdio.c
> > @@ -203,6 +203,11 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >         if (argc < 2)
> >                 return CMD_RET_USAGE;
> >
> > +#ifdef CONFIG_DM_MDIO
> > +       /* probe DM MII device before any operation so they are all accesible */
> > +       dm_mdio_probe_devices();
> > +#endif
> > +
> >         /*
> >          * We use the last specified parameters, unless new ones are
> >          * entered.
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index e6a4fdf30e..6fba5a84dd 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -11,6 +11,19 @@ config DM_ETH
> >           This is currently implemented in net/eth-uclass.c
> >           Look in include/net.h for details.
> >
> > +config DM_MDIO
> > +       bool "Enable Driver Model for MDIO devices"
> > +       depends on DM_ETH && PHYLIB
> > +       help
> > +         Enable driver model for MDIO devices
> > +
> > +         Adds UCLASS_MDIO DM class supporting MDIO buses that are probed as
> > +         stand-alone devices.  Useful in particular for systems that support
> > +         DM_ETH and have a stand-alone MDIO hardware block shared by multiple
> > +         Ethernet interfaces.
> > +         This is currently implemented in net/mdio-uclass.c
> > +         Look in include/miiphy.h for details.
> > +
> >  menuconfig NETDEVICES
> >         bool "Network device support"
> >         depends on NET
> > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> > index 09e0ad5391..90667e62cf 100644
> > --- a/include/dm/uclass-id.h
> > +++ b/include/dm/uclass-id.h
> > @@ -58,6 +58,7 @@ enum uclass_id {
> >         UCLASS_LPC,             /* x86 'low pin count' interface */
> >         UCLASS_MAILBOX,         /* Mailbox controller */
> >         UCLASS_MASS_STORAGE,    /* Mass storage device */
> > +       UCLASS_MDIO,            /* MDIO bus */
> >         UCLASS_MISC,            /* Miscellaneous device */
> >         UCLASS_MMC,             /* SD / MMC card or chip */
> >         UCLASS_MOD_EXP,         /* RSA Mod Exp device */
> > diff --git a/include/miiphy.h b/include/miiphy.h
> > index f11763affd..455feacd33 100644
> > --- a/include/miiphy.h
> > +++ b/include/miiphy.h
> > @@ -118,4 +118,54 @@ int bb_miiphy_write(struct mii_dev *miidev, int addr, int devad, int reg,
> >  #define ESTATUS_1000XF         0x8000
> >  #define ESTATUS_1000XH         0x4000
> >
> > +#ifdef CONFIG_DM_MDIO
> > +
> > +/**
> > + * struct mii_pdata - Platform data for MDIO buses
>
> wrong name: mii_pdata
>
> > + *
> > + * @mii_bus: Supporting MII legacy bus
> > + * @priv_pdata: device specific platdata
>
> there is no such field called priv_pdata.
>
> > + */
> > +struct mdio_perdev_priv {
> > +       struct mii_dev *mii_bus;
> > +};
> > +
> > +/**
> > + * struct mii_ops - MDIO bus operations
>
> struct mdio_ops
>
> > + *
> > + * @read: Read from a PHY register
> > + * @write: Write to a PHY register
> > + * @reset: Reset the MDIO bus, NULL if not supported
> > + */
> > +struct mdio_ops {
> > +       int (*read)(struct udevice *mdio_dev, int addr, int devad, int reg);
> > +       int (*write)(struct udevice *mdio_dev, int addr, int devad, int reg,
> > +                    u16 val);
> > +       int (*reset)(struct udevice *mdio_dev);
> > +};
> > +
> > +#define mdio_get_ops(dev) ((struct mdio_ops *)(dev)->driver->ops)
> > +
> > +/**
> > + * mii_dm_probe_devices - Call probe on all MII devices, currently used for
>
> wrong name "mii_dm_probe_devices". MII devices -> MDIO devices?
>
> > + * MDIO console commands.
> > + */
> > +void dm_mdio_probe_devices(void);
> > +
> > +/**
> > + * dm_mdio_phy_connect - Wrapper over phy_connect for DM MDIO
> > + *
> > + * @dev: mdio dev
> > + * @addr: PHY address on MDIO bus
> > + * @ethdev: ethernet device to connect to the PHY
> > + * @interface: MAC-PHY protocol
> > + *
> > + * @return pointer to phy_device, or 0 on error
> > + */
> > +struct phy_device *dm_mdio_phy_connect(struct udevice *dev, int addr,
> > +                                      struct udevice *ethdev,
> > +                                      phy_interface_t interface);
> > +
> > +#endif
> > +
> >  #endif
> > diff --git a/net/Makefile b/net/Makefile
> > index ce36362168..6251ff3991 100644
> > --- a/net/Makefile
> > +++ b/net/Makefile
> > @@ -15,6 +15,7 @@ obj-$(CONFIG_NET)      += eth-uclass.o
> >  else
> >  obj-$(CONFIG_NET)      += eth_legacy.o
> >  endif
> > +obj-$(CONFIG_DM_MDIO)  += mdio-uclass.o
> >  obj-$(CONFIG_NET)      += eth_common.o
> >  obj-$(CONFIG_CMD_LINK_LOCAL) += link_local.o
> >  obj-$(CONFIG_NET)      += net.o
> > diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
> > new file mode 100644
> > index 0000000000..6ae7b4ecfd
> > --- /dev/null
> > +++ b/net/mdio-uclass.c
> > @@ -0,0 +1,105 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2019
> > + * Alex Marginean, NXP
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <miiphy.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/uclass-internal.h>
> > +
> > +void dm_mdio_probe_devices(void)
> > +{
> > +       struct udevice *it;
> > +       struct uclass *uc;
> > +
> > +       uclass_get(UCLASS_MDIO, &uc);
> > +       uclass_foreach_dev(it, uc) {
> > +               device_probe(it);
> > +       }
> > +}
> > +
> > +static int dm_mdio_post_bind(struct udevice *dev)
> > +{
> > +       if (strchr(dev->name, ' ')) {
>
> Is such check a must have? If yes, could you please add some comments?
>
> > +               debug("\nError: MDIO device name \"%s\" has a space!\n",
> > +                     dev->name);
> > +               return -EINVAL;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static int dm_mdio_read(struct mii_dev *mii_bus, int addr, int devad, int reg)
>
> This does not look correct. Normally the dm_ functions should have
> udevice * as its first argument.
>
> > +{
> > +       struct udevice *dev = mii_bus->priv;
> > +
> > +       return mdio_get_ops(dev)->read(dev, addr, devad, reg);
> > +}
> > +
> > +static int dm_mdio_write(struct mii_dev *mii_bus, int addr, int devad, int reg,
>
> ditto
>
> > +                        u16 val)
> > +{
> > +       struct udevice *dev = mii_bus->priv;
> > +
> > +       return mdio_get_ops(dev)->write(dev, addr, devad, reg, val);
> > +}
> > +
> > +static int dm_mdio_reset(struct mii_dev *mii_bus)
>
> ditto
>
> > +{
> > +       struct udevice *dev = mii_bus->priv;
> > +
> > +       if (mdio_get_ops(dev)->reset)
> > +               return mdio_get_ops(dev)->reset(dev);
> > +       else
> > +               return 0;
> > +}
> > +
> > +static int dm_mdio_post_probe(struct udevice *dev)
> > +{
> > +       struct mdio_perdev_priv *pdata = dev_get_uclass_priv(dev);
> > +
> > +       pdata->mii_bus = mdio_alloc();
> > +       pdata->mii_bus->read = dm_mdio_read;
> > +       pdata->mii_bus->write = dm_mdio_write;
> > +       pdata->mii_bus->reset = dm_mdio_reset;
> > +       pdata->mii_bus->priv = dev;
> > +       strncpy(pdata->mii_bus->name, dev->name, MDIO_NAME_LEN);
> > +
> > +       return mdio_register(pdata->mii_bus);
> > +}
> > +
> > +static int dm_mdio_pre_remove(struct udevice *dev)
> > +{
> > +       struct mdio_perdev_priv *pdata = dev_get_uclass_priv(dev);
> > +       struct mdio_ops *ops = mdio_get_ops(dev);
> > +
> > +       if (ops->reset)
> > +               ops->reset(dev);
> > +       mdio_free(pdata->mii_bus);
> > +
> > +       return 0;
> > +}
> > +
> > +struct phy_device *dm_mdio_phy_connect(struct udevice *dev, int addr,
> > +                                      struct udevice *ethdev,
> > +                                      phy_interface_t interface)
> > +{
> > +       struct mdio_perdev_priv *pdata = dev_get_uclass_priv(dev);
> > +
> > +       if (device_probe(dev))
> > +               return 0;
> > +
> > +       return phy_connect(pdata->mii_bus, addr, ethdev, interface);
> > +}
> > +
> > +UCLASS_DRIVER(mii) = {
>
> UCLASS_DRIVER(mdio)?
>
> > +       .id = UCLASS_MDIO,
> > +       .name = "mii",
>
> "mdio"?
>
> > +       .post_bind  = dm_mdio_post_bind,
> > +       .post_probe = dm_mdio_post_probe,
> > +       .pre_remove = dm_mdio_pre_remove,
> > +       .per_device_auto_alloc_size = sizeof(struct mdio_perdev_priv),
> > +};
> > --
>
> Regards,
> Bin
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot


More information about the U-Boot mailing list