All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices
@ 2019-05-31 16:27 Alex Marginean
  2019-06-01 11:27 ` Joe Hershberger
                   ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Alex Marginean @ 2019-05-31 16:27 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Alex Marginean <alexm.osslist@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
+ *
+ * @mii_bus: Supporting MII legacy bus
+ * @priv_pdata: device specific platdata
+ */
+struct mdio_perdev_priv {
+	struct mii_dev *mii_bus;
+};
+
+/**
+ * struct mii_ops - MDIO bus operations
+ *
+ * @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
+ * 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, ' ')) {
+		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)
+{
+	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,
+			 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)
+{
+	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) = {
+	.id = UCLASS_MDIO,
+	.name = "mii",
+	.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),
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices
  2019-05-31 16:27 [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices Alex Marginean
@ 2019-06-01 11:27 ` Joe Hershberger
  2019-06-01 17:16 ` Bin Meng
  2019-06-03  9:47 ` [U-Boot] [PATCH 1/2 v2] " Alex Marginean
  2 siblings, 0 replies; 27+ messages in thread
From: Joe Hershberger @ 2019-06-01 11:27 UTC (permalink / raw)
  To: u-boot

On Fri, May 31, 2019 at 11:27 AM Alex Marginean <alexm.osslist@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.
>
> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>

Looks good!

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices
  2019-05-31 16:27 [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices Alex Marginean
  2019-06-01 11:27 ` Joe Hershberger
@ 2019-06-01 17:16 ` Bin Meng
  2019-06-01 18:41   ` Alex Marginean
  2019-06-01 18:42   ` Joe Hershberger
  2019-06-03  9:47 ` [U-Boot] [PATCH 1/2 v2] " Alex Marginean
  2 siblings, 2 replies; 27+ messages in thread
From: Bin Meng @ 2019-06-01 17:16 UTC (permalink / raw)
  To: u-boot

Hi Alex,

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices
  2019-06-01 17:16 ` Bin Meng
@ 2019-06-01 18:41   ` Alex Marginean
  2019-06-01 18:42   ` Joe Hershberger
  1 sibling, 0 replies; 27+ messages in thread
From: Alex Marginean @ 2019-06-01 18:41 UTC (permalink / raw)
  To: u-boot

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.osslist@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.osslist@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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices
  2019-06-01 17:16 ` Bin Meng
  2019-06-01 18:41   ` Alex Marginean
@ 2019-06-01 18:42   ` Joe Hershberger
  2019-06-01 18:44     ` Alex Marginean
  1 sibling, 1 reply; 27+ messages in thread
From: Joe Hershberger @ 2019-06-01 18:42 UTC (permalink / raw)
  To: u-boot

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@gmail.com> wrote:
>
> Hi Alex,
>
> On Sat, Jun 1, 2019 at 12:27 AM Alex Marginean <alexm.osslist@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@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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices
  2019-06-01 18:42   ` Joe Hershberger
@ 2019-06-01 18:44     ` Alex Marginean
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Marginean @ 2019-06-01 18:44 UTC (permalink / raw)
  To: u-boot

On 6/1/2019 9:42 PM, Joe Hershberger wrote:
> Hi Bin,
> 
> Thanks for the review. I shouldn't have tried to look at this while
> falling asleep in the airport. :(
> 
> -Joe

I guess I should have double checked the comments and the other things
too, sorry for that..
I'll send a v2 on Monday.

Thank you!
Alex

> 
> On Sat, Jun 1, 2019 at 12:16 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> Hi Alex,
>>
>> On Sat, Jun 1, 2019 at 12:27 AM Alex Marginean <alexm.osslist@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@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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 1/2 v2] net: introduce MDIO DM class for MDIO devices
  2019-05-31 16:27 [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices Alex Marginean
  2019-06-01 11:27 ` Joe Hershberger
  2019-06-01 17:16 ` Bin Meng
@ 2019-06-03  9:47 ` Alex Marginean
  2019-06-03  9:47   ` [U-Boot] [PATCH 2/2 v2] test: dm: add MDIO test Alex Marginean
                     ` (2 more replies)
  2 siblings, 3 replies; 27+ messages in thread
From: Alex Marginean @ 2019-06-03  9:47 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---

Changes in v2:
	- fixed several comments using wrong API names
	- dropped dm_ from names of internal functions that don't use udevice *
	- fixed UCLASS driver name
	- added missing mdio_unregister in dm_mdio_pre_remove
	- added a comment on why spaces in names aren't ok
	- added a comment on how static mdio_read/_write/_reset functions
	are used

 cmd/mdio.c             |   5 ++
 drivers/net/Kconfig    |  13 +++++
 include/dm/uclass-id.h |   1 +
 include/miiphy.h       |  49 ++++++++++++++++++
 net/Makefile           |   1 +
 net/mdio-uclass.c      | 115 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 184 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..e6dd441983 100644
--- a/include/miiphy.h
+++ b/include/miiphy.h
@@ -118,4 +118,53 @@ 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 mdio_perdev_priv - Per-device class data for MDIO DM
+ *
+ * @mii_bus: Supporting MII legacy bus
+ */
+struct mdio_perdev_priv {
+	struct mii_dev *mii_bus;
+};
+
+/**
+ * struct mdio_ops - MDIO bus operations
+ *
+ * @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)
+
+/**
+ * dm_mdio_probe_devices - Call probe on all MII devices, currently used for
+ * 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..36a404ff44
--- /dev/null
+++ b/net/mdio-uclass.c
@@ -0,0 +1,115 @@
+// 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)
+{
+	/*
+	 * MDIO command doesn't like spaces in names, don't allow them to keep
+	 * it happy
+	 */
+	if (strchr(dev->name, ' ')) {
+		debug("\nError: MDIO device name \"%s\" has a space!\n",
+		      dev->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * Following read/write/reset functions are registered with legacy MII code.
+ * These are called for PHY operations by upper layers and we further call the
+ * DM MDIO driver functions.
+ */
+static int mdio_read(struct mii_dev *mii_bus, int addr, int devad, int reg)
+{
+	struct udevice *dev = mii_bus->priv;
+
+	return mdio_get_ops(dev)->read(dev, addr, devad, reg);
+}
+
+static int mdio_write(struct mii_dev *mii_bus, int addr, int devad, int reg,
+		      u16 val)
+{
+	struct udevice *dev = mii_bus->priv;
+
+	return mdio_get_ops(dev)->write(dev, addr, devad, reg, val);
+}
+
+static int mdio_reset(struct mii_dev *mii_bus)
+{
+	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 = mdio_read;
+	pdata->mii_bus->write = mdio_write;
+	pdata->mii_bus->reset = 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_unregister(pdata->mii_bus);
+	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(mdio) = {
+	.id = UCLASS_MDIO,
+	.name = "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),
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 2/2 v2] test: dm: add MDIO test
  2019-06-03  9:47 ` [U-Boot] [PATCH 1/2 v2] " Alex Marginean
@ 2019-06-03  9:47   ` Alex Marginean
  2019-06-03 12:52     ` Bin Meng
  2019-06-03 12:52   ` [U-Boot] [PATCH 1/2 v2] net: introduce MDIO DM class for MDIO devices Bin Meng
  2019-06-03 16:10   ` [U-Boot] [PATCH 1/2 v3] " Alex Marginean
  2 siblings, 1 reply; 27+ messages in thread
From: Alex Marginean @ 2019-06-03  9:47 UTC (permalink / raw)
  To: u-boot

A very simple test for DM_MDIO, mimicks a register write/read through the
sandbox bus to a dummy PHY.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---

Changes in v2:
	- new patch, v1 didn't have a test included

 arch/sandbox/dts/test.dts  |  4 ++
 configs/sandbox_defconfig  |  2 +
 drivers/net/Kconfig        | 10 +++++
 drivers/net/Makefile       |  1 +
 drivers/net/mdio_sandbox.c | 92 ++++++++++++++++++++++++++++++++++++++
 test/dm/Makefile           |  1 +
 test/dm/mdio.c             | 48 ++++++++++++++++++++
 7 files changed, 158 insertions(+)
 create mode 100644 drivers/net/mdio_sandbox.c
 create mode 100644 test/dm/mdio.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 8b2d6451c6..70b7e4c275 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -799,6 +799,10 @@
 		dmas = <&dma 0>, <&dma 1>, <&dma 2>;
 		dma-names = "m2m", "tx0", "rx0";
 	};
+
+	mdio-test {
+		compatible = "sandbox,mdio_sandbox";
+	};
 };
 
 #include "sandbox_pmic.dtsi"
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 4877f1099a..2a00df9807 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -139,7 +139,9 @@ CONFIG_SPI_FLASH_SPANSION=y
 CONFIG_SPI_FLASH_STMICRO=y
 CONFIG_SPI_FLASH_SST=y
 CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_PHYLIB=y
 CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
 CONFIG_NVME=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6fba5a84dd..635f8d72c2 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -24,6 +24,16 @@ config DM_MDIO
 	  This is currently implemented in net/mdio-uclass.c
 	  Look in include/miiphy.h for details.
 
+config MDIO_SANDBOX
+	depends on DM_MDIO && SANDBOX
+	default y
+	bool "Sandbox: Mocked MDIO driver"
+	help
+	  This driver implements dummy read/write/reset MDIO functions mimicking
+	  a bus with a single PHY.
+
+	  This driver is used in for testing in test/dm/mdio.c
+
 menuconfig NETDEVICES
 	bool "Network device support"
 	depends on NET
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 8d02a37896..40038427db 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -77,3 +77,4 @@ obj-y += ti/
 obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
 obj-y += mscc_eswitch/
 obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
+obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
diff --git a/drivers/net/mdio_sandbox.c b/drivers/net/mdio_sandbox.c
new file mode 100644
index 0000000000..d55a7c4466
--- /dev/null
+++ b/drivers/net/mdio_sandbox.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019
+ * Alex Marginean, NXP
+ */
+
+#include <dm.h>
+#include <errno.h>
+#include <miiphy.h>
+
+#define SANDBOX_PHY_ADDR	5
+#define SANDBOX_PHY_REG		0
+
+struct mdio_sandbox_priv {
+	int enabled;
+	u16 reg;
+};
+
+static int mdio_sandbox_read(struct udevice *dev, int addr, int devad, int reg)
+{
+	struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+
+	if (!priv->enabled)
+		return -ENODEV;
+
+	if (addr != SANDBOX_PHY_ADDR)
+		return -ENODEV;
+	if (devad != MDIO_DEVAD_NONE)
+		return -ENODEV;
+	if (reg != SANDBOX_PHY_REG)
+		return -ENODEV;
+
+	return priv->reg;
+}
+
+static int mdio_sandbox_write(struct udevice *dev, int addr, int devad, int reg,
+			      u16 val)
+{
+	struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+
+	if (!priv->enabled)
+		return -ENODEV;
+
+	if (addr != SANDBOX_PHY_ADDR)
+		return -ENODEV;
+	if (devad != MDIO_DEVAD_NONE)
+		return -ENODEV;
+	if (reg != SANDBOX_PHY_REG)
+		return -ENODEV;
+
+	priv->reg = val;
+
+	return 0;
+}
+
+static int mdio_sandbox_reset(struct udevice *dev)
+{
+	struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+
+	priv->reg = 0;
+
+	return 0;
+}
+
+static const struct mdio_ops mdio_sandbox_ops = {
+	.read = mdio_sandbox_read,
+	.write = mdio_sandbox_write,
+	.reset = mdio_sandbox_reset,
+};
+
+int mdio_sandbox_probe(struct udevice *dev)
+{
+	struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+
+	priv->enabled = 1;
+
+	return 0;
+}
+
+static const struct udevice_id mdio_sandbox_ids[] = {
+	{ .compatible = "sandbox,mdio_sandbox" },
+	{ }
+};
+
+U_BOOT_DRIVER(mdio_sandbox) = {
+	.name		= "mdio_sandbox",
+	.id		= UCLASS_MDIO,
+	.of_match	= mdio_sandbox_ids,
+	.probe		= mdio_sandbox_probe,
+	.ops		= &mdio_sandbox_ops,
+	.priv_auto_alloc_size = sizeof(struct mdio_sandbox_priv),
+};
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 49857c5092..3f042e3ab4 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -60,4 +60,5 @@ obj-$(CONFIG_SOUND) += sound.o
 obj-$(CONFIG_TEE) += tee.o
 obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o
 obj-$(CONFIG_DMA) += dma.o
+obj-$(CONFIG_DM_MDIO) += mdio.o
 endif
diff --git a/test/dm/mdio.c b/test/dm/mdio.c
new file mode 100644
index 0000000000..aabb8c2d52
--- /dev/null
+++ b/test/dm/mdio.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019
+ * Alex Marginean, NXP
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/test.h>
+#include <misc.h>
+#include <test/ut.h>
+#include <miiphy.h>
+
+/* macros copied over from mdio_sandbox.c */
+#define SANDBOX_PHY_ADDR	5
+#define SANDBOX_PHY_REG		0
+
+#define TEST_REG_VALUE		0xabcd
+
+static int dm_test_mdio(struct unit_test_state *uts)
+{
+	struct uclass *uc;
+	struct udevice *dev;
+	struct mdio_ops *ops;
+	u16 reg;
+
+	ut_assertok(uclass_get(UCLASS_MDIO, &uc));
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_MDIO, "mdio-test", &dev));
+
+	ops = mdio_get_ops(dev);
+	ut_assertnonnull(ops);
+	ut_assertnonnull(ops->read);
+	ut_assertnonnull(ops->write);
+
+	ut_assertok(ops->write(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
+			       SANDBOX_PHY_REG, TEST_REG_VALUE));
+	reg = ops->read(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
+			SANDBOX_PHY_REG);
+	ut_asserteq(reg, TEST_REG_VALUE);
+
+	ut_assert(ops->read(dev, SANDBOX_PHY_ADDR + 1, MDIO_DEVAD_NONE,
+			    SANDBOX_PHY_REG) != 0);
+
+	return 0;
+}
+
+DM_TEST(dm_test_mdio, DM_TESTF_SCAN_FDT);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 1/2 v2] net: introduce MDIO DM class for MDIO devices
  2019-06-03  9:47 ` [U-Boot] [PATCH 1/2 v2] " Alex Marginean
  2019-06-03  9:47   ` [U-Boot] [PATCH 2/2 v2] test: dm: add MDIO test Alex Marginean
@ 2019-06-03 12:52   ` Bin Meng
  2019-06-03 16:10   ` [U-Boot] [PATCH 1/2 v3] " Alex Marginean
  2 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2019-06-03 12:52 UTC (permalink / raw)
  To: u-boot

On Mon, Jun 3, 2019 at 5:47 PM Alex Marginean <alexm.osslist@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.
>
> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
> ---
>
> Changes in v2:
>         - fixed several comments using wrong API names
>         - dropped dm_ from names of internal functions that don't use udevice *
>         - fixed UCLASS driver name
>         - added missing mdio_unregister in dm_mdio_pre_remove
>         - added a comment on why spaces in names aren't ok
>         - added a comment on how static mdio_read/_write/_reset functions
>         are used
>
>  cmd/mdio.c             |   5 ++
>  drivers/net/Kconfig    |  13 +++++
>  include/dm/uclass-id.h |   1 +
>  include/miiphy.h       |  49 ++++++++++++++++++
>  net/Makefile           |   1 +
>  net/mdio-uclass.c      | 115 +++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 184 insertions(+)
>  create mode 100644 net/mdio-uclass.c
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 2/2 v2] test: dm: add MDIO test
  2019-06-03  9:47   ` [U-Boot] [PATCH 2/2 v2] test: dm: add MDIO test Alex Marginean
@ 2019-06-03 12:52     ` Bin Meng
  2019-06-03 13:02       ` Alex Marginean
  0 siblings, 1 reply; 27+ messages in thread
From: Bin Meng @ 2019-06-03 12:52 UTC (permalink / raw)
  To: u-boot

Hi Alex,

On Mon, Jun 3, 2019 at 5:47 PM Alex Marginean <alexm.osslist@gmail.com> wrote:
>
> A very simple test for DM_MDIO, mimicks a register write/read through the
> sandbox bus to a dummy PHY.
>
> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
> ---
>
> Changes in v2:
>         - new patch, v1 didn't have a test included
>
>  arch/sandbox/dts/test.dts  |  4 ++
>  configs/sandbox_defconfig  |  2 +
>  drivers/net/Kconfig        | 10 +++++
>  drivers/net/Makefile       |  1 +
>  drivers/net/mdio_sandbox.c | 92 ++++++++++++++++++++++++++++++++++++++
>  test/dm/Makefile           |  1 +
>  test/dm/mdio.c             | 48 ++++++++++++++++++++
>  7 files changed, 158 insertions(+)
>  create mode 100644 drivers/net/mdio_sandbox.c
>  create mode 100644 test/dm/mdio.c
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>

However, please see some nits below.

> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 8b2d6451c6..70b7e4c275 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -799,6 +799,10 @@
>                 dmas = <&dma 0>, <&dma 1>, <&dma 2>;
>                 dma-names = "m2m", "tx0", "rx0";
>         };
> +
> +       mdio-test {
> +               compatible = "sandbox,mdio_sandbox";

nits: it reads better if we had "sandbox,mdio"

> +       };
>  };
>
>  #include "sandbox_pmic.dtsi"
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 4877f1099a..2a00df9807 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig

nits: I think we need update all sandbox defconfigs, or just update
arch/Kconfig and imply PHYLIB & DM_MDIO there.

> @@ -139,7 +139,9 @@ CONFIG_SPI_FLASH_SPANSION=y
>  CONFIG_SPI_FLASH_STMICRO=y
>  CONFIG_SPI_FLASH_SST=y
>  CONFIG_SPI_FLASH_WINBOND=y
> +CONFIG_PHYLIB=y
>  CONFIG_DM_ETH=y
> +CONFIG_DM_MDIO=y
>  CONFIG_NVME=y
>  CONFIG_PCI=y
>  CONFIG_DM_PCI=y
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 6fba5a84dd..635f8d72c2 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -24,6 +24,16 @@ config DM_MDIO
>           This is currently implemented in net/mdio-uclass.c
>           Look in include/miiphy.h for details.
>
> +config MDIO_SANDBOX
> +       depends on DM_MDIO && SANDBOX
> +       default y
> +       bool "Sandbox: Mocked MDIO driver"
> +       help
> +         This driver implements dummy read/write/reset MDIO functions mimicking
> +         a bus with a single PHY.
> +
> +         This driver is used in for testing in test/dm/mdio.c
> +
>  menuconfig NETDEVICES
>         bool "Network device support"
>         depends on NET
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 8d02a37896..40038427db 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -77,3 +77,4 @@ obj-y += ti/
>  obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
>  obj-y += mscc_eswitch/
>  obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
> +obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
> diff --git a/drivers/net/mdio_sandbox.c b/drivers/net/mdio_sandbox.c
> new file mode 100644
> index 0000000000..d55a7c4466
> --- /dev/null
> +++ b/drivers/net/mdio_sandbox.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2019
> + * Alex Marginean, NXP
> + */
> +
> +#include <dm.h>
> +#include <errno.h>
> +#include <miiphy.h>
> +
> +#define SANDBOX_PHY_ADDR       5
> +#define SANDBOX_PHY_REG                0
> +
> +struct mdio_sandbox_priv {
> +       int enabled;
> +       u16 reg;
> +};
> +
> +static int mdio_sandbox_read(struct udevice *dev, int addr, int devad, int reg)
> +{
> +       struct mdio_sandbox_priv *priv = dev_get_priv(dev);
> +
> +       if (!priv->enabled)
> +               return -ENODEV;
> +
> +       if (addr != SANDBOX_PHY_ADDR)
> +               return -ENODEV;
> +       if (devad != MDIO_DEVAD_NONE)
> +               return -ENODEV;
> +       if (reg != SANDBOX_PHY_REG)
> +               return -ENODEV;
> +
> +       return priv->reg;
> +}
> +
> +static int mdio_sandbox_write(struct udevice *dev, int addr, int devad, int reg,
> +                             u16 val)
> +{
> +       struct mdio_sandbox_priv *priv = dev_get_priv(dev);
> +
> +       if (!priv->enabled)
> +               return -ENODEV;
> +
> +       if (addr != SANDBOX_PHY_ADDR)
> +               return -ENODEV;
> +       if (devad != MDIO_DEVAD_NONE)
> +               return -ENODEV;
> +       if (reg != SANDBOX_PHY_REG)
> +               return -ENODEV;
> +
> +       priv->reg = val;
> +
> +       return 0;
> +}
> +
> +static int mdio_sandbox_reset(struct udevice *dev)
> +{
> +       struct mdio_sandbox_priv *priv = dev_get_priv(dev);
> +
> +       priv->reg = 0;
> +
> +       return 0;
> +}
> +
> +static const struct mdio_ops mdio_sandbox_ops = {
> +       .read = mdio_sandbox_read,
> +       .write = mdio_sandbox_write,
> +       .reset = mdio_sandbox_reset,
> +};
> +
> +int mdio_sandbox_probe(struct udevice *dev)

This should be static.

> +{
> +       struct mdio_sandbox_priv *priv = dev_get_priv(dev);
> +
> +       priv->enabled = 1;
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id mdio_sandbox_ids[] = {
> +       { .compatible = "sandbox,mdio_sandbox" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(mdio_sandbox) = {
> +       .name           = "mdio_sandbox",
> +       .id             = UCLASS_MDIO,
> +       .of_match       = mdio_sandbox_ids,
> +       .probe          = mdio_sandbox_probe,
> +       .ops            = &mdio_sandbox_ops,
> +       .priv_auto_alloc_size = sizeof(struct mdio_sandbox_priv),
> +};
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index 49857c5092..3f042e3ab4 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -60,4 +60,5 @@ obj-$(CONFIG_SOUND) += sound.o
>  obj-$(CONFIG_TEE) += tee.o
>  obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o
>  obj-$(CONFIG_DMA) += dma.o
> +obj-$(CONFIG_DM_MDIO) += mdio.o
>  endif
> diff --git a/test/dm/mdio.c b/test/dm/mdio.c
> new file mode 100644
> index 0000000000..aabb8c2d52
> --- /dev/null
> +++ b/test/dm/mdio.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2019
> + * Alex Marginean, NXP
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/test.h>
> +#include <misc.h>
> +#include <test/ut.h>
> +#include <miiphy.h>
> +
> +/* macros copied over from mdio_sandbox.c */
> +#define SANDBOX_PHY_ADDR       5
> +#define SANDBOX_PHY_REG                0
> +
> +#define TEST_REG_VALUE         0xabcd
> +
> +static int dm_test_mdio(struct unit_test_state *uts)
> +{
> +       struct uclass *uc;
> +       struct udevice *dev;
> +       struct mdio_ops *ops;
> +       u16 reg;
> +
> +       ut_assertok(uclass_get(UCLASS_MDIO, &uc));
> +
> +       ut_assertok(uclass_get_device_by_name(UCLASS_MDIO, "mdio-test", &dev));
> +
> +       ops = mdio_get_ops(dev);
> +       ut_assertnonnull(ops);
> +       ut_assertnonnull(ops->read);
> +       ut_assertnonnull(ops->write);
> +
> +       ut_assertok(ops->write(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
> +                              SANDBOX_PHY_REG, TEST_REG_VALUE));
> +       reg = ops->read(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
> +                       SANDBOX_PHY_REG);
> +       ut_asserteq(reg, TEST_REG_VALUE);
> +
> +       ut_assert(ops->read(dev, SANDBOX_PHY_ADDR + 1, MDIO_DEVAD_NONE,
> +                           SANDBOX_PHY_REG) != 0);

Please add a test for ops->reset.

> +
> +       return 0;
> +}
> +
> +DM_TEST(dm_test_mdio, DM_TESTF_SCAN_FDT);
> --

Regards,
Bin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 2/2 v2] test: dm: add MDIO test
  2019-06-03 12:52     ` Bin Meng
