[PATCH v4 0/8] Add MV88E6xxx DSA driver and use on gwventana

[PATCH v4 0/8] Add MV88E6xxx DSA driver and use on gwventana

[Tim Harvey]

This series adds a DSA driver for the MV88E6xxx based on
drivers/net/phy/mv88e61xx and uses it in the gwventana_gw5904_defconfig.

The hope is that the other three boards that use the MV88E61xx driver
can move to this as well eventually so that we can remove the non-dm
driver and the 4 Kconfig options it requires.

The MV88E6xxx has an MDIO interface thus DM_MDIO must be used so support
for a UCLASS_MDIO driver is added to the fec_mxc ethernet driver in a
way that allows a fallback to the previous non DM_MDIO case as there are
many boards out there using this driver that define DM_MDIO but do not
have the required dt props for a DM_MDIO driver which would cause a
regression.

Additionally some other patches are here suggested by Vladimir:
 - ensure MDIO children are scanned on post-bind is needed
 - sanity check DSA driver required ops are present
 - allow DSA drivers to not require xmit/recv functions
 - remove unecessary xmit/recv functions from ksz9477 driver

v4:
 - use standard Linux internal MDIO dt structure
 - use PHY_FIXED_ID define
 - rename to mv88e6xxx
 - sort includes alphabetically
 - remove dsa term from function names
 - reduce indentation level and remove unecessary code in of probe_mdio function
 - rename pdev to mdev to represent mdio device

v3:
 - fix mdios node in dt
 - add Vladimir's rb tag's

v2:
 - added Ramon's rb tag's to first two patches
 - add patches for dsa-uclass to sanity check ops and make xmit/recv
   optional
 - fec: fix fallback for non conforming DM_MDIO dts
 - mv88e61xx:
  - rebase on v2022.07-rc2 (use ofnode_get_phy_node)
  - remove unused commented out fields from struct
  - remove unused PORT_MASK macro
  - remove phy from priv struct name
  - refactor code from original drivers/net/phy/mv88e61xx with
    suggestions from review to consolidate some functions
    into mv88e61xx_dsa_port_enable
  - remove unecessary skiping of disabling of CPU port
  - remove unecessary dev_set_parent_priv
  - remove unnecessary static init flag
  - replace debug with a dev_warn if switch device-id unsupported
  - remove unnecessary xmit/recv functions as we rely on the fact that
    only a single port is active instead of mangling packets

Tested on a Gateworks GW5904 which has a Marvell 88E6176 switch hanging
off the IMX6 FEC.

Best Regards,

Tim

Tim Harvey (8):
  net: mdio-uclass: scan for dm mdio children on post-bind
  net: dsa: move cpu port probe to dsa_post_probe
  net: dsa: ensure dsa driver has proper ops
  net: dsa: allow rcv() and xmit() to be optional
  net: ksz9477: remove unnecessary xmit and recv functions
  net: fec: add support for DM_MDIO
  net: add MV88E61xx DSA driver
  board: gw_ventana: enable MV88E61XX DSA support

 arch/arm/dts/imx6qdl-gw5904.dtsi        |  31 +
 board/gateworks/gw_ventana/gw_ventana.c |  50 +-
 configs/gwventana_gw5904_defconfig      |   8 +-
 drivers/net/Kconfig                     |   7 +
 drivers/net/Makefile                    |   1 +
 drivers/net/fec_mxc.c                   |  90 ++-
 drivers/net/ksz9477.c                   |  23 -
 drivers/net/mv88e6xxx.c                 | 826 ++++++++++++++++++++++++
 net/dsa-uclass.c                        |  57 +-
 net/mdio-uclass.c                       |   4 +
 10 files changed, 1025 insertions(+), 72 deletions(-)
 create mode 100644 drivers/net/mv88e6xxx.c

-- 
2.25.1

[PATCH v4 1/8] net: mdio-uclass: scan for dm mdio children on post-bind

[Tim Harvey]

If a DM_MDIO driver is used we need to scan the subnodes as well.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
---
v4:
 - no changes
v3:
 - no changes
v2:
 - added Ramon's rb tag
---
 net/mdio-uclass.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
index 4401492ca015..d80037d0ac71 100644
--- a/net/mdio-uclass.c
+++ b/net/mdio-uclass.c
@@ -49,7 +49,11 @@ static int dm_mdio_post_bind(struct udevice *dev)
 		return -EINVAL;
 	}
 
+#if CONFIG_IS_ENABLED(OF_REAL)
+	return dm_scan_fdt_dev(dev);
+#else
 	return 0;
+#endif
 }
 
 int dm_mdio_read(struct udevice *mdio_dev, int addr, int devad, int reg)
-- 
2.25.1

[PATCH v4 2/8] net: dsa: move cpu port probe to dsa_post_probe

[Tim Harvey]

In order to ensure that a DSA driver probe gets called before
dsa_ops->port_probe move the port_probe of the cpu_port to
a post-probe function.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v4:
 - no changes
v3:
 - added Vladimir's rb tag
v2:
 - added Ramon's rb tag
