Hi Bin,

On 6/1/2019 8:16 PM, Bin Meng wrote:
Hi Alex,

On Sat, Jun 1, 2019 at 12:27 AM Alex Marginean <alexm.ossl...@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/*)

OK, will look into it.


Signed-off-by: Alex Marginean <alexm.ossl...@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

Oops, I had this renamed and refactored and I seem to missed the
comments entirely.  And a few other things as you pointed out below.


+ *
+ * @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

Thanks for pointing these out, I'll send a v2 with the fixes.

+ *
+ * @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?

These names may be used in mdio command and I'm guessing the command
gets confused if there are spaces, I'll put a comment here.


+               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.

These are not exported through dm api, instead they are registered to
legacy mii API.  I should probably rename the function and drop dm_,
I'll add comments to these functions to so people don't get confused.


+{
+       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"?

Yes, you're right, it should be "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


Thank you, Bin!
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to