@ 2019-06-03 13:02       ` Alex Marginean
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Marginean @ 2019-06-03 13:02 UTC (permalink / raw)
  To: u-boot

Hi Bin,

On 6/3/2019 3:52 PM, Bin Meng wrote:
> Hi Alex,
> 
> On Mon, Jun 3, 2019 at 5:47 PM Alex Marginean <alexm.osslist@gmail.com> wrote:
>>
>> A very simple test for DM_MDIO, mimicks a register write/read through the
>> sandbox bus to a dummy PHY.
>>
>> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
>> ---
>>
>> Changes in v2:
>>          - new patch, v1 didn't have a test included
>>
>>   arch/sandbox/dts/test.dts  |  4 ++
>>   configs/sandbox_defconfig  |  2 +
>>   drivers/net/Kconfig        | 10 +++++
>>   drivers/net/Makefile       |  1 +
>>   drivers/net/mdio_sandbox.c | 92 ++++++++++++++++++++++++++++++++++++++
>>   test/dm/Makefile           |  1 +
>>   test/dm/mdio.c             | 48 ++++++++++++++++++++
>>   7 files changed, 158 insertions(+)
>>   create mode 100644 drivers/net/mdio_sandbox.c
>>   create mode 100644 test/dm/mdio.c
>>
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> 
> However, please see some nits below.

Thank you for the review, I'll send a v3 for these later today.
Alex

> 
>> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
>> index 8b2d6451c6..70b7e4c275 100644
>> --- a/arch/sandbox/dts/test.dts
>> +++ b/arch/sandbox/dts/test.dts
>> @@ -799,6 +799,10 @@
>>                  dmas = <&dma 0>, <&dma 1>, <&dma 2>;
>>                  dma-names = "m2m", "tx0", "rx0";
>>          };
>> +
>> +       mdio-test {
>> +               compatible = "sandbox,mdio_sandbox";
> 
> nits: it reads better if we had "sandbox,mdio"
> 
>> +       };
>>   };
>>
>>   #include "sandbox_pmic.dtsi"
>> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
>> index 4877f1099a..2a00df9807 100644
>> --- a/configs/sandbox_defconfig
>> +++ b/configs/sandbox_defconfig
> 
> nits: I think we need update all sandbox defconfigs, or just update
> arch/Kconfig and imply PHYLIB & DM_MDIO there.
> 
>> @@ -139,7 +139,9 @@ CONFIG_SPI_FLASH_SPANSION=y
>>   CONFIG_SPI_FLASH_STMICRO=y
>>   CONFIG_SPI_FLASH_SST=y
>>   CONFIG_SPI_FLASH_WINBOND=y
>> +CONFIG_PHYLIB=y
>>   CONFIG_DM_ETH=y
>> +CONFIG_DM_MDIO=y
>>   CONFIG_NVME=y
>>   CONFIG_PCI=y
>>   CONFIG_DM_PCI=y
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index 6fba5a84dd..635f8d72c2 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -24,6 +24,16 @@ config DM_MDIO
>>            This is currently implemented in net/mdio-uclass.c
>>            Look in include/miiphy.h for details.
>>
>> +config MDIO_SANDBOX
>> +       depends on DM_MDIO && SANDBOX
>> +       default y
>> +       bool "Sandbox: Mocked MDIO driver"
>> +       help
>> +         This driver implements dummy read/write/reset MDIO functions mimicking
>> +         a bus with a single PHY.
>> +
>> +         This driver is used in for testing in test/dm/mdio.c
>> +
>>   menuconfig NETDEVICES
>>          bool "Network device support"
>>          depends on NET
>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
>> index 8d02a37896..40038427db 100644
>> --- a/drivers/net/Makefile
>> +++ b/drivers/net/Makefile
>> @@ -77,3 +77,4 @@ obj-y += ti/
>>   obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
>>   obj-y += mscc_eswitch/
>>   obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
>> +obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
>> diff --git a/drivers/net/mdio_sandbox.c b/drivers/net/mdio_sandbox.c
>> new file mode 100644
>> index 0000000000..d55a7c4466
>> --- /dev/null
>> +++ b/drivers/net/mdio_sandbox.c
>> @@ -0,0 +1,92 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2019
>> + * Alex Marginean, NXP
>> + */
>> +
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <miiphy.h>
>> +
>> +#define SANDBOX_PHY_ADDR       5
>> +#define SANDBOX_PHY_REG                0
>> +
>> +struct mdio_sandbox_priv {
>> +       int enabled;
>> +       u16 reg;
>> +};
>> +
>> +static int mdio_sandbox_read(struct udevice *dev, int addr, int devad, int reg)
>> +{
>> +       struct mdio_sandbox_priv *priv = dev_get_priv(dev);
>> +
>> +       if (!priv->enabled)
>> +               return -ENODEV;
>> +
>> +       if (addr != SANDBOX_PHY_ADDR)
>> +               return -ENODEV;
>> +       if (devad != MDIO_DEVAD_NONE)
>> +               return -ENODEV;
>> +       if (reg != SANDBOX_PHY_REG)
>> +               return -ENODEV;
>> +
>> +       return priv->reg;
>> +}
>> +
>> +static int mdio_sandbox_write(struct udevice *dev, int addr, int devad, int reg,
>> +                             u16 val)
>> +{
>> +       struct mdio_sandbox_priv *priv = dev_get_priv(dev);
>> +
>> +       if (!priv->enabled)
>> +               return -ENODEV;
>> +
>> +       if (addr != SANDBOX_PHY_ADDR)
>> +               return -ENODEV;
>> +       if (devad != MDIO_DEVAD_NONE)
>> +               return -ENODEV;
>> +       if (reg != SANDBOX_PHY_REG)
>> +               return -ENODEV;
>> +
>> +       priv->reg = val;
>> +
>> +       return 0;
>> +}
>> +
>> +static int mdio_sandbox_reset(struct udevice *dev)
>> +{
>> +       struct mdio_sandbox_priv *priv = dev_get_priv(dev);
>> +
>> +       priv->reg = 0;
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct mdio_ops mdio_sandbox_ops = {
>> +       .read = mdio_sandbox_read,
>> +       .write = mdio_sandbox_write,
>> +       .reset = mdio_sandbox_reset,
>> +};
>> +
>> +int mdio_sandbox_probe(struct udevice *dev)
> 
> This should be static.
> 
>> +{
>> +       struct mdio_sandbox_priv *priv = dev_get_priv(dev);
>> +
>> +       priv->enabled = 1;
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct udevice_id mdio_sandbox_ids[] = {
>> +       { .compatible = "sandbox,mdio_sandbox" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(mdio_sandbox) = {
>> +       .name           = "mdio_sandbox",
>> +       .id             = UCLASS_MDIO,
>> +       .of_match       = mdio_sandbox_ids,
>> +       .probe          = mdio_sandbox_probe,
>> +       .ops            = &mdio_sandbox_ops,
>> +       .priv_auto_alloc_size = sizeof(struct mdio_sandbox_priv),
>> +};
>> diff --git a/test/dm/Makefile b/test/dm/Makefile
>> index 49857c5092..3f042e3ab4 100644
>> --- a/test/dm/Makefile
>> +++ b/test/dm/Makefile
>> @@ -60,4 +60,5 @@ obj-$(CONFIG_SOUND) += sound.o
>>   obj-$(CONFIG_TEE) += tee.o
>>   obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o
>>   obj-$(CONFIG_DMA) += dma.o
>> +obj-$(CONFIG_DM_MDIO) += mdio.o
>>   endif
>> diff --git a/test/dm/mdio.c b/test/dm/mdio.c
>> new file mode 100644
>> index 0000000000..aabb8c2d52
>> --- /dev/null
>> +++ b/test/dm/mdio.c
>> @@ -0,0 +1,48 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2019
>> + * Alex Marginean, NXP
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <dm/test.h>
>> +#include <misc.h>
>> +#include <test/ut.h>
>> +#include <miiphy.h>
>> +
>> +/* macros copied over from mdio_sandbox.c */
>> +#define SANDBOX_PHY_ADDR       5
>> +#define SANDBOX_PHY_REG                0
>> +
>> +#define TEST_REG_VALUE         0xabcd
>> +
>> +static int dm_test_mdio(struct unit_test_state *uts)
>> +{
>> +       struct uclass *uc;
>> +       struct udevice *dev;
>> +       struct mdio_ops *ops;
>> +       u16 reg;
>> +
>> +       ut_assertok(uclass_get(UCLASS_MDIO, &uc));
>> +
>> +       ut_assertok(uclass_get_device_by_name(UCLASS_MDIO, "mdio-test", &dev));
>> +
>> +       ops = mdio_get_ops(dev);
>> +       ut_assertnonnull(ops);
>> +       ut_assertnonnull(ops->read);
>> +       ut_assertnonnull(ops->write);
>> +
>> +       ut_assertok(ops->write(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
>> +                              SANDBOX_PHY_REG, TEST_REG_VALUE));
>> +       reg = ops->read(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
>> +                       SANDBOX_PHY_REG);
>> +       ut_asserteq(reg, TEST_REG_VALUE);
>> +
>> +       ut_assert(ops->read(dev, SANDBOX_PHY_ADDR + 1, MDIO_DEVAD_NONE,
>> +                           SANDBOX_PHY_REG) != 0);
> 
> Please add a test for ops->reset.
> 
>> +
>> +       return 0;
>> +}
>> +
>> +DM_TEST(dm_test_mdio, DM_TESTF_SCAN_FDT);
>> --
> 
> Regards,
> Bin
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-03  9:47 ` [U-Boot] [PATCH 1/2 v2] " Alex Marginean
  2019-06-03  9:47   ` [U-Boot] [PATCH 2/2 v2] test: dm: add MDIO test Alex Marginean
  2019-06-03 12:52   ` [U-Boot] [PATCH 1/2 v2] net: introduce MDIO DM class for MDIO devices Bin Meng
