[U-Boot] Switch MVPP2 to use new MVMDIO

[U-Boot] Switch MVPP2 to use new MVMDIO

[nhed+uboot at starry.com]

This patchset includes several commits form Marvell's downstream repo
including Ken Ma's patch to replace the SMI implementation with his
own Marvell MDIO impolementation.  That MVMDIO implementation never
made it to upstream and has since been re-implemented by Alex
Marginean and seems to be in line for inclusion [1].  My last patch in
this set modified Ken's mvpp2 patch to use this new MVMDIO
implementation.  Few other mvpp2 patches picked as they seemed
important yet missing from upstream.

  [1] https://patchwork.ozlabs.org/cover/1136769/
  

[U-Boot] [PATCH 1/7] net: mvpp2x: fix traffic stuck after PHY start error

[nhed+uboot at starry.com]

From: Stefan Chulski <stefanc@marvell.com>

Issue:
- Network stuck if autonegotion fails

Issue root cause:
- During port open procedure if autonegotion fails, configuration of
  packet processor won't be finished and open procedure exits with error.
- However this won't prevent u-boot network framework from
  calling send and receive procedures.
- Using of transmit and receive function of not configured properly
  packet processor will cause traffic stuck.

Fix:
- Don't stop packet processor configuration if autonegotion failed.
  Only error message would be triggered.
- Exit transmit and receive function if there are no PHY link
  indication.
- U-boot network framework would call open procedure during next
  transmit initiation.

Signed-off-by: Stefan Chulski <stefanc@marvell.com>
Reviewed-by: Igal Liberman <igall@marvell.com>
Tested-by: Igal Liberman <igall@marvell.com>
Signed-off-by: Nevo Hed <nhed+github@starry.com>
---

 drivers/net/mvpp2.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
index bd89725e77..f36c8236b1 100644
--- a/drivers/net/mvpp2.c
+++ b/drivers/net/mvpp2.c
@@ -4494,7 +4494,7 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
 		gop_port_enable(port, 0);
 }
 
-static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
+static void mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
 {
 	struct phy_device *phy_dev;
 
@@ -4504,7 +4504,7 @@ static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
 		port->phy_dev = phy_dev;
 		if (!phy_dev) {
 			netdev_err(port->dev, "cannot connect to phy\n");
-			return -ENODEV;
+			return;
 		}
 		phy_dev->supported &= PHY_GBIT_FEATURES;
 		phy_dev->advertising = phy_dev->supported;
@@ -4516,18 +4516,14 @@ static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
 
 		phy_config(phy_dev);
 		phy_startup(phy_dev);
-		if (!phy_dev->link) {
+		if (!phy_dev->link)
 			printf("%s: No link\n", phy_dev->dev->name);
-			return -1;
-		}
-
-		port->init = 1;
+		else
+			port->init = 1;
 	} else {
 		mvpp2_egress_enable(port);
 		mvpp2_ingress_enable(port);
 	}
-
-	return 0;
 }
 
 static int mvpp2_open(struct udevice *dev, struct mvpp2_port *port)