---
 net/dsa-uclass.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index 3bf4351c847d..286718818517 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -466,7 +466,6 @@ static int dsa_pre_probe(struct udevice *dev)
 {
 	struct dsa_pdata *pdata = dev_get_uclass_plat(dev);
 	struct dsa_priv *priv = dev_get_uclass_priv(dev);
-	struct dsa_ops *ops = dsa_get_ops(dev);
 	int err;
 
 	priv->num_ports = pdata->num_ports;
@@ -482,6 +481,15 @@ static int dsa_pre_probe(struct udevice *dev)
 	if (err)
 		return err;
 
+	return 0;
+}
+
+static int dsa_post_probe(struct udevice *dev)
+{
+	struct dsa_priv *priv = dev_get_uclass_priv(dev);
+	struct dsa_ops *ops = dsa_get_ops(dev);
+	int err;
+
 	/* Simulate a probing event for the CPU port */
 	if (ops->port_probe) {
 		err = ops->port_probe(dev, priv->cpu_port,
@@ -491,13 +499,14 @@ static int dsa_pre_probe(struct udevice *dev)
 	}
 
 	return 0;
-}
+};
 
 UCLASS_DRIVER(dsa) = {
 	.id = UCLASS_DSA,
 	.name = "dsa",
 	.post_bind = dsa_post_bind,
 	.pre_probe = dsa_pre_probe,
+	.post_probe = dsa_post_probe,
 	.per_device_auto = sizeof(struct dsa_priv),
 	.per_device_plat_auto = sizeof(struct dsa_pdata),
 	.per_child_plat_auto = sizeof(struct dsa_port_pdata),
-- 
2.25.1

[PATCH v4 3/8] net: dsa: ensure dsa driver has proper ops

[Tim Harvey]

Add a function to sanity check a dsa driver having proper ops.

Suggested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v4:
 - no changes
v3:
 - added Vladimir's rb tag
v2: new patch
---
 net/dsa-uclass.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index 286718818517..cb21de17432b 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -342,6 +342,19 @@ U_BOOT_DRIVER(dsa_port) = {
 	.plat_auto = sizeof(struct eth_pdata),
 };
 
+static int dsa_sanitize_ops(struct udevice *dev)
+{
+	struct dsa_ops *ops = dsa_get_ops(dev);
+
+	if ((!ops->xmit || !ops->rcv) &&
+	    (!ops->port_enable && !ops->port_disable)) {
+		dev_err(dev, "Packets cannot be steered to ports\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /*
  * This function mostly deals with pulling information out of the device tree
  * into the pdata structure.
@@ -358,6 +371,10 @@ static int dsa_post_bind(struct udevice *dev)
 	if (!ofnode_valid(node))
 		return -ENODEV;
 
+	err = dsa_sanitize_ops(dev);
+	if (err)
+		return err;
+
 	pdata->master_node = ofnode_null();
 
 	node = ofnode_find_subnode(node, "ports");
-- 
2.25.1

[PATCH v4 4/8] net: dsa: allow rcv() and xmit() to be optional

[Tim Harvey]

Allow rcv() and xmit() dsa driver ops to be optional in case a driver
does not care to mangle a packet as in U-Boot only one network port is
enabled at a time and thus no packet mangling is necessary.

Suggested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v4:
 - no changes
v3:
 - added Vladimir's rb tag
v2: new patch
---
 net/dsa-uclass.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index cb21de17432b..9d63cde6b154 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -131,16 +131,14 @@ static void dsa_port_stop(struct udevice *pdev)
  * We copy the frame to a stack buffer where we have reserved headroom and
  * tailroom space.  Headroom and tailroom are set to 0.
  */
-static int dsa_port_send(struct udevice *pdev, void *packet, int length)
+static int dsa_port_mangle_packet(struct udevice *pdev, void *packet, int length)
 {
+	struct dsa_port_pdata *port_pdata = dev_get_parent_plat(pdev);
 	struct udevice *dev = dev_get_parent(pdev);
 	struct dsa_priv *priv = dev_get_uclass_priv(dev);
 	int head = priv->headroom, tail = priv->tailroom;
-	struct udevice *master = dsa_get_master(dev);
 	struct dsa_ops *ops = dsa_get_ops(dev);
 	uchar dsa_packet_tmp[PKTSIZE_ALIGN];
-	struct dsa_port_pdata *port_pdata;
-	int err;
 
 	if (length + head + tail > PKTSIZE_ALIGN)
 		return -EINVAL;
@@ -152,10 +150,21 @@ static int dsa_port_send(struct udevice *pdev, void *packet, int length)
 	/* copy back to preserve original buffer alignment */
 	memcpy(packet, dsa_packet_tmp, length);
 
-	port_pdata = dev_get_parent_plat(pdev);
-	err = ops->xmit(dev, port_pdata->index, packet, length);
-	if (err)
-		return err;
+	return ops->xmit(dev, port_pdata->index, packet, length);
+}
+
+static int dsa_port_send(struct udevice *pdev, void *packet, int length)
+{
+	struct udevice *dev = dev_get_parent(pdev);
+	struct udevice *master = dsa_get_master(dev);
+	struct dsa_ops *ops = dsa_get_ops(dev);
+	int err;
+
+	if (ops->xmit) {
+		err = dsa_port_mangle_packet(pdev, packet, length);
+		if (err)
+			return err;
+	}
 
 	return eth_get_ops(master)->send(master, packet, length);
 }
@@ -172,7 +181,7 @@ static int dsa_port_recv(struct udevice *pdev, int flags, uchar **packetp)
 	int length, port_index, err;
 
 	length = eth_get_ops(master)->recv(master, flags, packetp);
-	if (length <= 0)
+	if (length <= 0 || !ops->rcv)
 		return length;
 
 	/*
-- 
2.25.1

[PATCH v4 5/8] net: ksz9477: remove unnecessary xmit and recv functions

[Tim Harvey]

Remove the unnecessary xmit and recv functions.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v4:
 - no changes
v3:
 - added Vladimir's rb tag
v2: new patch
---
 drivers/net/ksz9477.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/net/ksz9477.c b/drivers/net/ksz9477.c
index ed8f1895cb12..fb5c76c600be 100644
--- a/drivers/net/ksz9477.c
+++ b/drivers/net/ksz9477.c
@@ -62,7 +62,6 @@
 
 struct ksz_dsa_priv {
 	struct udevice *dev;
-	int active_port;
 };
 
 static inline int ksz_read8(struct udevice *dev, u32 reg, u8 *val)
@@ -382,9 +381,6 @@ static int ksz_port_enable(struct udevice *dev, int port, struct phy_device *phy
 	data8 |= SW_START;
 	ksz_write8(priv->dev, REG_SW_OPERATION, data8);
 
-	/* keep track of current enabled non-cpu port */
-	priv->active_port = port;
-
 	return 0;
 }
 
@@ -413,28 +409,9 @@ static void ksz_port_disable(struct udevice *dev, int port, struct phy_device *p
 	 */
 }
 
-static int ksz_xmit(struct udevice *dev, int port, void *packet, int length)
-{
-	dev_dbg(dev, "%s P%d %d\n", __func__, port + 1, length);
-
-	return 0;
-}
-
-static int ksz_recv(struct udevice *dev, int *port, void *packet, int length)
-{
-	struct ksz_dsa_priv *priv = dev_get_priv(dev);
-
-	dev_dbg(dev, "%s P%d %d\n", __func__, priv->active_port + 1, length);
-	*port = priv->active_port;
-
-	return 0;
-};
-
 static const struct dsa_ops ksz_dsa_ops = {
 	.port_enable = ksz_port_enable,
 	.port_disable = ksz_port_disable,
-	.xmit = ksz_xmit,
-	.rcv = ksz_recv,
 };
 
 static int ksz_probe_mdio(struct udevice *dev)
-- 
2.25.1

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

[Tim Harvey]

Add support for DM_MDIO by registering a UCLASS_MDIO driver and
attempting to use it. This is necessary if wanting to use a DSA
driver for example hanging off of the FEC MAC.

Care is taken to fallback to non DM_MDIO mii bus as several boards define
DM_MDIO without having the proper device-tree configuration necessary
such as an mdio subnode, a phy-mode prop, and either a valid phy-handle
prop or fixed-phy subnode which will cause dm_eth_phy_connect() to fail.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v4:
 - no changes
v3:
 - no changes
v2:
 - fix fallback mechanism for legacy dt's that do not have phy-handle
   and mdio subnode
---
 drivers/net/fec_mxc.c | 90 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 8bc2b46d403f..03be4b4ec282 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -30,6 +30,8 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/mach-imx/sys_proto.h>
 #include <asm-generic/gpio.h>
+#include <dm/device_compat.h>
+#include <dm/lists.h>
 
 #include "fec_mxc.h"
 #include <eth_phy.h>
@@ -1026,6 +1028,81 @@ struct mii_dev *fec_get_miibus(ulong base_addr, int dev_id)
 	return bus;
 }
 
+#ifdef CONFIG_DM_MDIO
+struct dm_fec_mdio_priv {
+	struct ethernet_regs *regs;
+};
+
+static int dm_fec_mdio_read(struct udevice *dev, int addr, int devad, int reg)
+{
+	struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
+
+	return fec_mdio_read(priv->regs, addr, reg);
+}
+
+static int dm_fec_mdio_write(struct udevice *dev, int addr, int devad, int reg, u16 data)
+{
+	struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
+
+	return fec_mdio_write(priv->regs, addr, reg, data);
+}
+
+static const struct mdio_ops dm_fec_mdio_ops = {
+	.read = dm_fec_mdio_read,
+	.write = dm_fec_mdio_write,
+};
+
+static int dm_fec_mdio_probe(struct udevice *dev)
+{
+	struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
+
+	priv->regs = (struct ethernet_regs *)ofnode_get_addr(dev_ofnode(dev->parent));
+
+	return 0;
+}
+
+U_BOOT_DRIVER(fec_mdio) = {
+	.name		= "fec_mdio",
+	.id		= UCLASS_MDIO,
+	.probe		= dm_fec_mdio_probe,
+	.ops		= &dm_fec_mdio_ops,
+	.priv_auto	= sizeof(struct dm_fec_mdio_priv),
+};
+
+static int dm_fec_bind_mdio(struct udevice *dev)
+{
+	struct udevice *mdiodev;
+	const char *name;
+	ofnode mdio;
+	int ret = -ENODEV;
+
+	/* for a UCLASS_MDIO driver we need to bind and probe manually
+	 * for an internal MDIO bus that has no dt compatible of its own
+	 */
+	ofnode_for_each_subnode(mdio, dev_ofnode(dev)) {
+		name = ofnode_get_name(mdio);
+
+		if (strcmp(name, "mdio"))
+			continue;
+
+		ret = device_bind_driver_to_node(dev, "fec_mdio",
+						 name, mdio, &mdiodev);
+		if (ret) {
+			printf("%s bind %s failed: %d\n", __func__, name, ret);
+			break;
+		}
+
+		/* need to probe it as there is no compatible to do so */
+		ret = uclass_get_device_by_ofnode(UCLASS_MDIO, mdio, &mdiodev);
+		if (!ret)
+			return 0;
+		printf("%s probe %s failed: %d\n", __func__, name, ret);
+	}
+
+	return ret;
+}
+#endif
+
 static int fecmxc_read_rom_hwaddr(struct udevice *dev)
 {
 	struct fec_priv *priv = dev_get_priv(dev);
@@ -1089,7 +1166,7 @@ static int device_get_phy_addr(struct fec_priv *priv, struct udevice *dev)
 
 static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
 {
-	struct phy_device *phydev;
+	struct phy_device *phydev = NULL;
 	int addr;
 
 	addr = device_get_phy_addr(priv, dev);
@@ -1097,7 +1174,10 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
 	addr = CONFIG_FEC_MXC_PHYADDR;
 #endif
 
-	phydev = phy_connect(priv->bus, addr, dev, priv->interface);
+	if (IS_ENABLED(CONFIG_DM_MDIO))
+		phydev = dm_eth_phy_connect(dev);
+	if (!phydev)
+		phydev = phy_connect(priv->bus, addr, dev, priv->interface);
 	if (!phydev)
 		return -ENODEV;
 
@@ -1228,6 +1308,12 @@ static int fecmxc_probe(struct udevice *dev)
 
 	priv->dev_id = dev_seq(dev);
 
+	if (IS_ENABLED(CONFIG_DM_MDIO)) {
+		ret = dm_fec_bind_mdio(dev);
+		if (ret && ret != -ENODEV)
+			return ret;
+	}
+
 #ifdef CONFIG_DM_ETH_PHY
 	bus = eth_phy_get_mdio_bus(dev);
 #endif
-- 
2.25.1

[PATCH v4 7/8] net: add MV88E61xx DSA driver

[Tim Harvey]

Add a DSA driver for the MV88E61xx compatible GbE Ethernet switches.

Cc: Marek Behún <marek.behun@nic.cz>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v4:
 - rename to mv88e6xxx
 - sort includes alphabetically
 - remove dsa term from function names
 - reduce indentation level and remove unecessary code in of probe_mdio function
 - rename pdev to mdev to represent mdio device
v3:
 - Added Vladimir's rb tag
v2:
 - rebase on v2022.07-rc1 (use ofnode_get_phy_node)
 - remove unused commented out fields from struct
 - remove unused PORT_MASK macro
 - remove phy from priv struct name
 - refactor code from original drivers/net/phy/mv88e61xx with
   suggestions from review to consolidate some functions
   into mv88e61xx_dsa_port_enable
 - remove unecessary skiping of disabling of CPU port
 - remove unecessary dev_set_parent_priv
 - remove unnecessary static init flag
 - replace debug with a dev_warn if switch device-id unsupported
 - remove unnecessary xmit/recv functions as we rely on the fact that
   only a single port is active instead of mangling packets
---
 drivers/net/Kconfig     |   7 +
 drivers/net/Makefile    |   1 +
 drivers/net/mv88e6xxx.c | 831 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 839 insertions(+)
 create mode 100644 drivers/net/mv88e6xxx.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6bbbadc5eef3..5508dacbcc49 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -438,6 +438,13 @@ config KSZ9477
 	  This driver implements a DSA switch driver for the KSZ9477 family
 	  of GbE switches using the I2C interface.
 
+config MV88E6XXX
+	bool "Marvell MV88E6xxx GbE switch DSA driver"
+	depends on DM_DSA && DM_MDIO
+	help
+	  This driver implements a DSA switch driver for the MV88E6xxx family
+	  of GbE switches using the MDIO interface
+
 config MVGBE
 	bool "Marvell Orion5x/Kirkwood network interface support"
 	depends on ARCH_KIRKWOOD || ARCH_ORION5X
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 96b7678e9882..f05fd58e3fa7 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
 obj-$(CONFIG_MPC8XX_FEC) += mpc8xx_fec.o
 obj-$(CONFIG_MT7620_ETH) += mt7620-eth.o
 obj-$(CONFIG_MT7628_ETH) += mt7628-eth.o
+obj-$(CONFIG_MV88E6XXX) += mv88e6xxx.o
 obj-$(CONFIG_MVGBE) += mvgbe.o
 obj-$(CONFIG_MVMDIO) += mvmdio.o
 obj-$(CONFIG_MVNETA) += mvneta.o
diff --git a/drivers/net/mv88e6xxx.c b/drivers/net/mv88e6xxx.c
new file mode 100644
index 000000000000..a3a3f6cf4acc
--- /dev/null
+++ b/drivers/net/mv88e6xxx.c
@@ -0,0 +1,831 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2022
+ * Gateworks Corporation <www.gateworks.com>
+ * Tim Harvey <tharvey@gateworks.com>
+ *
+ * (C) Copyright 2015
+ * Elecsys Corporation <www.elecsyscorp.com>
+ * Kevin Smith <kevin.smith@elecsyscorp.com>
+ *
+ * Original driver:
+ * (C) Copyright 2009
+ * Marvell Semiconductor <www.marvell.com>
+ * Prafulla Wadaskar <prafulla@marvell.com>
+ */
+
+/*
+ * DSA driver for mv88e6xxx ethernet switches.
+ *
+ * This driver configures the mv88e6xxx for basic use as a DSA switch.
+ *
+ * This driver was adapted from drivers/net/phy/mv88e61xx and tested
+ * on the mv88e6176 via an SGMII interface.
+ */
+
+#include <bitfield.h>
+#include <common.h>
+#include <dm/device.h>
+#include <dm/device_compat.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <dm/of_extra.h>
+#include <linux/delay.h>
+#include <miiphy.h>
+#include <net/dsa.h>
+
+/* Device addresses */
+#define DEVADDR_PHY(p)			(p)
+#define DEVADDR_SERDES			0x0F
+
+/* SMI indirection registers for multichip addressing mode */
+#define SMI_CMD_REG			0x00
+#define SMI_DATA_REG			0x01
+
+/* Global registers */
+#define GLOBAL1_STATUS			0x00
+#define GLOBAL1_CTRL			0x04
+#define GLOBAL1_MON_CTRL		0x1A
+
+/* Global 2 registers */
+#define GLOBAL2_REG_PHY_CMD		0x18
+#define GLOBAL2_REG_PHY_DATA		0x19
+#define GLOBAL2_REG_SCRATCH		0x1A
+
+/* Port registers */
+#define PORT_REG_STATUS			0x00
+#define PORT_REG_PHYS_CTRL		0x01
+#define PORT_REG_SWITCH_ID		0x03
+#define PORT_REG_CTRL			0x04
+#define PORT_REG_VLAN_MAP		0x06
+#define PORT_REG_VLAN_ID		0x07
+#define PORT_REG_LED_CTRL		0x16
+
+/* Phy registers */
+#define PHY_REG_CTRL1			0x10
+#define PHY_REG_STATUS1			0x11
+#define PHY_REG_PAGE			0x16
+
+/* Serdes registers */
+#define SERDES_REG_CTRL_1		0x10
+
+/* Phy page numbers */
+#define PHY_PAGE_COPPER			0
+#define PHY_PAGE_SERDES			1
+
+/* Register fields */
+#define GLOBAL1_CTRL_SWRESET		BIT(15)
+
+#define GLOBAL1_MON_CTRL_CPUDEST_SHIFT	4
+#define GLOBAL1_MON_CTRL_CPUDEST_WIDTH	4
+
+#define PORT_REG_STATUS_SPEED_SHIFT	8
+#define PORT_REG_STATUS_SPEED_10	0
+#define PORT_REG_STATUS_SPEED_100	1
+#define PORT_REG_STATUS_SPEED_1000	2
+
+#define PORT_REG_STATUS_CMODE_MASK		0xF
+#define PORT_REG_STATUS_CMODE_100BASE_X		0x8
+#define PORT_REG_STATUS_CMODE_1000BASE_X	0x9
+#define PORT_REG_STATUS_CMODE_SGMII		0xa
+
+#define PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK	BIT(15)
+#define PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK	BIT(14)
+#define PORT_REG_PHYS_CTRL_PCS_AN_EN	BIT(10)
+#define PORT_REG_PHYS_CTRL_PCS_AN_RST	BIT(9)
+#define PORT_REG_PHYS_CTRL_FC_VALUE	BIT(7)
+#define PORT_REG_PHYS_CTRL_FC_FORCE	BIT(6)
+#define PORT_REG_PHYS_CTRL_LINK_VALUE	BIT(5)
+#define PORT_REG_PHYS_CTRL_LINK_FORCE	BIT(4)
+#define PORT_REG_PHYS_CTRL_DUPLEX_VALUE	BIT(3)
+#define PORT_REG_PHYS_CTRL_DUPLEX_FORCE	BIT(2)
+#define PORT_REG_PHYS_CTRL_SPD1000	BIT(1)
+#define PORT_REG_PHYS_CTRL_SPD100	BIT(0)
+#define PORT_REG_PHYS_CTRL_SPD_MASK	(BIT(1) | BIT(0))
+
+#define PORT_REG_CTRL_PSTATE_SHIFT	0
+#define PORT_REG_CTRL_PSTATE_WIDTH	2
+
+#define PORT_REG_VLAN_ID_DEF_VID_SHIFT	0
+#define PORT_REG_VLAN_ID_DEF_VID_WIDTH	12
+
+#define PORT_REG_VLAN_MAP_TABLE_SHIFT	0
+#define PORT_REG_VLAN_MAP_TABLE_WIDTH	11
+
+#define SERDES_REG_CTRL_1_FORCE_LINK	BIT(10)
+
+/* Field values */
+#define PORT_REG_CTRL_PSTATE_DISABLED	0
+#define PORT_REG_CTRL_PSTATE_FORWARD	3
+
+#define PHY_REG_CTRL1_ENERGY_DET_OFF	0
+#define PHY_REG_CTRL1_ENERGY_DET_SENSE_PULSE	1
+#define PHY_REG_CTRL1_ENERGY_DET_SENSE_ONLY	2
+#define PHY_REG_CTRL1_ENERGY_DET_SENSE_XMIT	3
+
+/* PHY Status Register */
+#define PHY_REG_STATUS1_SPEED		0xc000
+#define PHY_REG_STATUS1_GBIT		0x8000
+#define PHY_REG_STATUS1_100		0x4000
+#define PHY_REG_STATUS1_DUPLEX		0x2000
+#define PHY_REG_STATUS1_SPDDONE		0x0800
+#define PHY_REG_STATUS1_LINK		0x0400
+#define PHY_REG_STATUS1_ENERGY		0x0010
+
+/*
+ * Macros for building commands for indirect addressing modes.  These are valid
+ * for both the indirect multichip addressing mode and the PHY indirection
+ * required for the writes to any PHY register.
+ */
+#define SMI_BUSY			BIT(15)
+#define SMI_CMD_CLAUSE_22		BIT(12)
+#define SMI_CMD_CLAUSE_22_OP_READ	(2 << 10)
+#define SMI_CMD_CLAUSE_22_OP_WRITE	(1 << 10)
+
+#define SMI_CMD_READ			(SMI_BUSY | SMI_CMD_CLAUSE_22 | \
+					 SMI_CMD_CLAUSE_22_OP_READ)
+#define SMI_CMD_WRITE			(SMI_BUSY | SMI_CMD_CLAUSE_22 | \
+					 SMI_CMD_CLAUSE_22_OP_WRITE)
+
+#define SMI_CMD_ADDR_SHIFT		5
+#define SMI_CMD_ADDR_WIDTH		5
+#define SMI_CMD_REG_SHIFT		0
+#define SMI_CMD_REG_WIDTH		5
+
+/* Defines for Scratch and Misc Registers */
+#define SCRATCH_UPDATE			BIT(15)
+#define SCRATCH_ADDR_SHIFT		8
+#define SCRATCH_ADDR_WIDTH		7
+
+/* Scratch registers */
+#define SCRATCH_ADDR_GPIO0DIR		0x62 /* GPIO[7:0] direction 1=input */
+#define SCRATCH_ADDR_GPIO1DIR		0x63 /* GPIO[14:8] direction 1=input */
+#define SCRATCH_ADDR_GPIO0DATA		0x64 /* GPIO[7:0] data */
+#define SCRATCH_ADDR_GPIO1DATA		0x65 /* GPIO[14:8] data */
+#define SCRATCH_ADDR_GPIO0CTRL		0x68 /* GPIO[1:0] control */
+#define SCRATCH_ADDR_GPIO1CTRL		0x69 /* GPIO[3:2] control */
+
+/* ID register values for different switch models */
+#define PORT_SWITCH_ID_6020		0x0200
+#define PORT_SWITCH_ID_6070		0x0700
+#define PORT_SWITCH_ID_6071		0x0710
+#define PORT_SWITCH_ID_6096		0x0980
+#define PORT_SWITCH_ID_6097		0x0990
+#define PORT_SWITCH_ID_6172		0x1720
+#define PORT_SWITCH_ID_6176		0x1760
+#define PORT_SWITCH_ID_6220		0x2200
+#define PORT_SWITCH_ID_6240		0x2400
+#define PORT_SWITCH_ID_6250		0x2500
+#define PORT_SWITCH_ID_6352		0x3520
+
+struct mv88e6xxx_priv {
+	int smi_addr;
+	int id;
+	int port_count;		/* Number of switch ports */
+	int port_reg_base;	/* Base of the switch port registers */
+	u8 global1;	/* Offset of Switch Global 1 registers */
+	u8 global2;	/* Offset of Switch Global 2 registers */
+	u8 phy_ctrl1_en_det_shift; /* 'EDet' bit field offset */
+	u8 phy_ctrl1_en_det_width; /* Width of 'EDet' bit field */
+	u8 phy_ctrl1_en_det_ctrl;  /* 'EDet' control value */
+};
+
+static inline int smi_cmd(int cmd, int addr, int reg)
+{
+	cmd = bitfield_replace(cmd, SMI_CMD_ADDR_SHIFT, SMI_CMD_ADDR_WIDTH,
+			       addr);
+	cmd = bitfield_replace(cmd, SMI_CMD_REG_SHIFT, SMI_CMD_REG_WIDTH, reg);
+	return cmd;
+}
+
+static inline int smi_cmd_read(int addr, int reg)
+{
+	return smi_cmd(SMI_CMD_READ, addr, reg);
+}
+
+static inline int smi_cmd_write(int addr, int reg)
+{
+	return smi_cmd(SMI_CMD_WRITE, addr, reg);
+}
+
+/* Wait for the current SMI indirect command to complete */
+static int mv88e6xxx_smi_wait(struct udevice *dev, int smi_addr)
+{
+	int val;
+	u32 timeout = 100;
+
+	do {
+		val = dm_mdio_read(dev->parent, smi_addr, MDIO_DEVAD_NONE, SMI_CMD_REG);
+		if (val >= 0 && (val & SMI_BUSY) == 0)
+			return 0;
+
+		mdelay(1);
+	} while (--timeout);
+
+	dev_err(dev, "SMI busy timeout\n");
+	return -ETIMEDOUT;
+}
+
+/*
+ * The mv88e6xxx has three types of addresses: the smi bus address, the device
+ * address, and the register address.  The smi bus address distinguishes it on
+ * the smi bus from other PHYs or switches.  The device address determines
+ * which on-chip register set you are reading/writing (the various PHYs, their
+ * associated ports, or global configuration registers).  The register address
+ * is the offset of the register you are reading/writing.
+ *
+ * When the mv88e6xxx is hardware configured to have address zero, it behaves in
+ * single-chip addressing mode, where it responds to all SMI addresses, using
+ * the smi address as its device address.  This obviously only works when this
+ * is the only chip on the SMI bus.  This allows the driver to access device
+ * registers without using indirection.  When the chip is configured to a
+ * non-zero address, it only responds to that SMI address and requires indirect
+ * writes to access the different device addresses.
+ */
+static int mv88e6xxx_reg_read(struct udevice *dev, int addr, int reg)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	int smi_addr = priv->smi_addr;
+	int res;
+
+	/* In single-chip mode, the device can be addressed directly */
+	if (smi_addr == 0)
+		return dm_mdio_read(dev->parent, addr, MDIO_DEVAD_NONE, reg);
+
+	/* Wait for the bus to become free */
+	res = mv88e6xxx_smi_wait(dev, smi_addr);
+	if (res < 0)
+		return res;
+
+	/* Issue the read command */
+	res = dm_mdio_write(dev->parent, smi_addr, MDIO_DEVAD_NONE, SMI_CMD_REG,
+			    smi_cmd_read(addr, reg));
+	if (res < 0)
+		return res;
+
+	/* Wait for the read command to complete */
+	res = mv88e6xxx_smi_wait(dev, smi_addr);
+	if (res < 0)
+		return res;
+
+	/* Read the data */
+	res = dm_mdio_read(dev->parent, smi_addr, MDIO_DEVAD_NONE, SMI_DATA_REG);
+	if (res < 0)
+		return res;
+
+	return bitfield_extract(res, 0, 16);
+}
+
+/* See the comment above mv88e6xxx_reg_read */
+static int mv88e6xxx_reg_write(struct udevice *dev, int addr, int reg, u16 val)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	int smi_addr = priv->smi_addr;
+	int res;
+
+	/* In single-chip mode, the device can be addressed directly */
+	if (smi_addr == 0)
+		return dm_mdio_write(dev->parent, addr, MDIO_DEVAD_NONE, reg, val);
+
+	/* Wait for the bus to become free */
+	res = mv88e6xxx_smi_wait(dev, smi_addr);
+	if (res < 0)
+		return res;
+
+	/* Set the data to write */
+	res = dm_mdio_write(dev->parent, smi_addr, MDIO_DEVAD_NONE,
+			    SMI_DATA_REG, val);
+	if (res < 0)
+		return res;
+
+	/* Issue the write command */
+	res = dm_mdio_write(dev->parent, smi_addr, MDIO_DEVAD_NONE, SMI_CMD_REG,
+			    smi_cmd_write(addr, reg));
+	if (res < 0)
+		return res;
+
+	/* Wait for the write command to complete */
+	res = mv88e6xxx_smi_wait(dev, smi_addr);
+	if (res < 0)
+		return res;
+
+	return 0;
+}
+
+static int mv88e6xxx_phy_wait(struct udevice *dev)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	int val;
+	u32 timeout = 100;
+
+	do {
+		val = mv88e6xxx_reg_read(dev, priv->global2, GLOBAL2_REG_PHY_CMD);
+		if (val >= 0 && (val & SMI_BUSY) == 0)
+			return 0;
+
+		mdelay(1);
+	} while (--timeout);
+
+	return -ETIMEDOUT;
+}
+
+static int mv88e6xxx_phy_read_indirect(struct udevice *dev, int phyad, int devad, int reg)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	int res;
+
+	/* Issue command to read */
+	res = mv88e6xxx_reg_write(dev, priv->global2,
+				  GLOBAL2_REG_PHY_CMD,
+				  smi_cmd_read(phyad, reg));
+
+	/* Wait for data to be read */
+	res = mv88e6xxx_phy_wait(dev);
+	if (res < 0)
+		return res;
+
+	/* Read retrieved data */
+	return mv88e6xxx_reg_read(dev, priv->global2,
+				  GLOBAL2_REG_PHY_DATA);
+}
+
+static int mv88e6xxx_phy_write_indirect(struct udevice *dev, int phyad,
+					int devad, int reg, u16 data)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	int res;
+
+	/* Set the data to write */
+	res = mv88e6xxx_reg_write(dev, priv->global2,
+				  GLOBAL2_REG_PHY_DATA, data);
+	if (res < 0)
+		return res;
+	/* Issue the write command */
+	res = mv88e6xxx_reg_write(dev, priv->global2,
+				  GLOBAL2_REG_PHY_CMD,
+				  smi_cmd_write(phyad, reg));
+	if (res < 0)
+		return res;
+
+	/* Wait for command to complete */
+	return mv88e6xxx_phy_wait(dev);
+}
+
+/* Wrapper function to make calls to phy_read_indirect simpler */
+static int mv88e6xxx_phy_read(struct udevice *dev, int phy, int reg)
+{
+	return mv88e6xxx_phy_read_indirect(dev, DEVADDR_PHY(phy),
+					   MDIO_DEVAD_NONE, reg);
+}
+
+/* Wrapper function to make calls to phy_write_indirect simpler */
+static int mv88e6xxx_phy_write(struct udevice *dev, int phy, int reg, u16 val)
+{
+	return mv88e6xxx_phy_write_indirect(dev, DEVADDR_PHY(phy),
+					    MDIO_DEVAD_NONE, reg, val);
+}
+
+static int mv88e6xxx_port_read(struct udevice *dev, u8 port, u8 reg)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+
+	return mv88e6xxx_reg_read(dev, priv->port_reg_base + port, reg);
+}
+
+static int mv88e6xxx_port_write(struct udevice *dev, u8 port, u8 reg, u16 val)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+
+	return mv88e6xxx_reg_write(dev, priv->port_reg_base + port, reg, val);
+}
+
+static int mv88e6xxx_set_page(struct udevice *dev, u8 phy, u8 page)
+{
+	return mv88e6xxx_phy_write(dev, phy, PHY_REG_PAGE, page);
+}
+
+static int mv88e6xxx_get_switch_id(struct udevice *dev)
+{
+	int res;
+
+	res = mv88e6xxx_port_read(dev, 0, PORT_REG_SWITCH_ID);
+	if (res < 0)
+		return res;
+	return res & 0xfff0;
+}
+
+static bool mv88e6xxx_6352_family(struct udevice *dev)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+
+	switch (priv->id) {
+	case PORT_SWITCH_ID_6172:
+	case PORT_SWITCH_ID_6176:
+	case PORT_SWITCH_ID_6240:
+	case PORT_SWITCH_ID_6352:
+		return true;
+	}
+	return false;
+}
+
+static int mv88e6xxx_get_cmode(struct udevice *dev, u8 port)
+{
+	int res;
+
+	res = mv88e6xxx_port_read(dev, port, PORT_REG_STATUS);
+	if (res < 0)
+		return res;
+	return res & PORT_REG_STATUS_CMODE_MASK;
+}
+
+static int mv88e6xxx_switch_reset(struct udevice *dev)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	int time;
+	int val;
+	u8 port;
+
+	/* Disable all ports */
+	for (port = 0; port < priv->port_count; port++) {
+		val = mv88e6xxx_port_read(dev, port, PORT_REG_CTRL);
+		if (val < 0)
+			return val;
+		val = bitfield_replace(val, PORT_REG_CTRL_PSTATE_SHIFT,
+				       PORT_REG_CTRL_PSTATE_WIDTH,
+				       PORT_REG_CTRL_PSTATE_DISABLED);
+		val = mv88e6xxx_port_write(dev, port, PORT_REG_CTRL, val);
+		if (val < 0)
+			return val;
+	}
+
+	/* Wait 2 ms for queues to drain */
+	udelay(2000);
+
+	/* Reset switch */
+	val = mv88e6xxx_reg_read(dev, priv->global1, GLOBAL1_CTRL);
+	if (val < 0)
+		return val;
+	val |= GLOBAL1_CTRL_SWRESET;
+	val = mv88e6xxx_reg_write(dev, priv->global1, GLOBAL1_CTRL, val);
+	if (val < 0)
+		return val;
+
+	/* Wait up to 1 second for switch reset complete */
+	for (time = 1000; time; time--) {
+		val = mv88e6xxx_reg_read(dev, priv->global1, GLOBAL1_CTRL);
+		if (val >= 0 && ((val & GLOBAL1_CTRL_SWRESET) == 0))
+			break;
+		udelay(1000);
+	}
+	if (!time)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int mv88e6xxx_serdes_init(struct udevice *dev)
+{
+	int val;
+
+	val = mv88e6xxx_set_page(dev, DEVADDR_SERDES, PHY_PAGE_SERDES);
+	if (val < 0)
+		return val;
+
+	/* Power up serdes module */
+	val = mv88e6xxx_phy_read(dev, DEVADDR_SERDES, MII_BMCR);
+	if (val < 0)
+		return val;
+	val &= ~(BMCR_PDOWN);
+	val = mv88e6xxx_phy_write(dev, DEVADDR_SERDES, MII_BMCR, val);
+	if (val < 0)
+		return val;
+
+	return 0;
+}
+
+static int mv88e6xxx_fixed_port_setup(struct udevice *dev, u8 port)
+{
+	struct dsa_pdata *dsa_pdata = dev_get_uclass_plat(dev);
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	int val;
+
+	val = mv88e6xxx_port_read(dev, port, PORT_REG_PHYS_CTRL);
+	if (val < 0)
+		return val;
+
+	val &= ~(PORT_REG_PHYS_CTRL_SPD_MASK |
+		 PORT_REG_PHYS_CTRL_FC_VALUE |
+		 PORT_REG_PHYS_CTRL_FC_FORCE);
+	val |= PORT_REG_PHYS_CTRL_FC_FORCE |
+	       PORT_REG_PHYS_CTRL_DUPLEX_VALUE |
+	       PORT_REG_PHYS_CTRL_DUPLEX_FORCE;
+
+	if (priv->id == PORT_SWITCH_ID_6071) {
+		val |= PORT_REG_PHYS_CTRL_SPD100;
+	} else {
+		val |= PORT_REG_PHYS_CTRL_PCS_AN_EN |
+		       PORT_REG_PHYS_CTRL_PCS_AN_RST |
+		       PORT_REG_PHYS_CTRL_SPD1000;
+	}
+
+	if (port == dsa_pdata->cpu_port)
+		val |= PORT_REG_PHYS_CTRL_LINK_VALUE |
+		       PORT_REG_PHYS_CTRL_LINK_FORCE;
+
+	return mv88e6xxx_port_write(dev, port, PORT_REG_PHYS_CTRL, val);
+}
+
+/*
+ * This function is used to pre-configure the required register
+ * offsets, so that the indirect register access to the PHY registers
+ * is possible. This is necessary to be able to read the PHY ID
+ * while driver probing or in get_phy_id(). The globalN register
+ * offsets must be initialized correctly for a detected switch,
+ * otherwise detection of the PHY ID won't work!
+ */
+static int mv88e6xxx_priv_reg_offs_pre_init(struct udevice *dev)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+
+	/*
+	 * Initial 'port_reg_base' value must be an offset of existing
+	 * port register, then reading the ID should succeed. First, try
+	 * to read via port registers with device address 0x10 (88E6096
+	 * and compatible switches).
+	 */
+	priv->port_reg_base = 0x10;
+	priv->id = mv88e6xxx_get_switch_id(dev);
+	if (priv->id != 0xfff0) {
+		priv->global1 = 0x1B;
+		priv->global2 = 0x1C;
+		return 0;
+	}
+
+	/*
+	 * Now try via port registers with device address 0x08
+	 * (88E6020 and compatible switches).
+	 */
+	priv->port_reg_base = 0x08;
+	priv->id = mv88e6xxx_get_switch_id(dev);
+	if (priv->id != 0xfff0) {
+		priv->global1 = 0x0F;
+		priv->global2 = 0x07;
+		return 0;
+	}
+
+	dev_warn(dev, "%s Unknown ID 0x%x\n", __func__, priv->id);
+
+	return -ENODEV;
+}
+
+static int mv88e6xxx_mdio_read(struct udevice *dev, int addr, int devad, int reg)
+{
+	return mv88e6xxx_phy_read_indirect(dev->parent, DEVADDR_PHY(addr),
+					   MDIO_DEVAD_NONE, reg);
+}
+
+static int mv88e6xxx_mdio_write(struct udevice *dev, int addr, int devad,
+				int reg, u16 val)
+{
+	return mv88e6xxx_phy_write_indirect(dev->parent, DEVADDR_PHY(addr),
+					   MDIO_DEVAD_NONE, reg, val);
+}
+
+static const struct mdio_ops mv88e6xxx_mdio_ops = {
+	.read = mv88e6xxx_mdio_read,
+	.write = mv88e6xxx_mdio_write,
+};
+
+static int mv88e6xxx_mdio_bind(struct udevice *dev)
+{
+	char name[32];
+	static int num_devices;
+
+	sprintf(name, "mv88e6xxx-mdio-%d", num_devices++);
+	device_set_name(dev, name);
+
+	return 0;
+}
+
+U_BOOT_DRIVER(mv88e6xxx_mdio) = {
+	.name		= "mv88e6xxx_mdio",
+	.id		= UCLASS_MDIO,
+	.ops		= &mv88e6xxx_mdio_ops,
+	.bind		= mv88e6xxx_mdio_bind,
+	.priv_auto	= sizeof(struct mv88e6xxx_priv),
+	.plat_auto	= sizeof(struct mdio_perdev_priv),
+};
+
+static int mv88e6xxx_port_enable(struct udevice *dev, int port, struct phy_device *phy)
+{
+	struct dsa_pdata *dsa_pdata = dev_get_uclass_plat(dev);
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	int val, ret;
+
+	dev_dbg(dev, "%s P%d phy:0x%08x %s\n", __func__, port,
+		phy->phy_id, phy_string_for_interface(phy->interface));
+
+	/*
+	 * Enable energy-detect sensing on PHY, used to determine when a PHY
+	 * port is physically connected
+	 */
+	val = mv88e6xxx_phy_read(dev, port, PHY_REG_CTRL1);
+	if (val < 0)
+		return val;
+	val = bitfield_replace(val, priv->phy_ctrl1_en_det_shift,
+			       priv->phy_ctrl1_en_det_width,
+			       priv->phy_ctrl1_en_det_ctrl);
+	val = mv88e6xxx_phy_write(dev, port, PHY_REG_CTRL1, val);
+	if (val < 0)
+		return val;
+
+	/* enable port */
+	val = mv88e6xxx_port_read(dev, port, PORT_REG_CTRL);
+	if (val < 0)
+		return val;
+	val = bitfield_replace(val, PORT_REG_CTRL_PSTATE_SHIFT,
+			       PORT_REG_CTRL_PSTATE_WIDTH,
+			       PORT_REG_CTRL_PSTATE_FORWARD);
+	val = mv88e6xxx_port_write(dev, port, PORT_REG_CTRL, val);
+	if (val < 0)
+		return val;
+
+	/* configure RGMII delays for fixed link */
+	if (phy->phy_id == PHY_FIXED_ID) {
+		/* Physical Control register: Table 62 */
+		val = mv88e6xxx_port_read(dev, port, PORT_REG_PHYS_CTRL);
+		val &= ~(PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK ||
+			 PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK);
+		if (phy->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+		    phy->interface == PHY_INTERFACE_MODE_RGMII_RXID)
+			val |= PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK;
+		if (phy->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+		    phy->interface == PHY_INTERFACE_MODE_RGMII_TXID)
+			val |= PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK;
+		val |= BIT(5) | BIT(4); /* Force link */
+		ret = mv88e6xxx_port_write(dev, port, PORT_REG_PHYS_CTRL, val);
+		if (ret < 0)
+			return ret;
+
+		ret = mv88e6xxx_fixed_port_setup(dev, port);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (port == dsa_pdata->cpu_port) {
+		/* Set CPUDest for cpu port */
+		val = mv88e6xxx_reg_read(dev, priv->global1, GLOBAL1_MON_CTRL);
+		if (val < 0)
+			return val;
+		val = bitfield_replace(val, GLOBAL1_MON_CTRL_CPUDEST_SHIFT,
+				       GLOBAL1_MON_CTRL_CPUDEST_WIDTH, port);
+		val = mv88e6xxx_reg_write(dev, priv->global1, GLOBAL1_MON_CTRL, val);
+		if (val < 0)
+			return val;
+
+		if (mv88e6xxx_6352_family(dev)) {
+			/* initialize serdes */
+			val = mv88e6xxx_get_cmode(dev, dsa_pdata->cpu_port);
+			if (val < 0)
+				return val;
+			if (val == PORT_REG_STATUS_CMODE_100BASE_X ||
+			    val == PORT_REG_STATUS_CMODE_1000BASE_X ||
+			    val == PORT_REG_STATUS_CMODE_SGMII) {
+				val = mv88e6xxx_serdes_init(dev);
+				if (val < 0)
+					return val;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void mv88e6xxx_port_disable(struct udevice *dev, int port, struct phy_device *phy)
+{
+	int val;
+
+	dev_dbg(dev, "%s P%d phy:0x%x\n", __func__, port, phy->phy_id);
+
+	val = mv88e6xxx_port_read(dev, port, PORT_REG_CTRL);
+	if (val < 0)
+		return;
+	val = bitfield_replace(val, PORT_REG_CTRL_PSTATE_SHIFT,
+			       PORT_REG_CTRL_PSTATE_WIDTH,
+			       PORT_REG_CTRL_PSTATE_DISABLED);
+	val = mv88e6xxx_port_write(dev, port, PORT_REG_CTRL, val);
+	if (val < 0)
+		return;
+}
+
+static const struct dsa_ops mv88e6xxx_dsa_ops = {
+	.port_enable = mv88e6xxx_port_enable,
+	.port_disable = mv88e6xxx_port_disable,
+};
+
+/* bind and probe the switch mdios */
+static int mv88e6xxx_probe_mdio(struct udevice *dev)
+{
+	struct udevice *mdev;
+	ofnode node, mdios;
+	const char *name;
+	int ret;
+
+	/* bind phy ports of mdios child node to mv88e6xxx_mdio device */
+	mdios = dev_read_subnode(dev, "mdios");
+	if (!ofnode_valid(mdios))
+		return 0;
+
+	ofnode_for_each_subnode(node, mdios) {
+		name = ofnode_get_name(node);
+		ret = device_bind_driver_to_node(dev,
+						 "mv88e6xxx_mdio",
+						 name, node, NULL);
+		if (ret) {
+			dev_err(dev, "failed to bind %s: %d\n", name, ret);
+			continue;
+		}
+
+		/* need to probe it as there is no compatible to do so */
+		ret = uclass_get_device_by_ofnode(UCLASS_MDIO, node, &mdev);
+		if (ret) {
+			dev_err(dev, "failed to probe %s: %d\n", name, ret);
+			continue;
+		}
+	}
+
+	return ret;
+}
+
+static int mv88e6xxx_probe(struct udevice *dev)
+{
+	struct mv88e6xxx_priv *priv = dev_get_priv(dev);
+	struct udevice *master = dsa_get_master(dev);
+	int ret;
+
+	if (!master)
+		return -ENODEV;
+
+	dev_dbg(dev, "%s master:%s\n", __func__, master->name);
+
+	/* probe internal mdio bus */
+	ret = mv88e6xxx_probe_mdio(dev);
+	if (ret)
+		return ret;
+
+	ret = mv88e6xxx_priv_reg_offs_pre_init(dev);
+	if (ret)
+		return ret;
+
+	dev_dbg(dev, "ID=0x%x PORT_BASE=0x%02x GLOBAL1=0x%02x GLOBAL2=0x%02x\n",
+		priv->id, priv->port_reg_base, priv->global1, priv->global2);
+	switch (priv->id) {
+	case PORT_SWITCH_ID_6096:
+	case PORT_SWITCH_ID_6097:
+	case PORT_SWITCH_ID_6172:
+	case PORT_SWITCH_ID_6176:
+	case PORT_SWITCH_ID_6240:
+	case PORT_SWITCH_ID_6352:
+		priv->port_count = 11;
+		priv->phy_ctrl1_en_det_shift = 8;
+		priv->phy_ctrl1_en_det_width = 2;
+		priv->phy_ctrl1_en_det_ctrl =
+			PHY_REG_CTRL1_ENERGY_DET_SENSE_XMIT;
+		break;
+	case PORT_SWITCH_ID_6020:
+	case PORT_SWITCH_ID_6070:
+	case PORT_SWITCH_ID_6071:
+	case PORT_SWITCH_ID_6220:
+	case PORT_SWITCH_ID_6250:
+		priv->port_count = 7;
+		priv->phy_ctrl1_en_det_shift = 14;
+		priv->phy_ctrl1_en_det_width = 1;
+		priv->phy_ctrl1_en_det_ctrl =
+			PHY_REG_CTRL1_ENERGY_DET_SENSE_PULSE;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	ret = mv88e6xxx_switch_reset(dev);
+	if (ret < 0)
+		return ret;
+
+	dsa_set_tagging(dev, 0, 0);
+
+	return 0;
+}
+
+static const struct udevice_id mv88e6xxx_ids[] = {
+	{ .compatible = "marvell,mv88e6085" },
+	{ }
+};
+
+U_BOOT_DRIVER(mv88e6xxx) = {
+	.name		= "mv88e6xxx",
+	.id		= UCLASS_DSA,
+	.of_match	= mv88e6xxx_ids,
+	.probe		= mv88e6xxx_probe,
+	.ops		= &mv88e6xxx_dsa_ops,
+	.priv_auto	= sizeof(struct mv88e6xxx_priv),
+};
-- 
2.25.1

[PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Tim Harvey]

Add MV88E61XX DSA support:
 - update dt: U-Boot dsa driver requires different device-tree syntax
   than the linux driver in order to link the dsa ports to the mdio bus.
 - update defconfig
 - replace mv88e61xx_hw_reset weak override with board_phy_config support
   for mv88e61xx configuration that is outside the scope of the DSA driver

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
v4:
 - use standard Linux internal MDIO dt structure
 - use PHY_FIXED_ID define
v3:
 - move mdio's mdio@0 subnode
v2: no changes
---
 arch/arm/dts/imx6qdl-gw5904.dtsi        | 31 +++++++++++++++
 board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++----------------
 configs/gwventana_gw5904_defconfig      |  8 ++--
 drivers/net/mv88e6xxx.c                 | 29 ++++++--------
 4 files changed, 65 insertions(+), 53 deletions(-)

diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi
index 612b6e068e28..487dd7648274 100644
--- a/arch/arm/dts/imx6qdl-gw5904.dtsi
+++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
@@ -212,6 +212,27 @@
 			compatible = "marvell,mv88e6085";
 			reg = <0>;
 
+			mdio {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				sw_phy0: ethernet-phy@0 {
+					reg = <0x0>;
+				};
+
+				sw_phy1: ethernet-phy@1 {
+					reg = <0x1>;
+				};
+
+				sw_phy2: ethernet-phy@2 {
+					reg = <0x2>;
+				};
+
+				sw_phy3: ethernet-phy@3 {
+					reg = <0x3>;
+				};
+			};
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -219,27 +240,37 @@
 				port@0 {
 					reg = <0>;
 					label = "lan4";
+					phy-handle = <&sw_phy0>;
 				};
 
 				port@1 {
 					reg = <1>;
 					label = "lan3";
+					phy-handle = <&sw_phy1>;
 				};
 
 				port@2 {
 					reg = <2>;
 					label = "lan2";
+					phy-handle = <&sw_phy2>;
 				};
 
 				port@3 {
 					reg = <3>;
 					label = "lan1";
+					phy-handle = <&sw_phy3>;
 				};
 
 				port@5 {
 					reg = <5>;
 					label = "cpu";
 					ethernet = <&fec>;
+					phy-mode = "rgmii-id";
+
+					fixed-link {
+						speed = <1000>;
+						full-duplex;
+					};
 				};
 			};
 		};
diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c
index 99f52b9953e2..1c3da2c82707 100644
--- a/board/gateworks/gw_ventana/gw_ventana.c
+++ b/board/gateworks/gw_ventana/gw_ventana.c
@@ -83,44 +83,30 @@ int board_phy_config(struct phy_device *phydev)
 		break;
 	}
 
+	/* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */
+	if (phydev->phy_id == PHY_FIXED_ID &&
+	    (board_type == GW5904 || board_type == GW5909)) {
+		struct mii_dev *bus = miiphy_get_dev_by_name("mdio");
+
+		puts("MV88E61XX ");
+		/* GPIO[0] output CLK125 for RGMII_REFCLK */
+		bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe);
+		bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7);
+
+		/* Port 0-3 LED configuration: Table 80/82 */
+		/* LED configuration: 7:4-green (8=Activity)  3:0 amber (8=Link) */
+		bus->write(bus, 0x10, 0, 0x16, 0x8088);
+		bus->write(bus, 0x11, 0, 0x16, 0x8088);
+		bus->write(bus, 0x12, 0, 0x16, 0x8088);
+		bus->write(bus, 0x13, 0, 0x16, 0x8088);
+	}
+
 	if (phydev->drv->config)
 		phydev->drv->config(phydev);
 
 	return 0;
 }
 