@ 2019-06-03 16:10   ` Alex Marginean
  2019-06-03 16:12     ` [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test Alex Marginean
                       ` (4 more replies)
  2 siblings, 5 replies; 27+ messages in thread
From: Alex Marginean @ 2019-06-03 16:10 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---

Changes in v2:
	- fixed several comments using wrong API names
	- dropped dm_ from names of internal functions that don't use udevice *
	- fixed UCLASS driver name
	- added missing mdio_unregister in dm_mdio_pre_remove
	- added a comment on why spaces in names aren't ok
	- added a comment on how static mdio_read/_write/_reset functions
	are used
Changes in v3:
	- none

 cmd/mdio.c             |   5 ++
 drivers/net/Kconfig    |  13 +++++
 include/dm/uclass-id.h |   1 +
 include/miiphy.h       |  49 ++++++++++++++++++
 net/Makefile           |   1 +
 net/mdio-uclass.c      | 115 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 184 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..e6dd441983 100644
--- a/include/miiphy.h
+++ b/include/miiphy.h
@@ -118,4 +118,53 @@ 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 mdio_perdev_priv - Per-device class data for MDIO DM
+ *
+ * @mii_bus: Supporting MII legacy bus
+ */
+struct mdio_perdev_priv {
+	struct mii_dev *mii_bus;
+};
+
+/**
+ * struct mdio_ops - MDIO bus operations
+ *
+ * @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)
+
+/**
+ * dm_mdio_probe_devices - Call probe on all MII devices, currently used for
+ * 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..36a404ff44
--- /dev/null
+++ b/net/mdio-uclass.c
@@ -0,0 +1,115 @@
+// 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)
+{
+	/*
+	 * MDIO command doesn't like spaces in names, don't allow them to keep
+	 * it happy
+	 */
+	if (strchr(dev->name, ' ')) {
+		debug("\nError: MDIO device name \"%s\" has a space!\n",
+		      dev->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * Following read/write/reset functions are registered with legacy MII code.
+ * These are called for PHY operations by upper layers and we further call the
+ * DM MDIO driver functions.
+ */
+static int mdio_read(struct mii_dev *mii_bus, int addr, int devad, int reg)
+{
+	struct udevice *dev = mii_bus->priv;
+
+	return mdio_get_ops(dev)->read(dev, addr, devad, reg);
+}
+
+static int mdio_write(struct mii_dev *mii_bus, int addr, int devad, int reg,
+		      u16 val)
+{
+	struct udevice *dev = mii_bus->priv;
+
+	return mdio_get_ops(dev)->write(dev, addr, devad, reg, val);
+}
+
+static int mdio_reset(struct mii_dev *mii_bus)
+{
+	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 = mdio_read;
+	pdata->mii_bus->write = mdio_write;
+	pdata->mii_bus->reset = 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_unregister(pdata->mii_bus);
+	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(mdio) = {
+	.id = UCLASS_MDIO,
+	.name = "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),
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test
  2019-06-03 16:10   ` [U-Boot] [PATCH 1/2 v3] " Alex Marginean
@ 2019-06-03 16:12     ` Alex Marginean
  2019-06-04  2:20       ` Bin Meng
                         ` (2 more replies)
  2019-06-04  2:18     ` [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices Bin Meng
                       ` (3 subsequent siblings)
  4 siblings, 3 replies; 27+ messages in thread
From: Alex Marginean @ 2019-06-03 16:12 UTC (permalink / raw)
  To: u-boot

A very simple test for DM_MDIO, mimicks a register write/read through the
sandbox bus to a dummy PHY.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---

Changes in v2:
	- new patch, v1 didn't have a test included
Changes in v3:
	- DM_MDIO and PHYLIB are now implied in arc/Kconfig
	- added basic test for mdio reset op
	- simplified compatible, static probe function

 arch/Kconfig               |  2 +
 arch/sandbox/dts/test.dts  |  4 ++
 drivers/net/Kconfig        | 10 +++++
 drivers/net/Makefile       |  1 +
 drivers/net/mdio_sandbox.c | 92 ++++++++++++++++++++++++++++++++++++++
 test/dm/Makefile           |  1 +
 test/dm/mdio.c             | 53 ++++++++++++++++++++++
 7 files changed, 163 insertions(+)
 create mode 100644 drivers/net/mdio_sandbox.c
 create mode 100644 test/dm/mdio.c

diff --git a/arch/Kconfig b/arch/Kconfig
index e574b0d441..1e62a7615d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -120,6 +120,8 @@ config SANDBOX
 	imply VIRTIO_NET
 	imply DM_SOUND
 	imply PCH
+	imply PHYLIB
+	imply DM_MDIO
 
 config SH
 	bool "SuperH architecture"
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 8b2d6451c6..46d8a56d0f 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -799,6 +799,10 @@
 		dmas = <&dma 0>, <&dma 1>, <&dma 2>;
 		dma-names = "m2m", "tx0", "rx0";
 	};
+
+	mdio-test {
+		compatible = "sandbox,mdio";
+	};
 };
 
 #include "sandbox_pmic.dtsi"
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6fba5a84dd..635f8d72c2 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -24,6 +24,16 @@ config DM_MDIO
 	  This is currently implemented in net/mdio-uclass.c
 	  Look in include/miiphy.h for details.
 
+config MDIO_SANDBOX
+	depends on DM_MDIO && SANDBOX
+	default y
+	bool "Sandbox: Mocked MDIO driver"
+	help
+	  This driver implements dummy read/write/reset MDIO functions mimicking
+	  a bus with a single PHY.
+
+	  This driver is used in for testing in test/dm/mdio.c
+
 menuconfig NETDEVICES
 	bool "Network device support"
 	depends on NET
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 8d02a37896..40038427db 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -77,3 +77,4 @@ obj-y += ti/
 obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
 obj-y += mscc_eswitch/
 obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
+obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
diff --git a/drivers/net/mdio_sandbox.c b/drivers/net/mdio_sandbox.c
new file mode 100644
index 0000000000..07515e078c
--- /dev/null
+++ b/drivers/net/mdio_sandbox.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019
+ * Alex Marginean, NXP
+ */
+
+#include <dm.h>
+#include <errno.h>
+#include <miiphy.h>
+
+#define SANDBOX_PHY_ADDR	5
+#define SANDBOX_PHY_REG		0
+
+struct mdio_sandbox_priv {
+	int enabled;
+	u16 reg;
+};
+
+static int mdio_sandbox_read(struct udevice *dev, int addr, int devad, int reg)
+{
+	struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+
+	if (!priv->enabled)
+		return -ENODEV;
+
+	if (addr != SANDBOX_PHY_ADDR)
+		return -ENODEV;
+	if (devad != MDIO_DEVAD_NONE)
+		return -ENODEV;
+	if (reg != SANDBOX_PHY_REG)
+		return -ENODEV;
+
+	return priv->reg;
+}
+
+static int mdio_sandbox_write(struct udevice *dev, int addr, int devad, int reg,
+			      u16 val)
+{
+	struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+
+	if (!priv->enabled)
+		return -ENODEV;
+
+	if (addr != SANDBOX_PHY_ADDR)
+		return -ENODEV;
+	if (devad != MDIO_DEVAD_NONE)
+		return -ENODEV;
+	if (reg != SANDBOX_PHY_REG)
+		return -ENODEV;
+
+	priv->reg = val;
+
+	return 0;
+}
+
+static int mdio_sandbox_reset(struct udevice *dev)
+{
+	struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+
+	priv->reg = 0;
+
+	return 0;
+}
+
+static const struct mdio_ops mdio_sandbox_ops = {
+	.read = mdio_sandbox_read,
+	.write = mdio_sandbox_write,
+	.reset = mdio_sandbox_reset,
+};
+
+static int mdio_sandbox_probe(struct udevice *dev)
+{
+	struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+
+	priv->enabled = 1;
+
+	return 0;
+}
+
+static const struct udevice_id mdio_sandbox_ids[] = {
+	{ .compatible = "sandbox,mdio" },
+	{ }
+};
+
+U_BOOT_DRIVER(mdio_sandbox) = {
+	.name		= "mdio_sandbox",
+	.id		= UCLASS_MDIO,
+	.of_match	= mdio_sandbox_ids,
+	.probe		= mdio_sandbox_probe,
+	.ops		= &mdio_sandbox_ops,
+	.priv_auto_alloc_size = sizeof(struct mdio_sandbox_priv),
+};
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 49857c5092..3f042e3ab4 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -60,4 +60,5 @@ obj-$(CONFIG_SOUND) += sound.o
 obj-$(CONFIG_TEE) += tee.o
 obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o
 obj-$(CONFIG_DMA) += dma.o
+obj-$(CONFIG_DM_MDIO) += mdio.o
 endif
diff --git a/test/dm/mdio.c b/test/dm/mdio.c
new file mode 100644
index 0000000000..5b66255f7d
--- /dev/null
+++ b/test/dm/mdio.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019
+ * Alex Marginean, NXP
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/test.h>
+#include <misc.h>
+#include <test/ut.h>
+#include <miiphy.h>
+
+/* macros copied over from mdio_sandbox.c */
+#define SANDBOX_PHY_ADDR	5
+#define SANDBOX_PHY_REG		0
+
+#define TEST_REG_VALUE		0xabcd
+
+static int dm_test_mdio(struct unit_test_state *uts)
+{
+	struct uclass *uc;
+	struct udevice *dev;
+	struct mdio_ops *ops;
+	u16 reg;
+
+	ut_assertok(uclass_get(UCLASS_MDIO, &uc));
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_MDIO, "mdio-test", &dev));
+
+	ops = mdio_get_ops(dev);
+	ut_assertnonnull(ops);
+	ut_assertnonnull(ops->read);
+	ut_assertnonnull(ops->write);
+
+	ut_assertok(ops->write(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
+			       SANDBOX_PHY_REG, TEST_REG_VALUE));
+	reg = ops->read(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
+			SANDBOX_PHY_REG);
+	ut_asserteq(reg, TEST_REG_VALUE);
+
+	ut_assert(ops->read(dev, SANDBOX_PHY_ADDR + 1, MDIO_DEVAD_NONE,
+			    SANDBOX_PHY_REG) != 0);
+
+	ut_assertok(ops->reset(dev));
+	reg = ops->read(dev, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
+			SANDBOX_PHY_REG);
+	ut_asserteq(reg, 0);
+
+	return 0;
+}
+
+DM_TEST(dm_test_mdio, DM_TESTF_SCAN_FDT);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-03 16:10   ` [U-Boot] [PATCH 1/2 v3] " Alex Marginean
  2019-06-03 16:12     ` [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test Alex Marginean
@ 2019-06-04  2:18     ` Bin Meng
  2019-06-10 20:04     ` Joe Hershberger
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2019-06-04  2:18 UTC (permalink / raw)
  To: u-boot

Hi Alex,

On Tue, Jun 4, 2019 at 12:11 AM Alex Marginean <alexm.osslist@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.
>
> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
> ---
>
> Changes in v2:
>         - fixed several comments using wrong API names
>         - dropped dm_ from names of internal functions that don't use udevice *
>         - fixed UCLASS driver name
>         - added missing mdio_unregister in dm_mdio_pre_remove
>         - added a comment on why spaces in names aren't ok
>         - added a comment on how static mdio_read/_write/_reset functions
>         are used
> Changes in v3:
>         - none
>
>  cmd/mdio.c             |   5 ++
>  drivers/net/Kconfig    |  13 +++++
>  include/dm/uclass-id.h |   1 +
>  include/miiphy.h       |  49 ++++++++++++++++++
>  net/Makefile           |   1 +
>  net/mdio-uclass.c      | 115 +++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 184 insertions(+)
>  create mode 100644 net/mdio-uclass.c
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Please keep the RoB tag if there is nothing changed so that I know
this has been reviewed :)

Regards,
Bin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test
  2019-06-03 16:12     ` [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test Alex Marginean
@ 2019-06-04  2:20       ` Bin Meng
  2019-06-10 20:04       ` Joe Hershberger
  2019-07-15 22:51       ` [U-Boot] " Joe Hershberger
  2 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2019-06-04  2:20 UTC (permalink / raw)
  To: u-boot

On Tue, Jun 4, 2019 at 12:12 AM Alex Marginean <alexm.osslist@gmail.com> wrote:
>
> A very simple test for DM_MDIO, mimicks a register write/read through the
> sandbox bus to a dummy PHY.
>
> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
> ---
>
> Changes in v2:
>         - new patch, v1 didn't have a test included
> Changes in v3:
>         - DM_MDIO and PHYLIB are now implied in arc/Kconfig
>         - added basic test for mdio reset op
>         - simplified compatible, static probe function
>
>  arch/Kconfig               |  2 +
>  arch/sandbox/dts/test.dts  |  4 ++
>  drivers/net/Kconfig        | 10 +++++
>  drivers/net/Makefile       |  1 +
>  drivers/net/mdio_sandbox.c | 92 ++++++++++++++++++++++++++++++++++++++
>  test/dm/Makefile           |  1 +
>  test/dm/mdio.c             | 53 ++++++++++++++++++++++
>  7 files changed, 163 insertions(+)
>  create mode 100644 drivers/net/mdio_sandbox.c
>  create mode 100644 test/dm/mdio.c
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-03 16:10   ` [U-Boot] [PATCH 1/2 v3] " Alex Marginean
  2019-06-03 16:12     ` [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test Alex Marginean
  2019-06-04  2:18     ` [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices Bin Meng
@ 2019-06-10 20:04     ` Joe Hershberger
  2019-06-10 20:25     ` Joe Hershberger
  2019-07-15 22:51     ` [U-Boot] " Joe Hershberger
  4 siblings, 0 replies; 27+ messages in thread
From: Joe Hershberger @ 2019-06-10 20:04 UTC (permalink / raw)
  To: u-boot

On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
>
> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
> ---
>
> Changes in v2:
>         - fixed several comments using wrong API names
>         - dropped dm_ from names of internal functions that don't use udevice *
>         - fixed UCLASS driver name
>         - added missing mdio_unregister in dm_mdio_pre_remove
>         - added a comment on why spaces in names aren't ok
>         - added a comment on how static mdio_read/_write/_reset functions
>         are used
> Changes in v3:
>         - none
>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test
  2019-06-03 16:12     ` [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test Alex Marginean
  2019-06-04  2:20       ` Bin Meng
@ 2019-06-10 20:04       ` Joe Hershberger
  2019-07-15 22:51       ` [U-Boot] " Joe Hershberger
  2 siblings, 0 replies; 27+ messages in thread
From: Joe Hershberger @ 2019-06-10 20:04 UTC (permalink / raw)
  To: u-boot

On Mon, Jun 3, 2019 at 11:13 AM Alex Marginean <alexm.osslist@gmail.com> wrote:
>
> A very simple test for DM_MDIO, mimicks a register write/read through the
> sandbox bus to a dummy PHY.
>
> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-03 16:10   ` [U-Boot] [PATCH 1/2 v3] " Alex Marginean
                       ` (2 preceding siblings ...)
  2019-06-10 20:04     ` Joe Hershberger
@ 2019-06-10 20:25     ` Joe Hershberger
  2019-06-11  7:17       ` Alex Marginean
  2019-07-15 22:51     ` [U-Boot] " Joe Hershberger
  4 siblings, 1 reply; 27+ messages in thread
From: Joe Hershberger @ 2019-06-10 20:25 UTC (permalink / raw)
  To: u-boot

On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
>
> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
> ---
>
> Changes in v2:
>         - fixed several comments using wrong API names
>         - dropped dm_ from names of internal functions that don't use udevice *
>         - fixed UCLASS driver name
>         - added missing mdio_unregister in dm_mdio_pre_remove
>         - added a comment on why spaces in names aren't ok
>         - added a comment on how static mdio_read/_write/_reset functions
>         are used
> Changes in v3:
>         - none


Not sure if you already noticed this [1] or not, but there may be
something there that you want to incorporate or maybe not.

Cheers,
-Joe

[1] - https://patchwork.ozlabs.org/patch/939726/

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-10 20:25     ` Joe Hershberger
@ 2019-06-11  7:17       ` Alex Marginean
  2019-06-11  9:44         ` [U-Boot] [EXT] " Ken Ma
  0 siblings, 1 reply; 27+ messages in thread
From: Alex Marginean @ 2019-06-11  7:17 UTC (permalink / raw)
  To: u-boot

+Ken,

Hi Joe,

On 6/10/2019 11:25 PM, Joe Hershberger wrote:
> On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
>>
>> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
>> ---
>>
>> Changes in v2:
>>          - fixed several comments using wrong API names
>>          - dropped dm_ from names of internal functions that don't use udevice *
>>          - fixed UCLASS driver name
>>          - added missing mdio_unregister in dm_mdio_pre_remove
>>          - added a comment on why spaces in names aren't ok
>>          - added a comment on how static mdio_read/_write/_reset functions
>>          are used
>> Changes in v3:
>>          - none
> 
> 
> Not sure if you already noticed this [1] or not, but there may be
> something there that you want to incorporate or maybe not.
> 
> Cheers,
> -Joe
> 
> [1] - https://patchwork.ozlabs.org/patch/939726/
> 

I didn't notice it, thanks for pointing it out!
Apart from the obvious overlap of adding UCLASS_MDIO and code like
_post_probe they seem to deal with different needs.

Ken, can you please take a look at the patch I sent?  It has a
wrapper over phy_connect, but provides no helpers on how the caller
would get the PHY ADDR.  Do you want to try pulling the API you add on
top of the patch I sent, or do you want me to try?  It looks like it
would work with minimal effort.

Thank you!
Alex

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [EXT] Re: [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-11  7:17       ` Alex Marginean
@ 2019-06-11  9:44         ` Ken Ma
  2019-06-11 12:04           ` Alexandru Marginean
  0 siblings, 1 reply; 27+ messages in thread
From: Ken Ma @ 2019-06-11  9:44 UTC (permalink / raw)
  To: u-boot

Hi Alex

Thanks a lot for your information!

I think our patches have no essential difference.
The 2 patches have only small implementation difference:
	In my patch, mii bus ops functions(read/write/reset...) need to be implemented while in your patch mdio bus functions need to be implemented and then mii bus ops functions will call mdio bus ops functions.
	I had planned to reuse those existed mii ops functions such as smc911x_miiphy_read/ smc911x_miiphy_write/ sun8i_mdio_read/ sun8i_mdio_write... then it is easy for turning old mdio driver to DM.

Now I am not working on u-boot, so I am sorry that I will not do the pulling work.

Yours,
Ken

-----Original Message-----
From: Alex Marginean <alexm.osslist@gmail.com> 
Sent: Tuesday, June 11, 2019 9:18 AM
To: joe.hershberger at ni.com; Ken Ma <make@marvell.com>
Cc: u-boot <u-boot@lists.denx.de>; Joseph Hershberger <joseph.hershberger@ni.com>
Subject: [EXT] Re: [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices

External Email

----------------------------------------------------------------------
+Ken,

Hi Joe,

On 6/10/2019 11:25 PM, Joe Hershberger wrote:
> On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
>>
>> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
>> ---
>>
>> Changes in v2:
>>          - fixed several comments using wrong API names
>>          - dropped dm_ from names of internal functions that don't use udevice *
>>          - fixed UCLASS driver name
>>          - added missing mdio_unregister in dm_mdio_pre_remove
>>          - added a comment on why spaces in names aren't ok
>>          - added a comment on how static mdio_read/_write/_reset functions
>>          are used
>> Changes in v3:
>>          - none
> 
> 
> Not sure if you already noticed this [1] or not, but there may be 
> something there that you want to incorporate or maybe not.
> 
> Cheers,
> -Joe
> 
> [1] - https://patchwork.ozlabs.org/patch/939726/
> 

I didn't notice it, thanks for pointing it out!
Apart from the obvious overlap of adding UCLASS_MDIO and code like _post_probe they seem to deal with different needs.

Ken, can you please take a look at the patch I sent?  It has a wrapper over phy_connect, but provides no helpers on how the caller would get the PHY ADDR.  Do you want to try pulling the API you add on top of the patch I sent, or do you want me to try?  It looks like it would work with minimal effort.

Thank you!
Alex

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [EXT] Re: [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-11  9:44         ` [U-Boot] [EXT] " Ken Ma
@ 2019-06-11 12:04           ` Alexandru Marginean
  2019-06-14 16:55             ` Nevo Hed
  0 siblings, 1 reply; 27+ messages in thread
From: Alexandru Marginean @ 2019-06-11 12:04 UTC (permalink / raw)
  To: u-boot

Hi Ken,

On 6/11/2019 12:44 PM, Ken Ma wrote:
> Hi Alex
> 
> Thanks a lot for your information!
> 
> I think our patches have no essential difference.
> The 2 patches have only small implementation difference:
> In my patch, mii bus ops functions(read/write/reset...) need to be
> implemented while in your patch mdio bus functions need to be
> implemented and then mii bus ops functions will call mdio bus ops
> functions. > I had planned to reuse those existed mii ops functions such as
> smc911x_miiphy_read/ smc911x_miiphy_write/ sun8i_mdio_read/
> sun8i_mdio_write... then it is easy for turning old mdio driver to
> DM. >
> Now I am not working on u-boot, so I am sorry that I will not do the pulling work.
> 
> Yours,
> Ken

OK, I think I get what you wanted to do.  Either way it's not too
difficult to convert existing MDIOs to DM, but they have to start using
struct udevice.  That's similar to what was done on DM_ETH and others.

The helpers mapping eth/phy/mdio make sense and could be useful, that's
something I'll try to look into.

Thank you!
Alex


> 
> -----Original Message-----
> From: Alex Marginean <alexm.osslist@gmail.com>
> Sent: Tuesday, June 11, 2019 9:18 AM
> To: joe.hershberger at ni.com; Ken Ma <make@marvell.com>
> Cc: u-boot <u-boot@lists.denx.de>; Joseph Hershberger <joseph.hershberger@ni.com>
> Subject: [EXT] Re: [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
> 
> External Email
> 
> ----------------------------------------------------------------------
> +Ken,
> 
> Hi Joe,
> 
> On 6/10/2019 11:25 PM, Joe Hershberger wrote:
>> On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
>>>
>>> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
>>> ---
>>>
>>> Changes in v2:
>>>           - fixed several comments using wrong API names
>>>           - dropped dm_ from names of internal functions that don't use udevice *
>>>           - fixed UCLASS driver name
>>>           - added missing mdio_unregister in dm_mdio_pre_remove
>>>           - added a comment on why spaces in names aren't ok
>>>           - added a comment on how static mdio_read/_write/_reset functions
>>>           are used
>>> Changes in v3:
>>>           - none
>>
>>
>> Not sure if you already noticed this [1] or not, but there may be
>> something there that you want to incorporate or maybe not.
>>
>> Cheers,
>> -Joe
>>
>> [1] - https://patchwork.ozlabs.org/patch/939726/
>>
> 
> I didn't notice it, thanks for pointing it out!
> Apart from the obvious overlap of adding UCLASS_MDIO and code like _post_probe they seem to deal with different needs.
> 
> Ken, can you please take a look at the patch I sent?  It has a wrapper over phy_connect, but provides no helpers on how the caller would get the PHY ADDR.  Do you want to try pulling the API you add on top of the patch I sent, or do you want me to try?  It looks like it would work with minimal effort.
> 
> Thank you!
> Alex
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [EXT] Re: [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-11 12:04           ` Alexandru Marginean
@ 2019-06-14 16:55             ` Nevo Hed
  2019-06-14 18:26               ` Alex Marginean
  0 siblings, 1 reply; 27+ messages in thread
From: Nevo Hed @ 2019-06-14 16:55 UTC (permalink / raw)
  To: u-boot

Hi Alex

In another thread (https://lists.denx.de/pipermail/u-boot/2019-June/371933.html)
I asked Ken (before learning of their cut-backs) if I should take a
stab at re-integrating his work
where Joe also pointed out the impending acceptance of your work.

I'm not sure where we left off here in this thread and wondering if
you are working on that or not.  Ive been staring at this for a bit
and ready to take a stab at it if you are not.

if easier - i'm `nhed73` on the #u-boot irc channel

Thanks
  ---Nevo

On Tue, Jun 11, 2019 at 8:04 AM Alexandru Marginean
<alexandru.marginean@nxp.com> wrote:
>
> Hi Ken,
>
> On 6/11/2019 12:44 PM, Ken Ma wrote:
> > Hi Alex
> >
> > Thanks a lot for your information!
> >
> > I think our patches have no essential difference.
> > The 2 patches have only small implementation difference:
> > In my patch, mii bus ops functions(read/write/reset...) need to be
> > implemented while in your patch mdio bus functions need to be
> > implemented and then mii bus ops functions will call mdio bus ops
> > functions. > I had planned to reuse those existed mii ops functions such as
> > smc911x_miiphy_read/ smc911x_miiphy_write/ sun8i_mdio_read/
> > sun8i_mdio_write... then it is easy for turning old mdio driver to
> > DM. >
> > Now I am not working on u-boot, so I am sorry that I will not do the pulling work.
> >
> > Yours,
> > Ken
>
> OK, I think I get what you wanted to do.  Either way it's not too
> difficult to convert existing MDIOs to DM, but they have to start using
> struct udevice.  That's similar to what was done on DM_ETH and others.
>
> The helpers mapping eth/phy/mdio make sense and could be useful, that's
> something I'll try to look into.
>
> Thank you!
> Alex
>
>
> >
> > -----Original Message-----
> > From: Alex Marginean <alexm.osslist@gmail.com>
> > Sent: Tuesday, June 11, 2019 9:18 AM
> > To: joe.hershberger at ni.com; Ken Ma <make@marvell.com>
> > Cc: u-boot <u-boot@lists.denx.de>; Joseph Hershberger <joseph.hershberger@ni.com>
> > Subject: [EXT] Re: [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > +Ken,
> >
> > Hi Joe,
> >
> > On 6/10/2019 11:25 PM, Joe Hershberger wrote:
> >> On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
> >>>
> >>> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
> >>> ---
> >>>
> >>> Changes in v2:
> >>>           - fixed several comments using wrong API names
> >>>           - dropped dm_ from names of internal functions that don't use udevice *
> >>>           - fixed UCLASS driver name
> >>>           - added missing mdio_unregister in dm_mdio_pre_remove
> >>>           - added a comment on why spaces in names aren't ok
> >>>           - added a comment on how static mdio_read/_write/_reset functions
> >>>           are used
> >>> Changes in v3:
> >>>           - none
> >>
> >>
> >> Not sure if you already noticed this [1] or not, but there may be
> >> something there that you want to incorporate or maybe not.
> >>
> >> Cheers,
> >> -Joe
> >>
> >> [1] - https://patchwork.ozlabs.org/patch/939726/
> >>
> >
> > I didn't notice it, thanks for pointing it out!
> > Apart from the obvious overlap of adding UCLASS_MDIO and code like _post_probe they seem to deal with different needs.
> >
> > Ken, can you please take a look at the patch I sent?  It has a wrapper over phy_connect, but provides no helpers on how the caller would get the PHY ADDR.  Do you want to try pulling the API you add on top of the patch I sent, or do you want me to try?  It looks like it would work with minimal effort.
> >
> > Thank you!
> > Alex
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [EXT] Re: [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-14 16:55             ` Nevo Hed
@ 2019-06-14 18:26               ` Alex Marginean
  2019-07-08 21:40                 ` Joe Hershberger
  0 siblings, 1 reply; 27+ messages in thread
From: Alex Marginean @ 2019-06-14 18:26 UTC (permalink / raw)
  To: u-boot

Hi Nevo,

On 6/14/2019 7:55 PM, Nevo Hed wrote:
> Hi Alex
> 
> In another thread (https://lists.denx.de/pipermail/u-boot/2019-June/371933.html)
> I asked Ken (before learning of their cut-backs) if I should take a
> stab at re-integrating his work
> where Joe also pointed out the impending acceptance of your work.
> 
> I'm not sure where we left off here in this thread and wondering if
> you are working on that or not.  Ive been staring at this for a bit
> and ready to take a stab at it if you are not.

I am planning to add helpers to MDIO uclass, along the lines Ken sent
initially, on top of the current patch.  It looks like the helpers would
fit just fine.  I didn't get to do it yet though.

One difference between the two DM MDIO implementations is the way MDIO
ops work, the patch I sent abstracts away the legacy mii_bus from DM
MDIO driver.  I would keep that as it, which means a bit more rework on
the marvell driver.  I didn't look into this in detail yet.
I think I can send a version of the helpers in [1] in about a week,
maybe more, right now I'm trying to get MDIO MUX DM support out for
review plus some updates for the NXP platform I'm working on.  I can
take a stab at porting the marvell driver too, but I can't test it.
If you want to put some time into this right now feel free to do it :)

> 
> if easier - i'm `nhed73` on the #u-boot irc channel
> 
> Thanks
>    ---Nevo

I can join if you want to discuss details, let me know.

Thanks!
Alex


> 
> On Tue, Jun 11, 2019 at 8:04 AM Alexandru Marginean
> <alexandru.marginean@nxp.com> wrote:
>>
>> Hi Ken,
>>
>> On 6/11/2019 12:44 PM, Ken Ma wrote:
>>> Hi Alex
>>>
>>> Thanks a lot for your information!
>>>
>>> I think our patches have no essential difference.
>>> The 2 patches have only small implementation difference:
>>> In my patch, mii bus ops functions(read/write/reset...) need to be
>>> implemented while in your patch mdio bus functions need to be
>>> implemented and then mii bus ops functions will call mdio bus ops
>>> functions. > I had planned to reuse those existed mii ops functions such as
>>> smc911x_miiphy_read/ smc911x_miiphy_write/ sun8i_mdio_read/
>>> sun8i_mdio_write... then it is easy for turning old mdio driver to
>>> DM. >
>>> Now I am not working on u-boot, so I am sorry that I will not do the pulling work.
>>>
>>> Yours,
>>> Ken
>>
>> OK, I think I get what you wanted to do.  Either way it's not too
>> difficult to convert existing MDIOs to DM, but they have to start using
>> struct udevice.  That's similar to what was done on DM_ETH and others.
>>
>> The helpers mapping eth/phy/mdio make sense and could be useful, that's
>> something I'll try to look into.
>>
>> Thank you!
>> Alex
>>
>>
>>>
>>> -----Original Message-----
>>> From: Alex Marginean <alexm.osslist@gmail.com>
>>> Sent: Tuesday, June 11, 2019 9:18 AM
>>> To: joe.hershberger at ni.com; Ken Ma <make@marvell.com>
>>> Cc: u-boot <u-boot@lists.denx.de>; Joseph Hershberger <joseph.hershberger@ni.com>
>>> Subject: [EXT] Re: [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
>>>
>>> External Email
>>>
>>> ----------------------------------------------------------------------
>>> +Ken,
>>>
>>> Hi Joe,
>>>
>>> On 6/10/2019 11:25 PM, Joe Hershberger wrote:
>>>> On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
>>>>>
>>>>> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>>            - fixed several comments using wrong API names
>>>>>            - dropped dm_ from names of internal functions that don't use udevice *
>>>>>            - fixed UCLASS driver name
>>>>>            - added missing mdio_unregister in dm_mdio_pre_remove
>>>>>            - added a comment on why spaces in names aren't ok
>>>>>            - added a comment on how static mdio_read/_write/_reset functions
>>>>>            are used
>>>>> Changes in v3:
>>>>>            - none
>>>>
>>>>
>>>> Not sure if you already noticed this [1] or not, but there may be
>>>> something there that you want to incorporate or maybe not.
>>>>
>>>> Cheers,
>>>> -Joe
>>>>
>>>> [1] - https://patchwork.ozlabs.org/patch/939726/
>>>>
>>>
>>> I didn't notice it, thanks for pointing it out!
>>> Apart from the obvious overlap of adding UCLASS_MDIO and code like _post_probe they seem to deal with different needs.
>>>
>>> Ken, can you please take a look at the patch I sent?  It has a wrapper over phy_connect, but provides no helpers on how the caller would get the PHY ADDR.  Do you want to try pulling the API you add on top of the patch I sent, or do you want me to try?  It looks like it would work with minimal effort.
>>>
>>> Thank you!
>>> Alex
>>>
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [EXT] Re: [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-06-14 18:26               ` Alex Marginean
@ 2019-07-08 21:40                 ` Joe Hershberger
  2019-07-08 22:17                   ` Alex Marginean
  0 siblings, 1 reply; 27+ messages in thread
From: Joe Hershberger @ 2019-07-08 21:40 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 14, 2019 at 1:26 PM Alex Marginean <alexm.osslist@gmail.com> wrote:
>
> Hi Nevo,
>
> On 6/14/2019 7:55 PM, Nevo Hed wrote:
> > Hi Alex
> >
> > In another thread (https://lists.denx.de/pipermail/u-boot/2019-June/371933.html)
> > I asked Ken (before learning of their cut-backs) if I should take a
> > stab at re-integrating his work
> > where Joe also pointed out the impending acceptance of your work.
> >
> > I'm not sure where we left off here in this thread and wondering if
> > you are working on that or not.  Ive been staring at this for a bit
> > and ready to take a stab at it if you are not.
>
> I am planning to add helpers to MDIO uclass, along the lines Ken sent
> initially, on top of the current patch.  It looks like the helpers would
> fit just fine.  I didn't get to do it yet though.

I'm intending to take this as is... I assume that fits with your plan for this.

Thanks,
-Joe

>
> One difference between the two DM MDIO implementations is the way MDIO
> ops work, the patch I sent abstracts away the legacy mii_bus from DM
> MDIO driver.  I would keep that as it, which means a bit more rework on
> the marvell driver.  I didn't look into this in detail yet.
> I think I can send a version of the helpers in [1] in about a week,
> maybe more, right now I'm trying to get MDIO MUX DM support out for
> review plus some updates for the NXP platform I'm working on.  I can
> take a stab at porting the marvell driver too, but I can't test it.
> If you want to put some time into this right now feel free to do it :)
>
> >
> > if easier - i'm `nhed73` on the #u-boot irc channel
> >
> > Thanks
> >    ---Nevo
>
> I can join if you want to discuss details, let me know.
>
> Thanks!
> Alex
>
>
> >
> > On Tue, Jun 11, 2019 at 8:04 AM Alexandru Marginean
> > <alexandru.marginean@nxp.com> wrote:
> >>
> >> Hi Ken,
> >>
> >> On 6/11/2019 12:44 PM, Ken Ma wrote:
> >>> Hi Alex
> >>>
> >>> Thanks a lot for your information!
> >>>
> >>> I think our patches have no essential difference.
> >>> The 2 patches have only small implementation difference:
> >>> In my patch, mii bus ops functions(read/write/reset...) need to be
> >>> implemented while in your patch mdio bus functions need to be
> >>> implemented and then mii bus ops functions will call mdio bus ops
> >>> functions. > I had planned to reuse those existed mii ops functions such as
> >>> smc911x_miiphy_read/ smc911x_miiphy_write/ sun8i_mdio_read/
> >>> sun8i_mdio_write... then it is easy for turning old mdio driver to
> >>> DM. >
> >>> Now I am not working on u-boot, so I am sorry that I will not do the pulling work.
> >>>
> >>> Yours,
> >>> Ken
> >>
> >> OK, I think I get what you wanted to do.  Either way it's not too
> >> difficult to convert existing MDIOs to DM, but they have to start using
> >> struct udevice.  That's similar to what was done on DM_ETH and others.
> >>
> >> The helpers mapping eth/phy/mdio make sense and could be useful, that's
> >> something I'll try to look into.
> >>
> >> Thank you!
> >> Alex
> >>
> >>
> >>>
> >>> -----Original Message-----
> >>> From: Alex Marginean <alexm.osslist@gmail.com>
> >>> Sent: Tuesday, June 11, 2019 9:18 AM
> >>> To: joe.hershberger at ni.com; Ken Ma <make@marvell.com>
> >>> Cc: u-boot <u-boot@lists.denx.de>; Joseph Hershberger <joseph.hershberger@ni.com>
> >>> Subject: [EXT] Re: [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
> >>>
> >>> External Email
> >>>
> >>> ----------------------------------------------------------------------
> >>> +Ken,
> >>>
> >>> Hi Joe,
> >>>
> >>> On 6/10/2019 11:25 PM, Joe Hershberger wrote:
> >>>> On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
> >>>>>
> >>>>> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
> >>>>> ---
> >>>>>
> >>>>> Changes in v2:
> >>>>>            - fixed several comments using wrong API names
> >>>>>            - dropped dm_ from names of internal functions that don't use udevice *
> >>>>>            - fixed UCLASS driver name
> >>>>>            - added missing mdio_unregister in dm_mdio_pre_remove
> >>>>>            - added a comment on why spaces in names aren't ok
> >>>>>            - added a comment on how static mdio_read/_write/_reset functions
> >>>>>            are used
> >>>>> Changes in v3:
> >>>>>            - none
> >>>>
> >>>>
> >>>> Not sure if you already noticed this [1] or not, but there may be
> >>>> something there that you want to incorporate or maybe not.
> >>>>
> >>>> Cheers,
> >>>> -Joe
> >>>>
> >>>> [1] - https://patchwork.ozlabs.org/patch/939726/
> >>>>
> >>>
> >>> I didn't notice it, thanks for pointing it out!
> >>> Apart from the obvious overlap of adding UCLASS_MDIO and code like _post_probe they seem to deal with different needs.
> >>>
> >>> Ken, can you please take a look at the patch I sent?  It has a wrapper over phy_connect, but provides no helpers on how the caller would get the PHY ADDR.  Do you want to try pulling the API you add on top of the patch I sent, or do you want me to try?  It looks like it would work with minimal effort.
> >>>
> >>> Thank you!
> >>> Alex
> >>>
> >>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot at lists.denx.de
> >> https://lists.denx.de/listinfo/u-boot
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [EXT] Re: [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
  2019-07-08 21:40                 ` Joe Hershberger
@ 2019-07-08 22:17                   ` Alex Marginean
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Marginean @ 2019-07-08 22:17 UTC (permalink / raw)
  To: u-boot

On 7/9/2019 12:40 AM, Joe Hershberger wrote:
> On Fri, Jun 14, 2019 at 1:26 PM Alex Marginean <alexm.osslist@gmail.com> wrote:
>>
>> Hi Nevo,
>>
>> On 6/14/2019 7:55 PM, Nevo Hed wrote:
>>> Hi Alex
>>>
>>> In another thread (https://lists.denx.de/pipermail/u-boot/2019-June/371933.html)
>>> I asked Ken (before learning of their cut-backs) if I should take a
>>> stab at re-integrating his work
>>> where Joe also pointed out the impending acceptance of your work.
>>>
>>> I'm not sure where we left off here in this thread and wondering if
>>> you are working on that or not.  Ive been staring at this for a bit
>>> and ready to take a stab at it if you are not.
>>
>> I am planning to add helpers to MDIO uclass, along the lines Ken sent
>> initially, on top of the current patch.  It looks like the helpers would
>> fit just fine.  I didn't get to do it yet though.
> 
> I'm intending to take this as is... I assume that fits with your plan for this.

Thanks Joe, it does fit with the plan so far.
Nevo and I started a conversation on the side on a respin of the Marvell
MDIO driver, I'll send that out when it's ready too.

Alex

> 
> Thanks,
> -Joe
> 
>>
>> One difference between the two DM MDIO implementations is the way MDIO
>> ops work, the patch I sent abstracts away the legacy mii_bus from DM
>> MDIO driver.  I would keep that as it, which means a bit more rework on
>> the marvell driver.  I didn't look into this in detail yet.
>> I think I can send a version of the helpers in [1] in about a week,
>> maybe more, right now I'm trying to get MDIO MUX DM support out for
>> review plus some updates for the NXP platform I'm working on.  I can
>> take a stab at porting the marvell driver too, but I can't test it.
>> If you want to put some time into this right now feel free to do it :)
>>
>>>
>>> if easier - i'm `nhed73` on the #u-boot irc channel
>>>
>>> Thanks
>>>     ---Nevo
>>
>> I can join if you want to discuss details, let me know.
>>
>> Thanks!
>> Alex
>>
>>
>>>
>>> On Tue, Jun 11, 2019 at 8:04 AM Alexandru Marginean
>>> <alexandru.marginean@nxp.com> wrote:
>>>>
>>>> Hi Ken,
>>>>
>>>> On 6/11/2019 12:44 PM, Ken Ma wrote:
>>>>> Hi Alex
>>>>>
>>>>> Thanks a lot for your information!
>>>>>
>>>>> I think our patches have no essential difference.
>>>>> The 2 patches have only small implementation difference:
>>>>> In my patch, mii bus ops functions(read/write/reset...) need to be
>>>>> implemented while in your patch mdio bus functions need to be
>>>>> implemented and then mii bus ops functions will call mdio bus ops
>>>>> functions. > I had planned to reuse those existed mii ops functions such as
>>>>> smc911x_miiphy_read/ smc911x_miiphy_write/ sun8i_mdio_read/
>>>>> sun8i_mdio_write... then it is easy for turning old mdio driver to
>>>>> DM. >
>>>>> Now I am not working on u-boot, so I am sorry that I will not do the pulling work.
>>>>>
>>>>> Yours,
>>>>> Ken
>>>>
>>>> OK, I think I get what you wanted to do.  Either way it's not too
>>>> difficult to convert existing MDIOs to DM, but they have to start using
>>>> struct udevice.  That's similar to what was done on DM_ETH and others.
>>>>
>>>> The helpers mapping eth/phy/mdio make sense and could be useful, that's
>>>> something I'll try to look into.
>>>>
>>>> Thank you!
>>>> Alex
>>>>
>>>>
>>>>>
>>>>> -----Original Message-----
>>>>> From: Alex Marginean <alexm.osslist@gmail.com>
>>>>> Sent: Tuesday, June 11, 2019 9:18 AM
>>>>> To: joe.hershberger at ni.com; Ken Ma <make@marvell.com>
>>>>> Cc: u-boot <u-boot@lists.denx.de>; Joseph Hershberger <joseph.hershberger@ni.com>
>>>>> Subject: [EXT] Re: [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
>>>>>
>>>>> External Email
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>> +Ken,
>>>>>
>>>>> Hi Joe,
>>>>>
>>>>> On 6/10/2019 11:25 PM, Joe Hershberger wrote:
>>>>>> On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean <alexm.osslist@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.
>>>>>>>
>>>>>>> Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v2:
>>>>>>>             - fixed several comments using wrong API names
>>>>>>>             - dropped dm_ from names of internal functions that don't use udevice *
>>>>>>>             - fixed UCLASS driver name
>>>>>>>             - added missing mdio_unregister in dm_mdio_pre_remove
>>>>>>>             - added a comment on why spaces in names aren't ok
>>>>>>>             - added a comment on how static mdio_read/_write/_reset functions
>>>>>>>             are used
>>>>>>> Changes in v3:
>>>>>>>             - none
>>>>>>
>>>>>>
>>>>>> Not sure if you already noticed this [1] or not, but there may be
>>>>>> something there that you want to incorporate or maybe not.
>>>>>>
>>>>>> Cheers,
>>>>>> -Joe
>>>>>>
>>>>>> [1] - https://patchwork.ozlabs.org/patch/939726/
>>>>>>
>>>>>
>>>>> I didn't notice it, thanks for pointing it out!
>>>>> Apart from the obvious overlap of adding UCLASS_MDIO and code like _post_probe they seem to deal with different needs.
>>>>>
>>>>> Ken, can you please take a look at the patch I sent?  It has a wrapper over phy_connect, but provides no helpers on how the caller would get the PHY ADDR.  Do you want to try pulling the API you add on top of the patch I sent, or do you want me to try?  It looks like it would work with minimal effort.
>>>>>
>>>>> Thank you!
>>>>> Alex
>>>>>
>>>>
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot at lists.denx.de
>>>> https://lists.denx.de/listinfo/u-boot
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> https://lists.denx.de/listinfo/u-boot
>>>
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] net: introduce MDIO DM class for MDIO devices
  2019-06-03 16:10   ` [U-Boot] [PATCH 1/2 v3] " Alex Marginean
                       ` (3 preceding siblings ...)
  2019-06-10 20:25     ` Joe Hershberger
@ 2019-07-15 22:51     ` Joe Hershberger
  4 siblings, 0 replies; 27+ messages in thread
From: Joe Hershberger @ 2019-07-15 22:51 UTC (permalink / raw)
  To: u-boot

Hi Alex,

https://patchwork.ozlabs.org/patch/1109368/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] test: dm: add MDIO test
  2019-06-03 16:12     ` [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test Alex Marginean
  2019-06-04  2:20       ` Bin Meng
  2019-06-10 20:04       ` Joe Hershberger
@ 2019-07-15 22:51       ` Joe Hershberger
  2 siblings, 0 replies; 27+ messages in thread
From: Joe Hershberger @ 2019-07-15 22:51 UTC (permalink / raw)
  To: u-boot

Hi Alex,

https://patchwork.ozlabs.org/patch/1109369/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2019-07-15 22:51 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-31 16:27 [U-Boot] [PATCH] net: introduce MDIO DM class for MDIO devices Alex Marginean
2019-06-01 11:27 ` Joe Hershberger
2019-06-01 17:16 ` Bin Meng
2019-06-01 18:41   ` Alex Marginean
2019-06-01 18:42   ` Joe Hershberger
2019-06-01 18:44     ` Alex Marginean
2019-06-03  9:47 ` [U-Boot] [PATCH 1/2 v2] " Alex Marginean
2019-06-03  9:47   ` [U-Boot] [PATCH 2/2 v2] test: dm: add MDIO test Alex Marginean
2019-06-03 12:52     ` Bin Meng
2019-06-03 13:02       ` Alex Marginean
2019-06-03 12:52   ` [U-Boot] [PATCH 1/2 v2] net: introduce MDIO DM class for MDIO devices Bin Meng
2019-06-03 16:10   ` [U-Boot] [PATCH 1/2 v3] " Alex Marginean
2019-06-03 16:12     ` [U-Boot] [PATCH 2/2 v3] test: dm: add MDIO test Alex Marginean
2019-06-04  2:20       ` Bin Meng
2019-06-10 20:04       ` Joe Hershberger
2019-07-15 22:51       ` [U-Boot] " Joe Hershberger
2019-06-04  2:18     ` [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices Bin Meng
2019-06-10 20:04     ` Joe Hershberger
2019-06-10 20:25     ` Joe Hershberger
2019-06-11  7:17       ` Alex Marginean
2019-06-11  9:44         ` [U-Boot] [EXT] " Ken Ma
2019-06-11 12:04           ` Alexandru Marginean
2019-06-14 16:55             ` Nevo Hed
2019-06-14 18:26               ` Alex Marginean
2019-07-08 21:40                 ` Joe Hershberger
2019-07-08 22:17                   ` Alex Marginean
2019-07-15 22:51     ` [U-Boot] " Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.