@@ -4567,10 +4563,7 @@ static int mvpp2_open(struct udevice *dev, struct mvpp2_port *port)
 	}
 
 	if (port->phy_node) {
-		err = mvpp2_phy_connect(dev, port);
-		if (err < 0)
-			return err;
-
+		mvpp2_phy_connect(dev, port);
 		mvpp2_link_event(port);
 	} else {
 		mvpp2_egress_enable(port);
@@ -5175,6 +5168,10 @@ static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
 	struct mvpp2_rx_queue *rxq;
 	u8 *data;
 
+	if (port->phy_node)
+		if (!port->phy_dev->link)
+			return 0;
+
 	/* Process RX packets */
 	rxq = port->rxqs[0];
 
@@ -5240,6 +5237,10 @@ static int mvpp2_send(struct udevice *dev, void *packet, int length)
 	int tx_done;
 	int timeout;
 
+	if (port->phy_node)
+		if (!port->phy_dev->link)
+			return 0;
+
 	txq = port->txqs[0];
 	aggr_txq = &port->priv->aggr_txqs[smp_processor_id()];
 
-- 
2.21.0

[U-Boot] [PATCH 2/7] net: mvpp2: Replace SMI implementation with marvell MDIO API

[nhed+uboot at starry.com]

From: Ken Ma <make@marvell.com>

Since marvell MDIO driver is added, SMI function implementation in
mvpp2 driver can be removed and NETA driver can use marvell MDIO API
directly.
This replacement also fixes 2 old issues:
1. Each pp2 port device should have its own mdio bus field member since
   some pp2 ports may use SMI mdio bus while some other pp2 ports may
   use XSMI mdio bus, but the old mvpp2_base device has a shared bus
   for its all pp2 ports; this patch moves mdio bus field member from
   struct mvpp2 to struct mvpp2_port;
2. Old code uses mvpp2_base device name as mdio bus name, but for
   Armada80x0, cp0 ethernet device and cp1 ethernet device have the
   same device name - "ethernet at 0"; and because mdio_register() checks
   unique name, then the second probed mvpp2_base device fails to
   register mdio bus; since new marvell MDIO driver has resolved the
   unique name issue - different mdio names can be set in fdt and if
   a mdio name is not set, the default mdio name will be generated from
   the mdio bus base address, so this issue is fixed by this
   replacement.

Signed-off-by: Ken Ma <make@marvell.com>
Reviewed-by: Igal Liberman <igall@marvell.com>
Tested-by: Igal Liberman <igall@marvell.com>
Signed-off-by: Nevo Hed <nhed+github@starry.com>
---

 drivers/net/Kconfig |   1 +
 drivers/net/mvpp2.c | 158 ++++----------------------------------------
 2 files changed, 13 insertions(+), 146 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 5fd31b03cf..d8c4dd6f4d 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -297,6 +297,7 @@ config MVPP2
 	bool "Marvell Armada 375/7K/8K network interface support"
 	depends on ARMADA_375 || ARMADA_8K
 	select PHYLIB
+	select MVMDIO
 	help
 	  This driver supports the network interface units in the
 	  Marvell ARMADA 375, 7K and 8K SoCs.
diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
index f36c8236b1..b6dfed5c54 100644
--- a/drivers/net/mvpp2.c
+++ b/drivers/net/mvpp2.c
@@ -32,6 +32,7 @@
 #include <linux/mbus.h>
 #include <asm-generic/gpio.h>
 #include <fdt_support.h>
+#include <mdio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -62,8 +63,6 @@ do {									\
 #define MTU			1500
 #define RX_BUFFER_SIZE		(ALIGN(MTU + WRAP, ARCH_DMA_MINALIGN))
 
-#define MVPP2_SMI_TIMEOUT			10000
-
 /* RX Fifo Registers */
 #define MVPP2_RX_DATA_FIFO_SIZE_REG(port)	(0x00 + 4 * (port))
 #define MVPP2_RX_ATTR_FIFO_SIZE_REG(port)	(0x20 + 4 * (port))
@@ -490,23 +489,8 @@ do {									\
 #define MVPP2_QUEUE_NEXT_DESC(q, index) \
 	(((index) < (q)->last_desc) ? ((index) + 1) : 0)
 
-/* SMI: 0xc0054 -> offset 0x54 to lms_base */
-#define MVPP21_SMI				0x0054
 /* PP2.2: SMI: 0x12a200 -> offset 0x1200 to iface_base */
 #define MVPP22_SMI				0x1200
-#define     MVPP2_PHY_REG_MASK			0x1f
-/* SMI register fields */
-#define     MVPP2_SMI_DATA_OFFS			0	/* Data */
-#define     MVPP2_SMI_DATA_MASK			(0xffff << MVPP2_SMI_DATA_OFFS)
-#define     MVPP2_SMI_DEV_ADDR_OFFS		16	/* PHY device address */
-#define     MVPP2_SMI_REG_ADDR_OFFS		21	/* PHY device reg addr*/
-#define     MVPP2_SMI_OPCODE_OFFS		26	/* Write/Read opcode */
-#define     MVPP2_SMI_OPCODE_READ		(1 << MVPP2_SMI_OPCODE_OFFS)
-#define     MVPP2_SMI_READ_VALID		(1 << 27)	/* Read Valid */
-#define     MVPP2_SMI_BUSY			(1 << 28)	/* Busy */
-
-#define     MVPP2_PHY_ADDR_MASK			0x1f
-#define     MVPP2_PHY_REG_MASK			0x1f
 
 /* Additional PPv2.2 offsets */
 #define MVPP22_MPCS				0x007000
@@ -973,7 +957,6 @@ struct mvpp2_port {
 
 	struct phy_device *phy_dev;
 	phy_interface_t phy_interface;
-	int phy_node;
 	int phyaddr;
 	struct mii_dev *bus;
 #ifdef CONFIG_DM_GPIO
@@ -4562,7 +4545,7 @@ static int mvpp2_open(struct udevice *dev, struct mvpp2_port *port)
 		return err;
 	}
 
-	if (port->phy_node) {
+	if (port->phyaddr < PHY_MAX_ADDR) {
 		mvpp2_phy_connect(dev, port);
 		mvpp2_link_event(port);
 	} else {
@@ -4701,6 +4684,7 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
 	u32 id;
 	u32 phyaddr = 0;
 	int phy_mode = -1;
+	int ret;
 
 	/* Default mdio_base from the same eth base */
 	if (port->priv->hw_version == MVPP21)
@@ -4719,17 +4703,12 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
 			dev_err(&pdev->dev, "could not find phy address\n");
 			return -1;
 		}
-
-		phy_ofnode = ofnode_get_parent(offset_to_ofnode(phy_node));
-		phy_base = ofnode_get_addr(phy_ofnode);
-		port->mdio_base = (void *)phy_base;
-
-		if (port->mdio_base < 0) {
-			dev_err(&pdev->dev, "could not find mdio base address\n");
-			return -1;
-		}
+		ret = mdio_mii_bus_get_from_phy(phy_node, &port->bus);
+		if (ret)
+			return ret;
 	} else {
-		phy_node = 0;
+		/* phy_addr is set to invalid value */
+		phyaddr = PHY_MAX_ADDR;
 	}
 
 	phy_mode_str = fdt_getprop(gd->fdt_blob, port_node, "phy-mode", NULL);
@@ -4767,7 +4746,6 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
 		port->first_rxq = port->id * rxq_number;
 	else
 		port->first_rxq = port->id * port->priv->max_port_rxqs;
-	port->phy_node = phy_node;
 	port->phy_interface = phy_mode;
 	port->phyaddr = phyaddr;
 
@@ -5044,119 +5022,7 @@ static int mvpp2_init(struct udevice *dev, struct mvpp2 *priv)
 	return 0;
 }
 
-/* SMI / MDIO functions */
-
-static int smi_wait_ready(struct mvpp2_port *priv)
-{
-	u32 timeout = MVPP2_SMI_TIMEOUT;
-	u32 smi_reg;
-
-	/* wait till the SMI is not busy */
-	do {
-		/* read smi register */
-		smi_reg = readl(priv->mdio_base);
-		if (timeout-- == 0) {
-			printf("Error: SMI busy timeout\n");
-			return -EFAULT;
-		}
-	} while (smi_reg & MVPP2_SMI_BUSY);
-
-	return 0;
-}
-
-/*
- * mpp2_mdio_read - miiphy_read callback function.
- *
- * Returns 16bit phy register value, or 0xffff on error
- */
-static int mpp2_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
-{
-	struct mvpp2_port *priv = bus->priv;
-	u32 smi_reg;
-	u32 timeout;
-
-	/* check parameters */
-	if (addr > MVPP2_PHY_ADDR_MASK) {
-		printf("Error: Invalid PHY address %d\n", addr);
-		return -EFAULT;
-	}
-
-	if (reg > MVPP2_PHY_REG_MASK) {
-		printf("Err: Invalid register offset %d\n", reg);
-		return -EFAULT;
-	}
-
-	/* wait till the SMI is not busy */
-	if (smi_wait_ready(priv) < 0)
-		return -EFAULT;
-
-	/* fill the phy address and regiser offset and read opcode */
-	smi_reg = (addr << MVPP2_SMI_DEV_ADDR_OFFS)
-		| (reg << MVPP2_SMI_REG_ADDR_OFFS)
-		| MVPP2_SMI_OPCODE_READ;
-
-	/* write the smi register */
-	writel(smi_reg, priv->mdio_base);
-
-	/* wait till read value is ready */
-	timeout = MVPP2_SMI_TIMEOUT;
-
-	do {
-		/* read smi register */
-		smi_reg = readl(priv->mdio_base);
-		if (timeout-- == 0) {
-			printf("Err: SMI read ready timeout\n");
-			return -EFAULT;
-		}
-	} while (!(smi_reg & MVPP2_SMI_READ_VALID));
-
-	/* Wait for the data to update in the SMI register */
-	for (timeout = 0; timeout < MVPP2_SMI_TIMEOUT; timeout++)
-		;
-
-	return readl(priv->mdio_base) & MVPP2_SMI_DATA_MASK;
-}
-
-/*
- * mpp2_mdio_write - miiphy_write callback function.
- *
- * Returns 0 if write succeed, -EINVAL on bad parameters
- * -ETIME on timeout
- */
-static int mpp2_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
-			   u16 value)
-{
-	struct mvpp2_port *priv = bus->priv;
-	u32 smi_reg;
-
-	/* check parameters */
-	if (addr > MVPP2_PHY_ADDR_MASK) {
-		printf("Error: Invalid PHY address %d\n", addr);
-		return -EFAULT;
-	}
-
-	if (reg > MVPP2_PHY_REG_MASK) {
-		printf("Err: Invalid register offset %d\n", reg);
-		return -EFAULT;
-	}
-
-	/* wait till the SMI is not busy */
-	if (smi_wait_ready(priv) < 0)
-		return -EFAULT;
-
-	/* fill the phy addr and reg offset and write opcode and data */
-	smi_reg = value << MVPP2_SMI_DATA_OFFS;
-	smi_reg |= (addr << MVPP2_SMI_DEV_ADDR_OFFS)
-		| (reg << MVPP2_SMI_REG_ADDR_OFFS);
-	smi_reg &= ~MVPP2_SMI_OPCODE_READ;
-
-	/* write the smi register */
-	writel(smi_reg, priv->mdio_base);
-
-	return 0;
-}
-
-static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
+int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
 {
 	struct mvpp2_port *port = dev_get_priv(dev);
 	struct mvpp2_rx_desc *rx_desc;
@@ -5168,7 +5034,7 @@ static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
 	struct mvpp2_rx_queue *rxq;
 	u8 *data;
 
-	if (port->phy_node)
+	if (port->phyaddr < PHY_MAX_ADDR)
 		if (!port->phy_dev->link)
 			return 0;
 
@@ -5237,7 +5103,7 @@ static int mvpp2_send(struct udevice *dev, void *packet, int length)
 	int tx_done;
 	int timeout;
 
-	if (port->phy_node)
+	if (port->phyaddr < PHY_MAX_ADDR)
 		if (!port->phy_dev->link)
 			return 0;
 
@@ -5474,7 +5340,7 @@ static int mvpp2_probe(struct udevice *dev)
 			port->gop_id * MVPP22_PORT_OFFSET;
 
 		/* Set phy address of the port */
-		if(port->phy_node)
+		if (port->phyaddr < PHY_MAX_ADDR)
 			mvpp22_smi_phy_addr_cfg(port);
 
 		/* GoP Init */
-- 
2.21.0

[U-Boot] [PATCH 3/7] net: mvpp2: mark phy as invalid in case of missing appropriate driver

[nhed+uboot at starry.com]

From: Grzegorz Jaszczyk <jaz@semihalf.com>

If the phy doesn't match with any existing u-boot drivers the phy
framework will connect it to generic one which uid == 0xffffffff. In
this case act as if the phy wouldn't be declared in dts. Otherwise in
case of 3310 (for which the driver doesn't exist) the link will be
marked as always down. Removing phy entry from dts in case of 3310 is
not good option because it is required for the phy_fw_down procedure.

This patch fixes the issue with the link always down on MCBIN board.

Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Reviewed-by: Igal Liberman <igall@marvell.com>
Tested-by: Igal Liberman <igall@marvell.com>
Signed-off-by: Nevo Hed <nhed+github@starry.com>
---

 drivers/net/mvpp2.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
index b6dfed5c54..fae7090121 100644
--- a/drivers/net/mvpp2.c
+++ b/drivers/net/mvpp2.c
@@ -4484,6 +4484,27 @@ static void mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
 	if (!port->init || port->link == 0) {
 		phy_dev = phy_connect(port->bus, port->phyaddr, dev,
 				      port->phy_interface);
+
+		/* If the phy doesn't match with any existing u-boot drivers the
+		 * phy framework will connect it to generic one which
+		 * uid == 0xffffffff. In this case act as if the phy wouldn't be
+		 * declared in dts. Otherwise in case of 3310 (for which the
+		 * driver doesn't exist) the link will not be correctly
+		 * detected. Removing phy entry from dts in case of 3310 is not
+		 * an option because it is required for the phy_fw_down
+		 * procedure.
+		 */
+		if (phy_dev->drv->uid == 0xffffffff) {/* Generic phy */
+			netdev_warn(port->dev,
+				    "Marking phy as invalid, link will not be checked\n");
+			/* set phy_addr to invalid value */
+			port->phyaddr = PHY_MAX_ADDR;
+			mvpp2_egress_enable(port);
+			mvpp2_ingress_enable(port);
+
+			return;
+		}
+
 		port->phy_dev = phy_dev;
 		if (!phy_dev) {
 			netdev_err(port->dev, "cannot connect to phy\n");
-- 
2.21.0

[U-Boot] [PATCH 4/7] net: mvpp2: no deref null

[nhed+uboot at starry.com]

From: Nevo Hed <nhed+github@starry.com>

phy_dev ptr is set from return of phy_connect() and is used before
test to see if NULL.  Obviously since the test already sxists someoen
made the determination that this NULL is possible.

Signed-off-by: Nevo Hed <nhed+github@starry.com>
---

 drivers/net/mvpp2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
index fae7090121..5f908113f2 100644
--- a/drivers/net/mvpp2.c
+++ b/drivers/net/mvpp2.c
@@ -4494,7 +4494,8 @@ static void mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
 		 * an option because it is required for the phy_fw_down
 		 * procedure.
 		 */
-		if (phy_dev->drv->uid == 0xffffffff) {/* Generic phy */
+		if (phy_dev &&
+		    phy_dev->drv->uid == 0xffffffff) {/* Generic phy */
 			netdev_warn(port->dev,
 				    "Marking phy as invalid, link will not be checked\n");
 			/* set phy_addr to invalid value */
-- 
2.21.0

[U-Boot] [PATCH 5/7] net: mvpp2: allow MDIO registration for fixed links

[nhed+uboot at starry.com]

From: Igal Liberman <igall@marvell.com>

Currently, there are 2 valid cases for interface, PHY
and mdio relation:
  - If an interface has PHY handler, it'll call
    mdio_mii_bus_get_from_phy(), which will register
    MDIO bus.
  - If we want to use fixed-link for an interface,
    PHY handle is not defined in the DTS, and no
    MDIO is registered.

There is a third case, for some boards (with switch),
the MDIO is used for switch configuration, but the interface
itself uses fixed link. This patch allows this option by
checking if fixed-link subnode is defined, in this case,
MDIO bus is registers, but the PHY address is set to
PHY_MAX_ADDR for this interface, so this interface will
not try to access the PHY later on.

In addition, remove unnecessary check after
calling fdtdec_get_int().

This squahses a subsequent fix by same author

Signed-off-by: Igal Liberman <igall@marvell.com>
Reviewed-by: Kostya Porotchkin <kostap@marvell.com>
Reviewed-by: Stefan Chulski <stefanc@marvell.com>
Signed-off-by: Nevo Hed <nhed+github@starry.com>
---

 drivers/net/mvpp2.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
index 5f908113f2..da550450eb 100644
--- a/drivers/net/mvpp2.c
+++ b/drivers/net/mvpp2.c
@@ -4707,6 +4707,7 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
 	u32 phyaddr = 0;
 	int phy_mode = -1;
 	int ret;
+	int fixed_link = 0;
 
 	/* Default mdio_base from the same eth base */
 	if (port->priv->hw_version == MVPP21)
@@ -4715,16 +4716,17 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
 		port->mdio_base = port->priv->iface_base + MVPP22_SMI;
 
 	phy_node = fdtdec_lookup_phandle(gd->fdt_blob, port_node, "phy");
+	fixed_link = fdt_subnode_offset(gd->fdt_blob, port_node, "fixed-link");
 
 	if (phy_node > 0) {
-		ofnode phy_ofnode;
-		fdt_addr_t phy_base;
-
-		phyaddr = fdtdec_get_int(gd->fdt_blob, phy_node, "reg", 0);
-		if (phyaddr < 0) {
-			dev_err(&pdev->dev, "could not find phy address\n");
-			return -1;
+		if (fixed_link != -FDT_ERR_NOTFOUND) {
+			/* phy_addr is set to invalid value for fixed links */
+			phyaddr = PHY_MAX_ADDR;
+		} else {
+			phyaddr = fdtdec_get_int(gd->fdt_blob, phy_node,
+						 "reg", 0);
 		}
+
 		ret = mdio_mii_bus_get_from_phy(phy_node, &port->bus);
 		if (ret)
 			return ret;
-- 
2.21.0

[U-Boot] [PATCH 6/7] arm64: mvebu: armada-cp110-*dtsi: add xmdio nodes

[nhed+uboot at starry.com]

From: Nevo Hed <nhed+github@starry.com>

Based on upstream-linux
See https://github.com/torvalds/linux/commit/f66b2aff.

However made the XSMI register window 0x16 (22) bytes per my reading
of the functional spec.  Similar commits in Marvels own repo bump it
to 0x200 (512) bytes but I did not see the reasoning for that.

https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/commit/4d932b4.

Also added device-name attributes to prevent ambiguity in the `mdio`
command.

Signed-off-by: Nevo Hed <nhed+github@starry.com>
---

 arch/arm/dts/armada-cp110-master.dtsi | 9 +++++++++
 arch/arm/dts/armada-cp110-slave.dtsi  | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm/dts/armada-cp110-master.dtsi b/arch/arm/dts/armada-cp110-master.dtsi
index e4c17e9f4b..cd5c974482 100644
--- a/arch/arm/dts/armada-cp110-master.dtsi
+++ b/arch/arm/dts/armada-cp110-master.dtsi
@@ -99,6 +99,15 @@
 				device-name = "cpm-mdio";
 			};
 
+			cpm_xmdio: mdio at 12a600 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "marvell,xmdio";
+				reg = <0x12a600 0x16>;
+				status = "disabled";
+				device-name = "cpm-xmdio";
+			};
+
 			cpm_syscon0: system-controller at 440000 {
 				compatible = "marvell,cp110-system-controller0",
 					     "syscon";
diff --git a/arch/arm/dts/armada-cp110-slave.dtsi b/arch/arm/dts/armada-cp110-slave.dtsi
index 2fbd7b5514..b426a4eb69 100644
--- a/arch/arm/dts/armada-cp110-slave.dtsi
+++ b/arch/arm/dts/armada-cp110-slave.dtsi
@@ -99,6 +99,15 @@
 				device-name = "cps-mdio";
 			};
 
+			cps_xmdio: mdio at 12a600 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "marvell,xmdio";
+				reg = <0x12a600 0x16>;
+				status = "disabled";
+				device-name = "cps-xmdio";
+			};
+
 			cps_syscon0: system-controller at 440000 {
 				compatible = "marvell,cp110-system-controller0",
 					     "syscon";
-- 
2.21.0

[U-Boot] [PATCH 7/7] net: mvpp2: use new MVMDIO driver

[nhed+uboot at starry.com]

From: Nevo Hed <nhed+github@starry.com>

An eralier commit in this changeset is taken from Marvells repos but was
based on an MVMDIO implementation that never made it into U-Boot.  With
this patch the mvpp2 driver switches to use the new MVMDIO driver that is
based on a more universal mdio-uclass implementation.

Signed-off-by: Nevo Hed <nhed+github@starry.com>
---

 drivers/net/Kconfig |  1 +
 drivers/net/mvpp2.c | 42 ++++++++++--------------------------------
 2 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index d8c4dd6f4d..81f39d0928 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -298,6 +298,7 @@ config MVPP2
 	depends on ARMADA_375 || ARMADA_8K
 	select PHYLIB
 	select MVMDIO
+	select DM_MDIO
 	help
 	  This driver supports the network interface units in the
 	  Marvell ARMADA 375, 7K and 8K SoCs.
diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
index da550450eb..8091a411a4 100644
--- a/drivers/net/mvpp2.c
+++ b/drivers/net/mvpp2.c
@@ -32,7 +32,7 @@
 #include <linux/mbus.h>
 #include <asm-generic/gpio.h>
 #include <fdt_support.h>
-#include <mdio.h>
+#include <linux/mdio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -936,7 +936,6 @@ struct mvpp2_port {
 
 	/* Per-port registers' base address */
 	void __iomem *base;
-	void __iomem *mdio_base;
 
 	struct mvpp2_rx_queue **rxqs;
 	struct mvpp2_tx_queue **txqs;
@@ -958,7 +957,7 @@ struct mvpp2_port {
 	struct phy_device *phy_dev;
 	phy_interface_t phy_interface;
 	int phyaddr;
-	struct mii_dev *bus;
+	struct udevice *mdio_dev;
 #ifdef CONFIG_DM_GPIO
 	struct gpio_desc phy_reset_gpio;
 	struct gpio_desc phy_tx_disable_gpio;
@@ -4482,8 +4481,8 @@ static void mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
 	struct phy_device *phy_dev;
 
 	if (!port->init || port->link == 0) {
-		phy_dev = phy_connect(port->bus, port->phyaddr, dev,
-				      port->phy_interface);
+		phy_dev = dm_mdio_phy_connect(port->mdio_dev, port->phyaddr,
+					      dev, port->phy_interface);
 
 		/* If the phy doesn't match with any existing u-boot drivers the
 		 * phy framework will connect it to generic one which
@@ -4709,16 +4708,11 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
 	int ret;
 	int fixed_link = 0;
 
-	/* Default mdio_base from the same eth base */
-	if (port->priv->hw_version == MVPP21)
-		port->mdio_base = port->priv->lms_base + MVPP21_SMI;
-	else
-		port->mdio_base = port->priv->iface_base + MVPP22_SMI;
-
 	phy_node = fdtdec_lookup_phandle(gd->fdt_blob, port_node, "phy");
 	fixed_link = fdt_subnode_offset(gd->fdt_blob, port_node, "fixed-link");
 
 	if (phy_node > 0) {
+		int parent;
 		if (fixed_link != -FDT_ERR_NOTFOUND) {
 			/* phy_addr is set to invalid value for fixed links */
 			phyaddr = PHY_MAX_ADDR;
@@ -4727,7 +4721,9 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
 						 "reg", 0);
 		}
 
-		ret = mdio_mii_bus_get_from_phy(phy_node, &port->bus);
+		parent = fdt_parent_offset(gd->fdt_blob, phy_node);
+		ret = uclass_get_device_by_of_offset(UCLASS_MDIO, parent,
+						     &port->mdio_dev);
 		if (ret)
 			return ret;
 	} else {
@@ -5046,7 +5042,7 @@ static int mvpp2_init(struct udevice *dev, struct mvpp2 *priv)
 	return 0;
 }
 
-int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
+static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
 {
 	struct mvpp2_port *port = dev_get_priv(dev);
 	struct mvpp2_rx_desc *rx_desc;
@@ -5311,31 +5307,13 @@ static int mvpp2_probe(struct udevice *dev)
 {
 	struct mvpp2_port *port = dev_get_priv(dev);
 	struct mvpp2 *priv = dev_get_priv(dev->parent);
-	struct mii_dev *bus;
 	int err;
 
 	/* Only call the probe function for the parent once */
 	if (!priv->probe_done)
 		err = mvpp2_base_probe(dev->parent);
 
-	port->priv = dev_get_priv(dev->parent);
-
-	/* Create and register the MDIO bus driver */
-	bus = mdio_alloc();
-	if (!bus) {
-		printf("Failed to allocate MDIO bus\n");
-		return -ENOMEM;
-	}
-
-	bus->read = mpp2_mdio_read;
-	bus->write = mpp2_mdio_write;
-	snprintf(bus->name, sizeof(bus->name), dev->name);
-	bus->priv = (void *)port;
-	port->bus = bus;
-
-	err = mdio_register(bus);
-	if (err)
-		return err;
+	port->priv = priv;
 
 	err = phy_info_parse(dev, port);
 	if (err)
-- 
2.21.0

[U-Boot] [PATCH 5/7] net: mvpp2: allow MDIO registration for fixed links

[Simon Goldschmidt]

Am 02.08.2019 um 19:54 schrieb nhed+uboot at starry.com:
> From: Igal Liberman <igall@marvell.com>
> 
> Currently, there are 2 valid cases for interface, PHY
> and mdio relation:
>    - If an interface has PHY handler, it'll call
>      mdio_mii_bus_get_from_phy(), which will register
>      MDIO bus.
>    - If we want to use fixed-link for an interface,
>      PHY handle is not defined in the DTS, and no
>      MDIO is registered.
> 
> There is a third case, for some boards (with switch),
> the MDIO is used for switch configuration, but the interface
> itself uses fixed link. This patch allows this option by
> checking if fixed-link subnode is defined, in this case,
> MDIO bus is registers, but the PHY address is set to
> PHY_MAX_ADDR for this interface, so this interface will
> not try to access the PHY later on.

Instead of ad-hoc coding this, would it work to use "phy_connect()", 
which also checks for fixed-link?

Regards,
Simon

> 
> In addition, remove unnecessary check after
> calling fdtdec_get_int().
> 
> This squahses a subsequent fix by same author
> 
> Signed-off-by: Igal Liberman <igall@marvell.com>
> Reviewed-by: Kostya Porotchkin <kostap@marvell.com>
> Reviewed-by: Stefan Chulski <stefanc@marvell.com>
> Signed-off-by: Nevo Hed <nhed+github@starry.com>
> ---
> 
>   drivers/net/mvpp2.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> index 5f908113f2..da550450eb 100644
> --- a/drivers/net/mvpp2.c
> +++ b/drivers/net/mvpp2.c
> @@ -4707,6 +4707,7 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
>   	u32 phyaddr = 0;
>   	int phy_mode = -1;
>   	int ret;
> +	int fixed_link = 0;
>   
>   	/* Default mdio_base from the same eth base */
>   	if (port->priv->hw_version == MVPP21)
> @@ -4715,16 +4716,17 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
>   		port->mdio_base = port->priv->iface_base + MVPP22_SMI;
>   
>   	phy_node = fdtdec_lookup_phandle(gd->fdt_blob, port_node, "phy");
> +	fixed_link = fdt_subnode_offset(gd->fdt_blob, port_node, "fixed-link");
>   
>   	if (phy_node > 0) {
> -		ofnode phy_ofnode;
> -		fdt_addr_t phy_base;
> -
> -		phyaddr = fdtdec_get_int(gd->fdt_blob, phy_node, "reg", 0);
> -		if (phyaddr < 0) {
> -			dev_err(&pdev->dev, "could not find phy address\n");
> -			return -1;
> +		if (fixed_link != -FDT_ERR_NOTFOUND) {
> +			/* phy_addr is set to invalid value for fixed links */
> +			phyaddr = PHY_MAX_ADDR;
> +		} else {
> +			phyaddr = fdtdec_get_int(gd->fdt_blob, phy_node,
> +						 "reg", 0);
>   		}
> +
>   		ret = mdio_mii_bus_get_from_phy(phy_node, &port->bus);
>   		if (ret)
>   			return ret;
> 

[U-Boot] [PATCH 5/7] net: mvpp2: allow MDIO registration for fixed links

[Nevo Hed]

Thanks Simon
you are correct I should have not selected this commit from the
downstream mvl repo as
this feature of phy_connect is now in upstream (the downstream repo is
2018.12 based)

I will remove and resubmit the set

On Fri, Aug 2, 2019 at 3:01 PM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> Am 02.08.2019 um 19:54 schrieb nhed+uboot at starry.com:
> > From: Igal Liberman <igall@marvell.com>
> >
> > Currently, there are 2 valid cases for interface, PHY
> > and mdio relation:
> >    - If an interface has PHY handler, it'll call
> >      mdio_mii_bus_get_from_phy(), which will register
> >      MDIO bus.
> >    - If we want to use fixed-link for an interface,
> >      PHY handle is not defined in the DTS, and no
> >      MDIO is registered.
> >
> > There is a third case, for some boards (with switch),
> > the MDIO is used for switch configuration, but the interface
> > itself uses fixed link. This patch allows this option by
> > checking if fixed-link subnode is defined, in this case,
> > MDIO bus is registers, but the PHY address is set to
> > PHY_MAX_ADDR for this interface, so this interface will
> > not try to access the PHY later on.
>
> Instead of ad-hoc coding this, would it work to use "phy_connect()",
> which also checks for fixed-link?
>
> Regards,
> Simon
>
> >
> > In addition, remove unnecessary check after
> > calling fdtdec_get_int().
> >
> > This squahses a subsequent fix by same author
> >
> > Signed-off-by: Igal Liberman <igall@marvell.com>
> > Reviewed-by: Kostya Porotchkin <kostap@marvell.com>
> > Reviewed-by: Stefan Chulski <stefanc@marvell.com>
> > Signed-off-by: Nevo Hed <nhed+github@starry.com>
> > ---
> >
> >   drivers/net/mvpp2.c | 16 +++++++++-------
> >   1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> > index 5f908113f2..da550450eb 100644
> > --- a/drivers/net/mvpp2.c
> > +++ b/drivers/net/mvpp2.c
> > @@ -4707,6 +4707,7 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
> >       u32 phyaddr = 0;
> >       int phy_mode = -1;
> >       int ret;
> > +     int fixed_link = 0;
> >
> >       /* Default mdio_base from the same eth base */
> >       if (port->priv->hw_version == MVPP21)
> > @@ -4715,16 +4716,17 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
> >               port->mdio_base = port->priv->iface_base + MVPP22_SMI;
> >
> >       phy_node = fdtdec_lookup_phandle(gd->fdt_blob, port_node, "phy");
> > +     fixed_link = fdt_subnode_offset(gd->fdt_blob, port_node, "fixed-link");
> >
> >       if (phy_node > 0) {
> > -             ofnode phy_ofnode;
> > -             fdt_addr_t phy_base;
> > -
> > -             phyaddr = fdtdec_get_int(gd->fdt_blob, phy_node, "reg", 0);
> > -             if (phyaddr < 0) {
> > -                     dev_err(&pdev->dev, "could not find phy address\n");
> > -                     return -1;
> > +             if (fixed_link != -FDT_ERR_NOTFOUND) {
> > +                     /* phy_addr is set to invalid value for fixed links */
> > +                     phyaddr = PHY_MAX_ADDR;
> > +             } else {
> > +                     phyaddr = fdtdec_get_int(gd->fdt_blob, phy_node,
> > +                                              "reg", 0);
> >               }
> > +
> >               ret = mdio_mii_bus_get_from_phy(phy_node, &port->bus);
> >               if (ret)
> >                       return ret;
> >
>

[U-Boot] [PATCH 1/7] net: mvpp2x: fix traffic stuck after PHY start error

[Joe Hershberger]

On Fri, Aug 2, 2019 at 12:54 PM <nhed+uboot@starry.com> wrote:
>
> From: Stefan Chulski <stefanc@marvell.com>
>
> Issue:
> - Network stuck if autonegotion fails
>
> Issue root cause:
> - During port open procedure if autonegotion fails, configuration of
>   packet processor won't be finished and open procedure exits with error.
> - However this won't prevent u-boot network framework from
>   calling send and receive procedures.
> - Using of transmit and receive function of not configured properly
>   packet processor will cause traffic stuck.
>
> Fix:
> - Don't stop packet processor configuration if autonegotion failed.
>   Only error message would be triggered.
> - Exit transmit and receive function if there are no PHY link
>   indication.
> - U-boot network framework would call open procedure during next
>   transmit initiation.
>
> Signed-off-by: Stefan Chulski <stefanc@marvell.com>
> Reviewed-by: Igal Liberman <igall@marvell.com>
> Tested-by: Igal Liberman <igall@marvell.com>
> Signed-off-by: Nevo Hed <nhed+github@starry.com>
> ---
>
>  drivers/net/mvpp2.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> index bd89725e77..f36c8236b1 100644
> --- a/drivers/net/mvpp2.c
> +++ b/drivers/net/mvpp2.c
> @@ -4494,7 +4494,7 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
>                 gop_port_enable(port, 0);
>  }
>
> -static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
> +static void mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
>  {
>         struct phy_device *phy_dev;
>
> @@ -4504,7 +4504,7 @@ static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
>                 port->phy_dev = phy_dev;
>                 if (!phy_dev) {
>                         netdev_err(port->dev, "cannot connect to phy\n");
> -                       return -ENODEV;
> +                       return;

You need to return something here if you haven't changed the function prototype.

>                 }
>                 phy_dev->supported &= PHY_GBIT_FEATURES;
>                 phy_dev->advertising = phy_dev->supported;
> @@ -4516,18 +4516,14 @@ static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
>
>                 phy_config(phy_dev);
>                 phy_startup(phy_dev);
> -               if (!phy_dev->link) {
> +               if (!phy_dev->link)
>                         printf("%s: No link\n", phy_dev->dev->name);
> -                       return -1;
> -               }
> -
> -               port->init = 1;
> +               else
> +                       port->init = 1;
>         } else {
>                 mvpp2_egress_enable(port);
>                 mvpp2_ingress_enable(port);
>         }
> -
> -       return 0;

Same thing. You need to return an int.

>  }
>
>  static int mvpp2_open(struct udevice *dev, struct mvpp2_port *port)
> @@ -4567,10 +4563,7 @@ static int mvpp2_open(struct udevice *dev, struct mvpp2_port *port)
>         }
>
>         if (port->phy_node) {
> -               err = mvpp2_phy_connect(dev, port);
> -               if (err < 0)
> -                       return err;
> -
> +               mvpp2_phy_connect(dev, port);
>                 mvpp2_link_event(port);
>         } else {
>                 mvpp2_egress_enable(port);
> @@ -5175,6 +5168,10 @@ static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
>         struct mvpp2_rx_queue *rxq;
>         u8 *data;
>
> +       if (port->phy_node)
> +               if (!port->phy_dev->link)
> +                       return 0;
> +
>         /* Process RX packets */
>         rxq = port->rxqs[0];
>
> @@ -5240,6 +5237,10 @@ static int mvpp2_send(struct udevice *dev, void *packet, int length)
>         int tx_done;
>         int timeout;
>
> +       if (port->phy_node)
> +               if (!port->phy_dev->link)
> +                       return 0;
> +
>         txq = port->txqs[0];
>         aggr_txq = &port->priv->aggr_txqs[smp_processor_id()];
>
> --
> 2.21.0
>

[U-Boot] [PATCH 2/7] net: mvpp2: Replace SMI implementation with marvell MDIO API

[Joe Hershberger]

On Fri, Aug 2, 2019 at 12:54 PM <nhed+uboot@starry.com> wrote:
>
> From: Ken Ma <make@marvell.com>
>
> Since marvell MDIO driver is added, SMI function implementation in
> mvpp2 driver can be removed and NETA driver can use marvell MDIO API
> directly.
> This replacement also fixes 2 old issues:
> 1. Each pp2 port device should have its own mdio bus field member since
>    some pp2 ports may use SMI mdio bus while some other pp2 ports may
>    use XSMI mdio bus, but the old mvpp2_base device has a shared bus
>    for its all pp2 ports; this patch moves mdio bus field member from
>    struct mvpp2 to struct mvpp2_port;
> 2. Old code uses mvpp2_base device name as mdio bus name, but for
>    Armada80x0, cp0 ethernet device and cp1 ethernet device have the
>    same device name - "ethernet at 0"; and because mdio_register() checks
>    unique name, then the second probed mvpp2_base device fails to
>    register mdio bus; since new marvell MDIO driver has resolved the
>    unique name issue - different mdio names can be set in fdt and if
>    a mdio name is not set, the default mdio name will be generated from
>    the mdio bus base address, so this issue is fixed by this
>    replacement.
>
> Signed-off-by: Ken Ma <make@marvell.com>
> Reviewed-by: Igal Liberman <igall@marvell.com>
> Tested-by: Igal Liberman <igall@marvell.com>
> Signed-off-by: Nevo Hed <nhed+github@starry.com>
> ---
>
>  drivers/net/Kconfig |   1 +
>  drivers/net/mvpp2.c | 158 ++++----------------------------------------
>  2 files changed, 13 insertions(+), 146 deletions(-)
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 5fd31b03cf..d8c4dd6f4d 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -297,6 +297,7 @@ config MVPP2
>         bool "Marvell Armada 375/7K/8K network interface support"
>         depends on ARMADA_375 || ARMADA_8K
>         select PHYLIB
> +       select MVMDIO
>         help
>           This driver supports the network interface units in the
>           Marvell ARMADA 375, 7K and 8K SoCs.
> diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> index f36c8236b1..b6dfed5c54 100644
> --- a/drivers/net/mvpp2.c
> +++ b/drivers/net/mvpp2.c
> @@ -32,6 +32,7 @@
>  #include <linux/mbus.h>
>  #include <asm-generic/gpio.h>
>  #include <fdt_support.h>
> +#include <mdio.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -62,8 +63,6 @@ do {                                                                  \
>  #define MTU                    1500
>  #define RX_BUFFER_SIZE         (ALIGN(MTU + WRAP, ARCH_DMA_MINALIGN))
>
> -#define MVPP2_SMI_TIMEOUT                      10000
> -
>  /* RX Fifo Registers */
>  #define MVPP2_RX_DATA_FIFO_SIZE_REG(port)      (0x00 + 4 * (port))
>  #define MVPP2_RX_ATTR_FIFO_SIZE_REG(port)      (0x20 + 4 * (port))
> @@ -490,23 +489,8 @@ do {                                                                       \
>  #define MVPP2_QUEUE_NEXT_DESC(q, index) \
>         (((index) < (q)->last_desc) ? ((index) + 1) : 0)
>
> -/* SMI: 0xc0054 -> offset 0x54 to lms_base */
> -#define MVPP21_SMI                             0x0054
>  /* PP2.2: SMI: 0x12a200 -> offset 0x1200 to iface_base */
>  #define MVPP22_SMI                             0x1200
> -#define     MVPP2_PHY_REG_MASK                 0x1f
> -/* SMI register fields */
> -#define     MVPP2_SMI_DATA_OFFS                        0       /* Data */
> -#define     MVPP2_SMI_DATA_MASK                        (0xffff << MVPP2_SMI_DATA_OFFS)
> -#define     MVPP2_SMI_DEV_ADDR_OFFS            16      /* PHY device address */
> -#define     MVPP2_SMI_REG_ADDR_OFFS            21      /* PHY device reg addr*/
> -#define     MVPP2_SMI_OPCODE_OFFS              26      /* Write/Read opcode */
> -#define     MVPP2_SMI_OPCODE_READ              (1 << MVPP2_SMI_OPCODE_OFFS)
> -#define     MVPP2_SMI_READ_VALID               (1 << 27)       /* Read Valid */
> -#define     MVPP2_SMI_BUSY                     (1 << 28)       /* Busy */
> -
> -#define     MVPP2_PHY_ADDR_MASK                        0x1f
> -#define     MVPP2_PHY_REG_MASK                 0x1f
>
>  /* Additional PPv2.2 offsets */
>  #define MVPP22_MPCS                            0x007000
> @@ -973,7 +957,6 @@ struct mvpp2_port {
>
>         struct phy_device *phy_dev;
>         phy_interface_t phy_interface;
> -       int phy_node;
>         int phyaddr;
>         struct mii_dev *bus;
>  #ifdef CONFIG_DM_GPIO
> @@ -4562,7 +4545,7 @@ static int mvpp2_open(struct udevice *dev, struct mvpp2_port *port)
>                 return err;
>         }
>
> -       if (port->phy_node) {
> +       if (port->phyaddr < PHY_MAX_ADDR) {
>                 mvpp2_phy_connect(dev, port);
>                 mvpp2_link_event(port);
>         } else {
> @@ -4701,6 +4684,7 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
>         u32 id;
>         u32 phyaddr = 0;
>         int phy_mode = -1;
> +       int ret;
>
>         /* Default mdio_base from the same eth base */
>         if (port->priv->hw_version == MVPP21)
> @@ -4719,17 +4703,12 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
>                         dev_err(&pdev->dev, "could not find phy address\n");
>                         return -1;
>                 }
> -
> -               phy_ofnode = ofnode_get_parent(offset_to_ofnode(phy_node));
> -               phy_base = ofnode_get_addr(phy_ofnode);
> -               port->mdio_base = (void *)phy_base;
> -
> -               if (port->mdio_base < 0) {
> -                       dev_err(&pdev->dev, "could not find mdio base address\n");
> -                       return -1;
> -               }
> +               ret = mdio_mii_bus_get_from_phy(phy_node, &port->bus);
> +               if (ret)
> +                       return ret;
>         } else {
> -               phy_node = 0;
> +               /* phy_addr is set to invalid value */
> +               phyaddr = PHY_MAX_ADDR;
>         }
>
>         phy_mode_str = fdt_getprop(gd->fdt_blob, port_node, "phy-mode", NULL);
> @@ -4767,7 +4746,6 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
>                 port->first_rxq = port->id * rxq_number;
>         else
>                 port->first_rxq = port->id * port->priv->max_port_rxqs;
> -       port->phy_node = phy_node;
>         port->phy_interface = phy_mode;
>         port->phyaddr = phyaddr;
>
> @@ -5044,119 +5022,7 @@ static int mvpp2_init(struct udevice *dev, struct mvpp2 *priv)
>         return 0;
>  }
>
> -/* SMI / MDIO functions */
> -
> -static int smi_wait_ready(struct mvpp2_port *priv)
> -{
> -       u32 timeout = MVPP2_SMI_TIMEOUT;
> -       u32 smi_reg;
> -
> -       /* wait till the SMI is not busy */
> -       do {
> -               /* read smi register */
> -               smi_reg = readl(priv->mdio_base);
> -               if (timeout-- == 0) {
> -                       printf("Error: SMI busy timeout\n");
> -                       return -EFAULT;
> -               }
> -       } while (smi_reg & MVPP2_SMI_BUSY);
> -
> -       return 0;
> -}
> -
> -/*
> - * mpp2_mdio_read - miiphy_read callback function.
> - *
> - * Returns 16bit phy register value, or 0xffff on error
> - */
> -static int mpp2_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
> -{
> -       struct mvpp2_port *priv = bus->priv;
> -       u32 smi_reg;
> -       u32 timeout;
> -
> -       /* check parameters */
> -       if (addr > MVPP2_PHY_ADDR_MASK) {
> -               printf("Error: Invalid PHY address %d\n", addr);
> -               return -EFAULT;
> -       }
> -
> -       if (reg > MVPP2_PHY_REG_MASK) {
> -               printf("Err: Invalid register offset %d\n", reg);
> -               return -EFAULT;
> -       }
> -
> -       /* wait till the SMI is not busy */
> -       if (smi_wait_ready(priv) < 0)
> -               return -EFAULT;
> -
> -       /* fill the phy address and regiser offset and read opcode */
> -       smi_reg = (addr << MVPP2_SMI_DEV_ADDR_OFFS)
> -               | (reg << MVPP2_SMI_REG_ADDR_OFFS)
> -               | MVPP2_SMI_OPCODE_READ;
> -
> -       /* write the smi register */
> -       writel(smi_reg, priv->mdio_base);
> -
> -       /* wait till read value is ready */
> -       timeout = MVPP2_SMI_TIMEOUT;
> -
> -       do {
> -               /* read smi register */
> -               smi_reg = readl(priv->mdio_base);
> -               if (timeout-- == 0) {
> -                       printf("Err: SMI read ready timeout\n");
> -                       return -EFAULT;
> -               }
> -       } while (!(smi_reg & MVPP2_SMI_READ_VALID));
> -
> -       /* Wait for the data to update in the SMI register */
> -       for (timeout = 0; timeout < MVPP2_SMI_TIMEOUT; timeout++)
> -               ;
> -
> -       return readl(priv->mdio_base) & MVPP2_SMI_DATA_MASK;
> -}
> -
> -/*
> - * mpp2_mdio_write - miiphy_write callback function.
> - *
> - * Returns 0 if write succeed, -EINVAL on bad parameters
> - * -ETIME on timeout
> - */
> -static int mpp2_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
> -                          u16 value)
> -{
> -       struct mvpp2_port *priv = bus->priv;
> -       u32 smi_reg;
> -
> -       /* check parameters */
> -       if (addr > MVPP2_PHY_ADDR_MASK) {
> -               printf("Error: Invalid PHY address %d\n", addr);
> -               return -EFAULT;
> -       }
> -
> -       if (reg > MVPP2_PHY_REG_MASK) {
> -               printf("Err: Invalid register offset %d\n", reg);
> -               return -EFAULT;
> -       }
> -
> -       /* wait till the SMI is not busy */
> -       if (smi_wait_ready(priv) < 0)
> -               return -EFAULT;
> -
> -       /* fill the phy addr and reg offset and write opcode and data */
> -       smi_reg = value << MVPP2_SMI_DATA_OFFS;
> -       smi_reg |= (addr << MVPP2_SMI_DEV_ADDR_OFFS)
> -               | (reg << MVPP2_SMI_REG_ADDR_OFFS);
> -       smi_reg &= ~MVPP2_SMI_OPCODE_READ;
> -
> -       /* write the smi register */
> -       writel(smi_reg, priv->mdio_base);
> -
> -       return 0;
> -}
> -
> -static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
> +int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)

Why does this need to be visible outside of this translation unit?

>  {
>         struct mvpp2_port *port = dev_get_priv(dev);
>         struct mvpp2_rx_desc *rx_desc;
> @@ -5168,7 +5034,7 @@ static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
>         struct mvpp2_rx_queue *rxq;
>         u8 *data;
>
> -       if (port->phy_node)
> +       if (port->phyaddr < PHY_MAX_ADDR)
>                 if (!port->phy_dev->link)
>                         return 0;
>
> @@ -5237,7 +5103,7 @@ static int mvpp2_send(struct udevice *dev, void *packet, int length)
>         int tx_done;
>         int timeout;
>
> -       if (port->phy_node)
> +       if (port->phyaddr < PHY_MAX_ADDR)
>                 if (!port->phy_dev->link)
>                         return 0;
>
> @@ -5474,7 +5340,7 @@ static int mvpp2_probe(struct udevice *dev)
>                         port->gop_id * MVPP22_PORT_OFFSET;
>
>                 /* Set phy address of the port */
> -               if(port->phy_node)
> +               if (port->phyaddr < PHY_MAX_ADDR)
>                         mvpp22_smi_phy_addr_cfg(port);
>
>                 /* GoP Init */
> --
> 2.21.0
>

[U-Boot] [PATCH 3/7] net: mvpp2: mark phy as invalid in case of missing appropriate driver

[Joe Hershberger]

On Fri, Aug 2, 2019 at 12:54 PM <nhed+uboot@starry.com> wrote:
>
> From: Grzegorz Jaszczyk <jaz@semihalf.com>
>
> If the phy doesn't match with any existing u-boot drivers the phy
> framework will connect it to generic one which uid == 0xffffffff. In
> this case act as if the phy wouldn't be declared in dts. Otherwise in
> case of 3310 (for which the driver doesn't exist) the link will be
> marked as always down. Removing phy entry from dts in case of 3310 is
> not good option because it is required for the phy_fw_down procedure.
>
> This patch fixes the issue with the link always down on MCBIN board.
>
> Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> Reviewed-by: Igal Liberman <igall@marvell.com>
> Tested-by: Igal Liberman <igall@marvell.com>
> Signed-off-by: Nevo Hed <nhed+github@starry.com>
> ---
>
>  drivers/net/mvpp2.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> index b6dfed5c54..fae7090121 100644
> --- a/drivers/net/mvpp2.c
> +++ b/drivers/net/mvpp2.c
> @@ -4484,6 +4484,27 @@ static void mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
>         if (!port->init || port->link == 0) {
>                 phy_dev = phy_connect(port->bus, port->phyaddr, dev,
>                                       port->phy_interface);
> +
> +               /* If the phy doesn't match with any existing u-boot drivers the

Please use the project multi-line comment format. First line should be just: /*

> +                * phy framework will connect it to generic one which
> +                * uid == 0xffffffff. In this case act as if the phy wouldn't be
> +                * declared in dts. Otherwise in case of 3310 (for which the
> +                * driver doesn't exist) the link will not be correctly
> +                * detected. Removing phy entry from dts in case of 3310 is not
> +                * an option because it is required for the phy_fw_down
> +                * procedure.
> +                */
> +               if (phy_dev->drv->uid == 0xffffffff) {/* Generic phy */
> +                       netdev_warn(port->dev,
> +                                   "Marking phy as invalid, link will not be checked\n");
> +                       /* set phy_addr to invalid value */
> +                       port->phyaddr = PHY_MAX_ADDR;
> +                       mvpp2_egress_enable(port);
> +                       mvpp2_ingress_enable(port);
> +
> +                       return;
> +               }
> +
>                 port->phy_dev = phy_dev;
>                 if (!phy_dev) {
>                         netdev_err(port->dev, "cannot connect to phy\n");
> --
> 2.21.0
>

[U-Boot] [PATCH 4/7] net: mvpp2: no deref null

[Joe Hershberger]

On Fri, Aug 2, 2019 at 12:54 PM <nhed+uboot@starry.com> wrote:
>
> From: Nevo Hed <nhed+github@starry.com>
>
> phy_dev ptr is set from return of phy_connect() and is used before
> test to see if NULL.  Obviously since the test already sxists someoen
> made the determination that this NULL is possible.
>
> Signed-off-by: Nevo Hed <nhed+github@starry.com>
> ---
>
>  drivers/net/mvpp2.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> index fae7090121..5f908113f2 100644
> --- a/drivers/net/mvpp2.c
> +++ b/drivers/net/mvpp2.c
> @@ -4494,7 +4494,8 @@ static void mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
>                  * an option because it is required for the phy_fw_down
>                  * procedure.
>                  */
> -               if (phy_dev->drv->uid == 0xffffffff) {/* Generic phy */
> +               if (phy_dev &&
> +                   phy_dev->drv->uid == 0xffffffff) {/* Generic phy */

This should be squashed into the previous patch.

>                         netdev_warn(port->dev,
>                                     "Marking phy as invalid, link will not be checked\n");
>                         /* set phy_addr to invalid value */
> --
> 2.21.0
>

[U-Boot] [PATCH 1/7] net: mvpp2x: fix traffic stuck after PHY start error

[Joe Hershberger]

On Wed, Aug 14, 2019 at 10:36 AM Joe Hershberger
<joe.hershberger@gmail.com> wrote:
>
> On Fri, Aug 2, 2019 at 12:54 PM <nhed+uboot@starry.com> wrote:
> >
> > From: Stefan Chulski <stefanc@marvell.com>
> >
> > Issue:
> > - Network stuck if autonegotion fails
> >
> > Issue root cause:
> > - During port open procedure if autonegotion fails, configuration of
> >   packet processor won't be finished and open procedure exits with error.
> > - However this won't prevent u-boot network framework from
> >   calling send and receive procedures.
> > - Using of transmit and receive function of not configured properly
> >   packet processor will cause traffic stuck.
> >
> > Fix:
> > - Don't stop packet processor configuration if autonegotion failed.
> >   Only error message would be triggered.
> > - Exit transmit and receive function if there are no PHY link
> >   indication.
> > - U-boot network framework would call open procedure during next
> >   transmit initiation.
> >
> > Signed-off-by: Stefan Chulski <stefanc@marvell.com>
> > Reviewed-by: Igal Liberman <igall@marvell.com>
> > Tested-by: Igal Liberman <igall@marvell.com>
> > Signed-off-by: Nevo Hed <nhed+github@starry.com>
> > ---
> >
> >  drivers/net/mvpp2.c | 27 ++++++++++++++-------------
> >  1 file changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> > index bd89725e77..f36c8236b1 100644
> > --- a/drivers/net/mvpp2.c
> > +++ b/drivers/net/mvpp2.c
> > @@ -4494,7 +4494,7 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
> >                 gop_port_enable(port, 0);
> >  }
> >
> > -static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
> > +static void mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)

[2] - ^^^^

> >  {
> >         struct phy_device *phy_dev;
> >
> > @@ -4504,7 +4504,7 @@ static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)

[1] - ^^^^

> >                 port->phy_dev = phy_dev;
> >                 if (!phy_dev) {
> >                         netdev_err(port->dev, "cannot connect to phy\n");
> > -                       return -ENODEV;
> > +                       return;
>
> You need to return something here if you haven't changed the function prototype.

Apologies... my eyes went to [1] and not [2].

>
> >                 }
> >                 phy_dev->supported &= PHY_GBIT_FEATURES;
> >                 phy_dev->advertising = phy_dev->supported;
> > @@ -4516,18 +4516,14 @@ static int mvpp2_phy_connect(struct udevice *dev, struct mvpp2_port *port)
> >
> >                 phy_config(phy_dev);
> >                 phy_startup(phy_dev);
> > -               if (!phy_dev->link) {
> > +               if (!phy_dev->link)
> >                         printf("%s: No link\n", phy_dev->dev->name);
> > -                       return -1;
> > -               }
> > -
> > -               port->init = 1;
> > +               else
> > +                       port->init = 1;
> >         } else {
> >                 mvpp2_egress_enable(port);
> >                 mvpp2_ingress_enable(port);
> >         }
> > -
> > -       return 0;
>
> Same thing. You need to return an int.
>
> >  }
> >
> >  static int mvpp2_open(struct udevice *dev, struct mvpp2_port *port)
> > @@ -4567,10 +4563,7 @@ static int mvpp2_open(struct udevice *dev, struct mvpp2_port *port)
> >         }
> >
> >         if (port->phy_node) {
> > -               err = mvpp2_phy_connect(dev, port);
> > -               if (err < 0)
> > -                       return err;
> > -
> > +               mvpp2_phy_connect(dev, port);
> >                 mvpp2_link_event(port);
> >         } else {
> >                 mvpp2_egress_enable(port);
> > @@ -5175,6 +5168,10 @@ static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
> >         struct mvpp2_rx_queue *rxq;
> >         u8 *data;
> >
> > +       if (port->phy_node)
> > +               if (!port->phy_dev->link)
> > +                       return 0;
> > +
> >         /* Process RX packets */
> >         rxq = port->rxqs[0];
> >
> > @@ -5240,6 +5237,10 @@ static int mvpp2_send(struct udevice *dev, void *packet, int length)
> >         int tx_done;
> >         int timeout;
> >
> > +       if (port->phy_node)
> > +               if (!port->phy_dev->link)
> > +                       return 0;
> > +
> >         txq = port->txqs[0];
> >         aggr_txq = &port->priv->aggr_txqs[smp_processor_id()];
> >
> > --
> > 2.21.0
> >

[U-Boot] [PATCH 2/7] net: mvpp2: Replace SMI implementation with marvell MDIO API

[Nevo Hed]

On Wed, Aug 14, 2019 at 11:39 AM Joe Hershberger
<joe.hershberger@gmail.com> wrote:
>
> On Fri, Aug 2, 2019 at 12:54 PM <nhed+uboot@starry.com> wrote:
> >
> > From: Ken Ma <make@marvell.com>
> >

> > -
> > -static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
> > +int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
>
> Why does this need to be visible outside of this translation unit?

Hi Joe

Yes you are right, it should remain static.  This was in the
downstream commit from Ken I pulled in - I reversed that and made it
static again in my later commit in same patchset

https://patchwork.ozlabs.org/patch/1142905/

let me know if it is preferred to rewrite this commit  (I just don't
know what the right way when submitting changes by others where the
others' org instructed them to abandoned them)

[U-Boot] [PATCH 2/7] net: mvpp2: Replace SMI implementation with marvell MDIO API

[Joe Hershberger]

On Wed, Aug 14, 2019 at 12:09 PM Nevo Hed <nhed+uboot@starry.com> wrote:
>
> On Wed, Aug 14, 2019 at 11:39 AM Joe Hershberger
> <joe.hershberger@gmail.com> wrote:
> >
> > On Fri, Aug 2, 2019 at 12:54 PM <nhed+uboot@starry.com> wrote:
> > >
> > > From: Ken Ma <make@marvell.com>
> > >
>
> > > -
> > > -static int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
> > > +int mvpp2_recv(struct udevice *dev, int flags, uchar **packetp)
> >
> > Why does this need to be visible outside of this translation unit?
>
> Hi Joe
>
> Yes you are right, it should remain static.  This was in the
> downstream commit from Ken I pulled in - I reversed that and made it
> static again in my later commit in same patchset
>
> https://patchwork.ozlabs.org/patch/1142905/
>
> let me know if it is preferred to rewrite this commit  (I just don't
> know what the right way when submitting changes by others where the
> others' org instructed them to abandoned them)

When it is something minor like this, it's reasonable to simply make
the small adjustment and a brief comment in the commit log and leave
the author attribution.

Cheers,
-Joe