-#ifdef CONFIG_MV88E61XX_SWITCH
-int mv88e61xx_hw_reset(struct phy_device *phydev)
-{
-	struct mii_dev *bus = phydev->bus;
-
-	/* GPIO[0] output, CLK125 */
-	debug("enabling RGMII_REFCLK\n");
-	bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0,
-		   0x1a /*MV_SCRATCH_MISC*/,
-		   (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe);
-	bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0,
-		   0x1a /*MV_SCRATCH_MISC*/,
-		   (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7);
-
-	/* RGMII delay - Physical Control register bit[15:14] */
-	debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT);
-	/* forced 1000mbps full-duplex link */
-	bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe);
-	phydev->autoneg = AUTONEG_DISABLE;
-	phydev->speed = SPEED_1000;
-	phydev->duplex = DUPLEX_FULL;
-
-	/* LED configuration: 7:4-green (8=Activity)  3:0 amber (8=Link) */
-	bus->write(bus, 0x10, 0, 0x16, 0x8088);
-	bus->write(bus, 0x11, 0, 0x16, 0x8088);
-	bus->write(bus, 0x12, 0, 0x16, 0x8088);
-	bus->write(bus, 0x13, 0, 0x16, 0x8088);
-
-	return 0;
-}
-#endif // CONFIG_MV88E61XX_SWITCH
-
 #if defined(CONFIG_VIDEO_IPUV3)
 static void enable_hdmi(struct display_info_t const *dev)
 {
diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig
index 8c6baf0ae5e2..33b57c5a1c5d 100644
--- a/configs/gwventana_gw5904_defconfig
+++ b/configs/gwventana_gw5904_defconfig
@@ -111,11 +111,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y
 CONFIG_FSL_USDHC=y
 CONFIG_MTD=y
 CONFIG_PHYLIB=y
-CONFIG_MV88E61XX_SWITCH=y
-CONFIG_MV88E61XX_CPU_PORT=5
-CONFIG_MV88E61XX_PHY_PORTS=0xf
-CONFIG_MV88E61XX_FIXED_PORTS=0x0
+CONFIG_MV88E61XX=y
+CONFIG_PHY_FIXED=y
+CONFIG_DM_ETH=y
 CONFIG_DM_MDIO=y
+CONFIG_DM_DSA=y
 CONFIG_E1000=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
diff --git a/drivers/net/mv88e6xxx.c b/drivers/net/mv88e6xxx.c
index a3a3f6cf4acc..6064bb73257b 100644
--- a/drivers/net/mv88e6xxx.c
+++ b/drivers/net/mv88e6xxx.c
@@ -727,31 +727,26 @@ static const struct dsa_ops mv88e6xxx_dsa_ops = {
 static int mv88e6xxx_probe_mdio(struct udevice *dev)
 {
 	struct udevice *mdev;
-	ofnode node, mdios;
 	const char *name;
+	ofnode node;
 	int ret;
 
-	/* bind phy ports of mdios child node to mv88e6xxx_mdio device */
-	mdios = dev_read_subnode(dev, "mdios");
-	if (!ofnode_valid(mdios))
+	/* bind phy ports of mdio child node to mv88e6xxx_mdio device */
+	node = dev_read_subnode(dev, "mdio");
+	if (!ofnode_valid(node))
 		return 0;
 
-	ofnode_for_each_subnode(node, mdios) {
-		name = ofnode_get_name(node);
-		ret = device_bind_driver_to_node(dev,
-						 "mv88e6xxx_mdio",
-						 name, node, NULL);
-		if (ret) {
-			dev_err(dev, "failed to bind %s: %d\n", name, ret);
-			continue;
-		}
-
+	name = ofnode_get_name(node);
+	ret = device_bind_driver_to_node(dev,
+					 "mv88e6xxx_mdio",
+					 name, node, NULL);
+	if (ret) {
+		dev_err(dev, "failed to bind %s: %d\n", name, ret);
+	} else {
 		/* need to probe it as there is no compatible to do so */
 		ret = uclass_get_device_by_ofnode(UCLASS_MDIO, node, &mdev);
-		if (ret) {
+		if (ret)
 			dev_err(dev, "failed to probe %s: %d\n", name, ret);
-			continue;
-		}
 	}
 
 	return ret;
-- 
2.25.1

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

Hi Tim,

On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> Add MV88E61XX DSA support:
>  - update dt: U-Boot dsa driver requires different device-tree syntax
>    than the linux driver in order to link the dsa ports to the mdio bus.

Shouldn't the U-Boot dts version (imx6qdl-gw5904-u-boot.dtsi) be
updated instead?

Otherwise, this will cause problems when syncing imx6qdl-gw5904.dtsi
with Linux.

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Tim Harvey]

On Wed, Sep 28, 2022 at 12:56 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > Add MV88E61XX DSA support:
> >  - update dt: U-Boot dsa driver requires different device-tree syntax
> >    than the linux driver in order to link the dsa ports to the mdio bus.
>
> Shouldn't the U-Boot dts version (imx6qdl-gw5904-u-boot.dtsi) be
> updated instead?
>
> Otherwise, this will cause problems when syncing imx6qdl-gw5904.dtsi
> with Linux.

Fabio,

That is one route to go but I'm hoping to update the Linux dt as well
to match so I'm not worried about it at this time.

Best Regards,

Tim

Re: [PATCH v4 0/8] Add MV88E6xxx DSA driver and use on gwventana

[Fabio Estevam]

Hi Tim,

On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> This series adds a DSA driver for the MV88E6xxx based on
> drivers/net/phy/mv88e61xx and uses it in the gwventana_gw5904_defconfig.
>
> The hope is that the other three boards that use the MV88E61xx driver
> can move to this as well eventually so that we can remove the non-dm
> driver and the 4 Kconfig options it requires.
>
> The MV88E6xxx has an MDIO interface thus DM_MDIO must be used so support
> for a UCLASS_MDIO driver is added to the fec_mxc ethernet driver in a
> way that allows a fallback to the previous non DM_MDIO case as there are
> many boards out there using this driver that define DM_MDIO but do not
> have the required dt props for a DM_MDIO driver which would cause a
> regression.
>
> Additionally some other patches are here suggested by Vladimir:
>  - ensure MDIO children are scanned on post-bind is needed
>  - sanity check DSA driver required ops are present
>  - allow DSA drivers to not require xmit/recv functions
>  - remove unecessary xmit/recv functions from ksz9477 driver

I am trying to test your series on a custom imx8mn-based board with a Marvell
88E6320 switch.

I applied your series against U-Boot master, and added the change below
to support 88E6320:

diff --git a/drivers/net/mv88e6xxx.c b/drivers/net/mv88e6xxx.c
index 6064bb7325..09ce6dd114 100644
--- a/drivers/net/mv88e6xxx.c
+++ b/drivers/net/mv88e6xxx.c
@@ -176,6 +176,7 @@
 #define PORT_SWITCH_ID_6220            0x2200
 #define PORT_SWITCH_ID_6240            0x2400
 #define PORT_SWITCH_ID_6250            0x2500
+#define PORT_SWITCH_ID_6320            0x1150
 #define PORT_SWITCH_ID_6352            0x3520

 struct mv88e6xxx_priv {
@@ -792,6 +793,7 @@ static int mv88e6xxx_probe(struct udevice *dev)
        case PORT_SWITCH_ID_6071:
        case PORT_SWITCH_ID_6220:
        case PORT_SWITCH_ID_6250:
+       case PORT_SWITCH_ID_6320:
                priv->port_count = 7;
                priv->phy_ctrl1_en_det_shift = 14;
                priv->phy_ctrl1_en_det_width = 1;

When booting I get:

Error: ethernet@30be0000 address not set.
No ethernet found.

And 'dm tree' shows that the Ethernet drivers have not been probed:

=> dm tree

 ethernet      0  [   ]   fecmxc                |   |   `-- ethernet@30be0000
...
 mdio          0  [   ]   fec_mdio              |   |       `-- mdio

If you have any suggestions, please let me know.

Thanks

Re: [PATCH v4 0/8] Add MV88E6xxx DSA driver and use on gwventana

[Fabio Estevam]

On Mon, Oct 3, 2022 at 1:26 PM Fabio Estevam <festevam@gmail.com> wrote:

> And 'dm tree' shows that the Ethernet drivers have not been probed:
>
> => dm tree
>
>  ethernet      0  [   ]   fecmxc                |   |   `-- ethernet@30be0000
> ...
>  mdio          0  [   ]   fec_mdio              |   |       `-- mdio

This was an issue in Kconfig. Now with the new mv88e6xxx driver
properly selected I get:

SEC0:  RNG instantiated
Net:
Error: ethernet@30be0000 address not set.
Error: ethernet@30be0000 address not set.
Error: ethernet@30be0000 address not set.
Error: ethernet@30be0000 address not set.
Error: ethernet@30be0000 address not
...

and not able to access console as these errors keep coming in loop.

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

Hi Tim,

On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote:

> diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig
> index 8c6baf0ae5e2..33b57c5a1c5d 100644
> --- a/configs/gwventana_gw5904_defconfig
> +++ b/configs/gwventana_gw5904_defconfig
> @@ -111,11 +111,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y
>  CONFIG_FSL_USDHC=y
>  CONFIG_MTD=y
>  CONFIG_PHYLIB=y
> -CONFIG_MV88E61XX_SWITCH=y
> -CONFIG_MV88E61XX_CPU_PORT=5
> -CONFIG_MV88E61XX_PHY_PORTS=0xf
> -CONFIG_MV88E61XX_FIXED_PORTS=0x0
> +CONFIG_MV88E61XX=y

There is a typo here:

This one should be CONFIG_MV88E6XXX instead.

After fixing this and passing CONFIG_NET_RANDOM_ETHADDR=y to my custom
board defconfig:

 ethernet      0  [ + ]   fecmxc                |   |   `-- ethernet@30be0000
 bootdev       2  [   ]   eth_bootdev           |   |       |--
ethernet@30be0000.bootdev
 mdio          0  [ + ]   fec_mdio              |   |       `-- mdio
 dsa           0  [   ]   mv88e6xxx             |   |           `-- switch@0
 ethernet      1  [   ]   dsa-port              |   |               |-- lan4
 bootdev       3  [   ]   eth_bootdev           |   |               |
 `-- switch@0@0.bootdev
 ethernet      2  [   ]   dsa-port              |   |               |-- lan3
 bootdev       4  [   ]   eth_bootdev           |   |               |
 `-- switch@0@1.bootdev
 ethernet      3  [   ]   dsa-port              |   |               |-- lan2
 bootdev       5  [   ]   eth_bootdev           |   |               |
 `-- switch@0@2.bootdev
 ethernet      4  [   ]   dsa-port              |   |               |-- lan1
 bootdev       6  [   ]   eth_bootdev           |   |               |
 `-- switch@0@3.bootdev
 mdio          1  [ + ]   mv88e6xxx_mdio        |   |
|-- mv88e6xxx-mdio-0
 mdio          2  [   ]   mv88e6xxx_mdio        |   |
|-- mv88e6xxx-mdio-1
 mdio          3  [   ]   mv88e6xxx_mdio        |   |
|-- mv88e6xxx-mdio-2
 mdio          4  [   ]   mv88e6xxx_mdio        |   |
`-- mv88e6xxx-mdio-3

but still not able to get a DHCP address:

u-boot=> dhcp
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4

Thanks

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Tim Harvey]

On Mon, Oct 3, 2022 at 10:13 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> > diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig
> > index 8c6baf0ae5e2..33b57c5a1c5d 100644
> > --- a/configs/gwventana_gw5904_defconfig
> > +++ b/configs/gwventana_gw5904_defconfig
> > @@ -111,11 +111,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y
> >  CONFIG_FSL_USDHC=y
> >  CONFIG_MTD=y
> >  CONFIG_PHYLIB=y
> > -CONFIG_MV88E61XX_SWITCH=y
> > -CONFIG_MV88E61XX_CPU_PORT=5
> > -CONFIG_MV88E61XX_PHY_PORTS=0xf
> > -CONFIG_MV88E61XX_FIXED_PORTS=0x0
> > +CONFIG_MV88E61XX=y
>
> There is a typo here:
>
> This one should be CONFIG_MV88E6XXX instead.

Ooops - thanks for pointing that out. I have another patch on top to
merge two defconfigs that fixed that typo introduced with the v4
series that changed MV88E61XX to MV88E6XXX.

>
> After fixing this and passing CONFIG_NET_RANDOM_ETHADDR=y to my custom
> board defconfig:
>
>  ethernet      0  [ + ]   fecmxc                |   |   `-- ethernet@30be0000
>  bootdev       2  [   ]   eth_bootdev           |   |       |--
> ethernet@30be0000.bootdev
>  mdio          0  [ + ]   fec_mdio              |   |       `-- mdio
>  dsa           0  [   ]   mv88e6xxx             |   |           `-- switch@0
>  ethernet      1  [   ]   dsa-port              |   |               |-- lan4
>  bootdev       3  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@0.bootdev
>  ethernet      2  [   ]   dsa-port              |   |               |-- lan3
>  bootdev       4  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@1.bootdev
>  ethernet      3  [   ]   dsa-port              |   |               |-- lan2
>  bootdev       5  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@2.bootdev
>  ethernet      4  [   ]   dsa-port              |   |               |-- lan1
>  bootdev       6  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@3.bootdev
>  mdio          1  [ + ]   mv88e6xxx_mdio        |   |
> |-- mv88e6xxx-mdio-0
>  mdio          2  [   ]   mv88e6xxx_mdio        |   |
> |-- mv88e6xxx-mdio-1
>  mdio          3  [   ]   mv88e6xxx_mdio        |   |
> |-- mv88e6xxx-mdio-2
>  mdio          4  [   ]   mv88e6xxx_mdio        |   |
> `-- mv88e6xxx-mdio-3
>
> but still not able to get a DHCP address:
>
> u-boot=> dhcp
> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> BOOTP broadcast 4

With CONFIG_CMD_NET=y what does 'net list' show?

I have the following (with network attached to lan4 port)
Net:   MV88E61XX eth0: ethernet@2188000, eth1: lan4, eth2: lan3, eth3: lan2, eth
4: lan1
Hit any key to stop autoboot:  0
Ventana > print ethaddr
ethaddr=00:d0:12:f3:f2:f5
Ventana > dm tree
...
 ethernet      0  [ + ]   fecmxc                |   |   |-- ethernet@2188000
 bootdev       0  [   ]   eth_bootdev           |   |   |   |--
ethernet@2188000.bootdev
 mdio          0  [ + ]   fec_mdio              |   |   |   `-- mdio
 dsa           0  [ + ]   mv88e6xxx             |   |   |       `-- switch@0
 ethernet      1  [ + ]   dsa-port              |   |   |           |-- lan4
 bootdev       2  [   ]   eth_bootdev           |   |   |           |
 `-- switch@0@0.bootdev
 ethernet      2  [ + ]   dsa-port              |   |   |           |-- lan3
 bootdev       3  [   ]   eth_bootdev           |   |   |           |
 `-- switch@0@1.bootdev
 ethernet      3  [ + ]   dsa-port              |   |   |           |-- lan2
 bootdev       4  [   ]   eth_bootdev           |   |   |           |
 `-- switch@0@2.bootdev
 ethernet      4  [ + ]   dsa-port              |   |   |           |-- lan1
 bootdev       5  [   ]   eth_bootdev           |   |   |           |
 `-- switch@0@3.bootdev
 mdio          1  [ + ]   mv88e6xxx_mdio        |   |   |
`-- mv88e6xxx-mdio-0
...
Ventana > net list
eth0 : ethernet@2188000 00:d0:12:f3:f2:f5 active
eth1 : lan4 00:d0:12:f3:f2:f5
eth2 : lan3 00:d0:12:f3:f2:f5
eth3 : lan2 00:d0:12:f3:f2:f5
eth4 : lan1 00:d0:12:f3:f2:f5
Ventana > setenv ethact lan4
Ventana > dhcp
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 172.24.33.142 (1076 ms)

I do not use CONFIG_NET_RANDOM_ETHADDR and have a single MAC address
allocated to the board.

What board are you using and what does its fec/mdio dt look like?

Best Regards,

Tim

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

Hi Tim,

On Mon, Oct 3, 2022 at 3:22 PM Tim Harvey <tharvey@gateworks.com> wrote:

> With CONFIG_CMD_NET=y what does 'net list' show?
>
> I have the following (with network attached to lan4 port)
> Net:   MV88E61XX eth0: ethernet@2188000, eth1: lan4, eth2: lan3, eth3: lan2, eth
> 4: lan1
> Hit any key to stop autoboot:  0
> Ventana > print ethaddr
> ethaddr=00:d0:12:f3:f2:f5
> Ventana > dm tree
> ...
>  ethernet      0  [ + ]   fecmxc                |   |   |-- ethernet@2188000
>  bootdev       0  [   ]   eth_bootdev           |   |   |   |--
> ethernet@2188000.bootdev
>  mdio          0  [ + ]   fec_mdio              |   |   |   `-- mdio
>  dsa           0  [ + ]   mv88e6xxx             |   |   |       `-- switch@0
>  ethernet      1  [ + ]   dsa-port              |   |   |           |-- lan4
>  bootdev       2  [   ]   eth_bootdev           |   |   |           |
>  `-- switch@0@0.bootdev
>  ethernet      2  [ + ]   dsa-port              |   |   |           |-- lan3
>  bootdev       3  [   ]   eth_bootdev           |   |   |           |
>  `-- switch@0@1.bootdev
>  ethernet      3  [ + ]   dsa-port              |   |   |           |-- lan2
>  bootdev       4  [   ]   eth_bootdev           |   |   |           |
>  `-- switch@0@2.bootdev
>  ethernet      4  [ + ]   dsa-port              |   |   |           |-- lan1
>  bootdev       5  [   ]   eth_bootdev           |   |   |           |
>  `-- switch@0@3.bootdev
>  mdio          1  [ + ]   mv88e6xxx_mdio        |   |   |
> `-- mv88e6xxx-mdio-0
> ...
> Ventana > net list
> eth0 : ethernet@2188000 00:d0:12:f3:f2:f5 active
> eth1 : lan4 00:d0:12:f3:f2:f5
> eth2 : lan3 00:d0:12:f3:f2:f5
> eth3 : lan2 00:d0:12:f3:f2:f5
> eth4 : lan1 00:d0:12:f3:f2:f5
> Ventana > setenv ethact lan4
> Ventana > dhcp
> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> DHCP client bound to address 172.24.33.142 (1076 ms)
>
> I do not use CONFIG_NET_RANDOM_ETHADDR and have a single MAC address
> allocated to the board.

Below are the logs:

Net:   MV88E61XX
Warning: ethernet@30be0000 (eth0) using random MAC address - a6:a6:56:1c:28:0a
eth0: ethernet@30be0000
Hit any key to stop autoboot:  0

Your log also shows eth1-eth4. Mine does not.

Unlike your log, dsa-port is not getting probed here:

 ethernet      0  [ + ]   fecmxc                |   |   `-- ethernet@30be0000
 bootdev       2  [   ]   eth_bootdev           |   |       |--
ethernet@30be0000.bootdev
 mdio          0  [ + ]   fec_mdio              |   |       `-- mdio
 dsa           0  [   ]   mv88e6xxx             |   |           `-- switch@0
 ethernet      1  [   ]   dsa-port              |   |               |-- lan4
 bootdev       3  [   ]   eth_bootdev           |   |               |
 `-- switch@0@0.bootdev
 ethernet      2  [   ]   dsa-port              |   |               |-- lan3
 bootdev       4  [   ]   eth_bootdev           |   |               |
 `-- switch@0@1.bootdev
 ethernet      3  [   ]   dsa-port              |   |               |-- lan2
 bootdev       5  [   ]   eth_bootdev           |   |               |
 `-- switch@0@2.bootdev
 ethernet      4  [   ]   dsa-port              |   |               |-- lan1
 bootdev       6  [   ]   eth_bootdev           |   |               |
 `-- switch@0@3.bootdev
 mdio          1  [ + ]   mv88e6xxx_mdio        |   |
|-- mv88e6xxx-mdio-0
 mdio          2  [   ]   mv88e6xxx_mdio        |   |
|-- mv88e6xxx-mdio-1
 mdio          3  [   ]   mv88e6xxx_mdio        |   |
|-- mv88e6xxx-mdio-2
 mdio          4  [   ]   mv88e6xxx_mdio        |   |
`-- mv88e6xxx-mdio-3
 simple_bus    7  [   ]   simple_bus            |   `-- bus@32c00000
 usb           0  [   ]   ehci_mx6              |       `-- usb@32e40000
 phy           0  [   ]   nop_phy               |-- usbphynop1
 regulator     0  [   ]   regulator_fixed       |-- regulator-usdhc2
 sysreset      1  [   ]   wdt_reboot            |-- wdt-reboot
 bootstd       0  [   ]   bootstd_drv           `-- bootstd
 bootmeth      0  [   ]   bootmeth_distro           |-- distro
 bootmeth      1  [   ]   bootmeth_efi              |-- efi
 bootmeth      2  [   ]   bootmeth_pxe              |-- pxe
 bootmeth      3  [   ]   vbe_simple                `-- vbe_simple

u-boot=> net list
eth0 : ethernet@30be0000 a6:a6:56:1c:28:0a active
eth1 : lan4 00:00:00:00:00:00
eth2 : lan3 00:00:00:00:00:00
eth3 : lan2 00:00:00:00:00:00
eth4 : lan1 00:00:00:00:00:00
u-boot=> setenv ethact lan4
u-boot=> dhcp
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4
BOOTP broadcast 5
BOOTP broadcast 6
BOOTP broadcast 7
BOOTP broadcast 8
(IP is never retrieved)

> What board are you using and what does its fec/mdio dt look like?

I am using a custom imx8mn board.

The FEC/MDIO nodes are like this:
https://pastebin.com/raw/0SvLLzxj

Thanks,

Fabio Estevam

Re: [PATCH v4 0/8] Add MV88E6xxx DSA driver and use on gwventana

[Tim Harvey]

On Mon, Oct 3, 2022 at 9:50 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Mon, Oct 3, 2022 at 1:26 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> > And 'dm tree' shows that the Ethernet drivers have not been probed:
> >
> > => dm tree
> >
> >  ethernet      0  [   ]   fecmxc                |   |   `-- ethernet@30be0000
> > ...
> >  mdio          0  [   ]   fec_mdio              |   |       `-- mdio
>
> This was an issue in Kconfig. Now with the new mv88e6xxx driver
> properly selected I get:
>
> SEC0:  RNG instantiated
> Net:
> Error: ethernet@30be0000 address not set.
> Error: ethernet@30be0000 address not set.
> Error: ethernet@30be0000 address not set.
> Error: ethernet@30be0000 address not set.
> Error: ethernet@30be0000 address not
> ...
>
> and not able to access console as these errors keep coming in loop.

Fabio,

Looks like this is coming from net/eth-uclass.c with
CONFIG_NET_RANDOM_ETHADDR not defined and would be because there is no
ethaddr env to set the MAC addr?

If I undefine ethaddr on my board and define
CONFIG_NET_RANDOM_ETHADDR=y I get an random MAC that is the same for
all ports and DHCP/networking works but if I then disable
CONFIG_NET_RANDOM_ETHADDR I see the same behavior as you - so it looks
like the lack of a valid MAC addr causes eth_post_probe() to fail by
design?

Tim

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Tim Harvey]

On Mon, Oct 3, 2022 at 11:40 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Mon, Oct 3, 2022 at 3:22 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> > With CONFIG_CMD_NET=y what does 'net list' show?
> >
> > I have the following (with network attached to lan4 port)
> > Net:   MV88E61XX eth0: ethernet@2188000, eth1: lan4, eth2: lan3, eth3: lan2, eth
> > 4: lan1
> > Hit any key to stop autoboot:  0
> > Ventana > print ethaddr
> > ethaddr=00:d0:12:f3:f2:f5
> > Ventana > dm tree
> > ...
> >  ethernet      0  [ + ]   fecmxc                |   |   |-- ethernet@2188000
> >  bootdev       0  [   ]   eth_bootdev           |   |   |   |--
> > ethernet@2188000.bootdev
> >  mdio          0  [ + ]   fec_mdio              |   |   |   `-- mdio
> >  dsa           0  [ + ]   mv88e6xxx             |   |   |       `-- switch@0
> >  ethernet      1  [ + ]   dsa-port              |   |   |           |-- lan4
> >  bootdev       2  [   ]   eth_bootdev           |   |   |           |
> >  `-- switch@0@0.bootdev
> >  ethernet      2  [ + ]   dsa-port              |   |   |           |-- lan3
> >  bootdev       3  [   ]   eth_bootdev           |   |   |           |
> >  `-- switch@0@1.bootdev
> >  ethernet      3  [ + ]   dsa-port              |   |   |           |-- lan2
> >  bootdev       4  [   ]   eth_bootdev           |   |   |           |
> >  `-- switch@0@2.bootdev
> >  ethernet      4  [ + ]   dsa-port              |   |   |           |-- lan1
> >  bootdev       5  [   ]   eth_bootdev           |   |   |           |
> >  `-- switch@0@3.bootdev
> >  mdio          1  [ + ]   mv88e6xxx_mdio        |   |   |
> > `-- mv88e6xxx-mdio-0
> > ...
> > Ventana > net list
> > eth0 : ethernet@2188000 00:d0:12:f3:f2:f5 active
> > eth1 : lan4 00:d0:12:f3:f2:f5
> > eth2 : lan3 00:d0:12:f3:f2:f5
> > eth3 : lan2 00:d0:12:f3:f2:f5
> > eth4 : lan1 00:d0:12:f3:f2:f5
> > Ventana > setenv ethact lan4
> > Ventana > dhcp
> > BOOTP broadcast 1
> > BOOTP broadcast 2
> > BOOTP broadcast 3
> > DHCP client bound to address 172.24.33.142 (1076 ms)
> >
> > I do not use CONFIG_NET_RANDOM_ETHADDR and have a single MAC address
> > allocated to the board.
>
> Below are the logs:
>
> Net:   MV88E61XX
> Warning: ethernet@30be0000 (eth0) using random MAC address - a6:a6:56:1c:28:0a
> eth0: ethernet@30be0000
> Hit any key to stop autoboot:  0
>
> Your log also shows eth1-eth4. Mine does not.
>
> Unlike your log, dsa-port is not getting probed here:
>
>  ethernet      0  [ + ]   fecmxc                |   |   `-- ethernet@30be0000
>  bootdev       2  [   ]   eth_bootdev           |   |       |--
> ethernet@30be0000.bootdev
>  mdio          0  [ + ]   fec_mdio              |   |       `-- mdio
>  dsa           0  [   ]   mv88e6xxx             |   |           `-- switch@0
>  ethernet      1  [   ]   dsa-port              |   |               |-- lan4
>  bootdev       3  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@0.bootdev
>  ethernet      2  [   ]   dsa-port              |   |               |-- lan3
>  bootdev       4  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@1.bootdev
>  ethernet      3  [   ]   dsa-port              |   |               |-- lan2
>  bootdev       5  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@2.bootdev
>  ethernet      4  [   ]   dsa-port              |   |               |-- lan1
>  bootdev       6  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@3.bootdev
>  mdio          1  [ + ]   mv88e6xxx_mdio        |   |
> |-- mv88e6xxx-mdio-0
>  mdio          2  [   ]   mv88e6xxx_mdio        |   |
> |-- mv88e6xxx-mdio-1
>  mdio          3  [   ]   mv88e6xxx_mdio        |   |
> |-- mv88e6xxx-mdio-2
>  mdio          4  [   ]   mv88e6xxx_mdio        |   |
> `-- mv88e6xxx-mdio-3
>  simple_bus    7  [   ]   simple_bus            |   `-- bus@32c00000
>  usb           0  [   ]   ehci_mx6              |       `-- usb@32e40000
>  phy           0  [   ]   nop_phy               |-- usbphynop1
>  regulator     0  [   ]   regulator_fixed       |-- regulator-usdhc2
>  sysreset      1  [   ]   wdt_reboot            |-- wdt-reboot
>  bootstd       0  [   ]   bootstd_drv           `-- bootstd
>  bootmeth      0  [   ]   bootmeth_distro           |-- distro
>  bootmeth      1  [   ]   bootmeth_efi              |-- efi
>  bootmeth      2  [   ]   bootmeth_pxe              |-- pxe
>  bootmeth      3  [   ]   vbe_simple                `-- vbe_simple
>

Right - so it looks like the 'net: fec: add support for DM_MDIO' patch
is not doing its job to probe the mdio bus and ports.

Is CONFIG_DM_MDIO enabled and is dm_fec_bind_mdio returning successfully?

Is CONFIG_DM_ETH_PHY enabled? I don't use that and if I recall that
code never seemed right to me.

Tim

> u-boot=> net list
> eth0 : ethernet@30be0000 a6:a6:56:1c:28:0a active
> eth1 : lan4 00:00:00:00:00:00
> eth2 : lan3 00:00:00:00:00:00
> eth3 : lan2 00:00:00:00:00:00
> eth4 : lan1 00:00:00:00:00:00
> u-boot=> setenv ethact lan4
> u-boot=> dhcp
> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> BOOTP broadcast 4
> BOOTP broadcast 5
> BOOTP broadcast 6
> BOOTP broadcast 7
> BOOTP broadcast 8
> (IP is never retrieved)
>
> > What board are you using and what does its fec/mdio dt look like?
>
> I am using a custom imx8mn board.
>
> The FEC/MDIO nodes are like this:
> https://pastebin.com/raw/0SvLLzxj
>
> Thanks,
>
> Fabio Estevam

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

On 03/10/2022 16:09, Tim Harvey wrote:

> Right - so it looks like the 'net: fec: add support for DM_MDIO' patch
> is not doing its job to probe the mdio bus and ports.
> 
> Is CONFIG_DM_MDIO enabled and is dm_fec_bind_mdio returning 
> successfully?

Yes, for both.


> Is CONFIG_DM_ETH_PHY enabled? I don't use that and if I recall that
> code never seemed right to me.

CONFIG_DM_ETH_PHY is not enabled.

Regards,

Fabio Estevam
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-60 Fax: (+49)-8142-66989-80 Email: 
festevam@denx.de

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Tim Harvey]

On Mon, Oct 3, 2022 at 3:35 PM Fabio Estevam <festevam@denx.de> wrote:
>
> On 03/10/2022 16:09, Tim Harvey wrote:
>
> > Right - so it looks like the 'net: fec: add support for DM_MDIO' patch
> > is not doing its job to probe the mdio bus and ports.
> >
> > Is CONFIG_DM_MDIO enabled and is dm_fec_bind_mdio returning
> > successfully?
>
> Yes, for both.
>
>
> > Is CONFIG_DM_ETH_PHY enabled? I don't use that and if I recall that
> > code never seemed right to me.
>
> CONFIG_DM_ETH_PHY is not enabled.
>

Fabio,

I'm not sure why your configuration is behaving differently and think
you'll have to instrument code to help understand it.

Here is the chain of events I see on my imx6q fec <-> mv88e6085 configuration:
fecmxc_probe() calls dm_fec_bind_mdio() which finds the mdio node in
dt and binds the 'fec_mdio' driver to it then probes that driver. When
it binds via device_bind_driver_to_node() it should end up calling
dsa_post_bind() to bind to the dsa_port driver. Can you verify that
you have DM_DSA enabled and that dsa_post_bind() gets called?

You can enable debug for dsa-uclass.c and perhaps that will explain
more. In dsa_post_bind() there is a loop over pdata->num_ports which
calls device_bind_driver_to_node() to bind the node to
DSA_PORT_CHILD_DRV_NAME - I imagine that isn't happening due to one of
the checks prior to this failing silently.

Tim

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

Hi Tim,

On Mon, Oct 3, 2022 at 8:39 PM Tim Harvey <tharvey@gateworks.com> wrote:

> Fabio,
>
> I'm not sure why your configuration is behaving differently and think
> you'll have to instrument code to help understand it.
>
> Here is the chain of events I see on my imx6q fec <-> mv88e6085 configuration:

The switch I am using is a mv88e6320.

I instrumented the code a bit and I notice that I am not able to read
the switch ID.

mv88e6xxx_get_switch_id() fails because mv88e6xxx_port_read() returns 0xffff.

I am investigating this failure.

Thanks

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

On Mon, Oct 3, 2022 at 8:46 PM Fabio Estevam <festevam@gmail.com> wrote:

> The switch I am using is a mv88e6320.
>
> I instrumented the code a bit and I notice that I am not able to read
> the switch ID.
>
> mv88e6xxx_get_switch_id() fails because mv88e6xxx_port_read() returns 0xffff.

This was caused by the wrong description of the Ethernet switch reset.

With this fixed, dm tree looks a bit better:

 ethernet      0  [ + ]   fecmxc                |   |   `-- ethernet@30be0000
 bootdev       2  [   ]   eth_bootdev           |   |       |--
ethernet@30be0000.bootdev
 mdio          0  [ + ]   fec_mdio              |   |       `-- mdio
 dsa           0  [ + ]   mv88e6xxx             |   |           `-- switch@0
 ethernet      1  [   ]   dsa-port              |   |               |-- lan0
 bootdev       3  [   ]   eth_bootdev           |   |               |
 `-- switch@0@0.bootdev
 ethernet      2  [   ]   dsa-port              |   |               |-- lan1
 bootdev       4  [   ]   eth_bootdev           |   |               |
 `-- switch@0@1.bootdev
 ethernet      3  [ + ]   dsa-port              |   |               |-- lan3
 bootdev       5  [   ]   eth_bootdev           |   |               |
 `-- switch@0@3.bootdev
 mdio          1  [ + ]   mv88e6xxx_mdio        |   |
`-- mv88e6xxx-mdio-0

Boot log:
Net:   MV88E61XX
Warning: ethernet@30be0000 (eth0) using random MAC address - 36:6f:7f:0c:61:14
eth0: ethernet@30be0000Could not get PHY for mv88e6xxx-mdio-0: addr 0
Could not get PHY for mv88e6xxx-mdio-0: addr 1
, eth3: lan3

u-boot=> net list
eth0 : ethernet@30be0000 36:6f:7f:0c:61:14 active
eth1 : lan0 00:00:00:00:00:00
eth2 : lan1 00:00:00:00:00:00
eth3 : lan3 36:6f:7f:0c:61:14

but still not able to get DHCP IP.

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Tim Harvey]

On Mon, Oct 3, 2022 at 5:15 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Mon, Oct 3, 2022 at 8:46 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> > The switch I am using is a mv88e6320.
> >
> > I instrumented the code a bit and I notice that I am not able to read
> > the switch ID.
> >
> > mv88e6xxx_get_switch_id() fails because mv88e6xxx_port_read() returns 0xffff.
>
> This was caused by the wrong description of the Ethernet switch reset.

Seems like something that needs an error print? Which driver this?

>
> With this fixed, dm tree looks a bit better:
>
>  ethernet      0  [ + ]   fecmxc                |   |   `-- ethernet@30be0000
>  bootdev       2  [   ]   eth_bootdev           |   |       |--
> ethernet@30be0000.bootdev
>  mdio          0  [ + ]   fec_mdio              |   |       `-- mdio
>  dsa           0  [ + ]   mv88e6xxx             |   |           `-- switch@0
>  ethernet      1  [   ]   dsa-port              |   |               |-- lan0
>  bootdev       3  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@0.bootdev
>  ethernet      2  [   ]   dsa-port              |   |               |-- lan1
>  bootdev       4  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@1.bootdev
>  ethernet      3  [ + ]   dsa-port              |   |               |-- lan3
>  bootdev       5  [   ]   eth_bootdev           |   |               |
>  `-- switch@0@3.bootdev
>  mdio          1  [ + ]   mv88e6xxx_mdio        |   |
> `-- mv88e6xxx-mdio-0
>
> Boot log:
> Net:   MV88E61XX
> Warning: ethernet@30be0000 (eth0) using random MAC address - 36:6f:7f:0c:61:14
> eth0: ethernet@30be0000Could not get PHY for mv88e6xxx-mdio-0: addr 0
> Could not get PHY for mv88e6xxx-mdio-0: addr 1
> , eth3: lan3

This doesn't look correct - why could it not get the phy for lan1 and
lan2 yet lan3 was fine? I would assume a DT port/reg mismatch.

>
> u-boot=> net list
> eth0 : ethernet@30be0000 36:6f:7f:0c:61:14 active
> eth1 : lan0 00:00:00:00:00:00
> eth2 : lan1 00:00:00:00:00:00
> eth3 : lan3 36:6f:7f:0c:61:14
>
> but still not able to get DHCP IP.

Where are you setting your active port? By default eth0 is active and
thats the cpu uplink. If you have a network connection on lan3 port
you would need to 'setenv ethact lan3'.

Tim

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

Hi Tim,

On Mon, Oct 3, 2022 at 9:23 PM Tim Harvey <tharvey@gateworks.com> wrote:

> Where are you setting your active port? By default eth0 is active and
> thats the cpu uplink. If you have a network connection on lan3 port
> you would need to 'setenv ethact lan3'.

It works now: both 'dhcp' and 'tftp' commands are functional.

Thanks a lot for your work on this and for your help!

If you send a v5 with the CONFIG_MV88E6XXX=y typo fixed,
feel free to add for the whole series:

Reviewed-by: Fabio Estevam <festevam@denx.de>

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Tim Harvey]

On Mon, Oct 3, 2022 at 5:39 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Mon, Oct 3, 2022 at 9:23 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> > Where are you setting your active port? By default eth0 is active and
> > thats the cpu uplink. If you have a network connection on lan3 port
> > you would need to 'setenv ethact lan3'.
>
> It works now: both 'dhcp' and 'tftp' commands are functional.
>
> Thanks a lot for your work on this and for your help!
>
> If you send a v5 with the CONFIG_MV88E6XXX=y typo fixed,
> feel free to add for the whole series:
>
> Reviewed-by: Fabio Estevam <festevam@denx.de>

Fabio,

Thanks for testing this. I'll submit a v5 shortly with your rb tag and
support for 6320:
--- a/drivers/net/mv88e6xxx.c
+++ b/drivers/net/mv88e6xxx.c
@@ -176,6 +176,7 @@
 #define PORT_SWITCH_ID_6220            0x2200
 #define PORT_SWITCH_ID_6240            0x2400
 #define PORT_SWITCH_ID_6250            0x2500
+#define PORT_SWITCH_ID_6320            0x1150
 #define PORT_SWITCH_ID_6352            0x3520

 struct mv88e6xxx_priv {
@@ -792,6 +793,7 @@ static int mv88e6xxx_probe(struct udevice *dev)
        case PORT_SWITCH_ID_6071:
        case PORT_SWITCH_ID_6220:
        case PORT_SWITCH_ID_6250:
+       case PORT_SWITCH_ID_6320:
                priv->port_count = 7;
                priv->phy_ctrl1_en_det_shift = 14;
                priv->phy_ctrl1_en_det_width = 1;


Was there an error path you found in drivers/net/mv88e6xxx.c that
should print an error to explain a failed probe for better future
troubleshooting or was that failure somewhere else? It sounds like
your switch was held in reset such that register reads were failing.

Best Regards,

Tim

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

Hi Tim,

On Tue, Oct 4, 2022 at 12:26 PM Tim Harvey <tharvey@gateworks.com> wrote:

> Fabio,
>
> Thanks for testing this. I'll submit a v5 shortly with your rb tag and
> support for 6320:

Thanks for including the 6320 model!

> Was there an error path you found in drivers/net/mv88e6xxx.c that
> should print an error to explain a failed probe for better future
> troubleshooting or was that failure somewhere else? It sounds like
> your switch was held in reset such that register reads were failing.

Yes, that was the reason for the register read failure.

Initially, I had described the reset GPIO inside using reset-gpios inside
the switch node, just like described in the Linux binding document:
Documentation/devicetree/bindings/net/dsa/marvell.txt

It did not work, so I moved it to phy-reset-gpios inside the fec node.

I don't think this is a blocker for your v5 series. We can improve it
later so that
the binding from Linux could also work in U-Boot.

Thanks

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

Hi Tim,

On Tue, Oct 4, 2022 at 1:25 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Tue, Oct 4, 2022 at 12:26 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> > Fabio,
> >
> > Thanks for testing this. I'll submit a v5 shortly with your rb tag and
> > support for 6320:
>
> Thanks for including the 6320 model!
>
> > Was there an error path you found in drivers/net/mv88e6xxx.c that
> > should print an error to explain a failed probe for better future
> > troubleshooting or was that failure somewhere else? It sounds like

I forgot to reply to this one. Yes, it would help if we print an error like
this:

--- a/drivers/net/mv88e6xxx.c
+++ b/drivers/net/mv88e6xxx.c
@@ -410,8 +410,10 @@ static int mv88e6xxx_get_switch_id(struct udevice *dev)
        int res;

        res = mv88e6xxx_port_read(dev, 0, PORT_REG_SWITCH_ID);
-       if (res < 0)
+       if (res < 0) {
+               dev_err(dev, "Failed to read switch ID: %d\n", res);
                return res;
+       }
        return res & 0xfff0;
 }

Thanks

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Tim Harvey]

On Tue, Oct 4, 2022 at 9:47 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Tue, Oct 4, 2022 at 1:25 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Tim,
> >
> > On Tue, Oct 4, 2022 at 12:26 PM Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > > Fabio,
> > >
> > > Thanks for testing this. I'll submit a v5 shortly with your rb tag and
> > > support for 6320:
> >
> > Thanks for including the 6320 model!
> >
> > > Was there an error path you found in drivers/net/mv88e6xxx.c that
> > > should print an error to explain a failed probe for better future
> > > troubleshooting or was that failure somewhere else? It sounds like
>
> I forgot to reply to this one. Yes, it would help if we print an error like
> this:
>
> --- a/drivers/net/mv88e6xxx.c
> +++ b/drivers/net/mv88e6xxx.c
> @@ -410,8 +410,10 @@ static int mv88e6xxx_get_switch_id(struct udevice *dev)
>         int res;
>
>         res = mv88e6xxx_port_read(dev, 0, PORT_REG_SWITCH_ID);
> -       if (res < 0)
> +       if (res < 0) {
> +               dev_err(dev, "Failed to read switch ID: %d\n", res);
>                 return res;
> +       }
>         return res & 0xfff0;
>  }
>
> Thanks

Fabio,

I just sent a v5 before seeing this. I think all we are waiting for on
this series is for Vladimir to ok the dt changes as I believe I have
finally done the right thing there now.

If I need to do a v6 I'll add this in, otherwise I'll send it after it
gets accepted and merged.

Best Regards,

Tim

Re: [PATCH v4 8/8] board: gw_ventana: enable MV88E61XX DSA support

[Fabio Estevam]

Hi Tim,

On 04/10/2022 13:50, Tim Harvey wrote:

> Fabio,
> 
> I just sent a v5 before seeing this. I think all we are waiting for on
> this series is for Vladimir to ok the dt changes as I believe I have
> finally done the right thing there now.
> 
> If I need to do a v6 I'll add this in, otherwise I'll send it after it
> gets accepted and merged.

That's totally fine. Thanks!