[PATCH net 0/3] net: phy: bcm7xxx initial read/write workaround

[PATCH net 0/3] net: phy: bcm7xxx initial read/write workaround

[Florian Fainelli]

Hi David,

This patch series fixes occasional BCM7xxx PHY driver binding failure due
to a harware bug where the first read or write does not come out of the PHY
MDIO management controller.

Since we have two different MDIO controllers using this PHY, a similar
need to be replicated in GENET and UniMAC MDIO.

Thanks!

Florian Fainelli (3):
  net: phy: bcm7xxx: workaround MDIO management controller initial read
  net: bcmgenet: workaround initial read failures for integrated PHYs
  net: phy: mdio-bcm-unimac: workaround initial read failures for
    integrated PHYs

 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 54 ++++++++++++++++++++++++--
 drivers/net/phy/bcm7xxx.c                      |  7 ++++
 drivers/net/phy/mdio-bcm-unimac.c              | 43 ++++++++++++++++++++
 4 files changed, 101 insertions(+), 4 deletions(-)

-- 
2.1.0

[PATCH net] net: bcmgenet: power on MII block for all MII modes

[Florian Fainelli]

The RGMII block is currently only powered on when using RGMII or
RGMII_NO_ID, which is not correct when using the GENET interface in MII
or Reverse MII modes. We always need to power on the RGMII interface for
this block to properly work, regardless of the MII mode in which we
operate.

Fixes: aa09677cba423 ("net: bcmgenet: add MDIO routines")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
David,

I am targetting "net" here since this is a bug fix, however, we have few
people using MII or Reverse MII with GENET, such that there is no need to
queue this for -stable unless you want to.

Thanks!

 drivers/net/ethernet/broadcom/genet/bcmmii.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index e7651b3c6c57..420949cc55aa 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -299,9 +299,6 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 			phy_name = "external RGMII (no delay)";
 		else
 			phy_name = "external RGMII (TX delay)";
-		reg = bcmgenet_ext_readl(priv, EXT_RGMII_OOB_CTRL);
-		reg |= RGMII_MODE_EN | id_mode_dis;
-		bcmgenet_ext_writel(priv, reg, EXT_RGMII_OOB_CTRL);
 		bcmgenet_sys_writel(priv,
 				    PORT_MODE_EXT_GPHY, SYS_PORT_CTRL);
 		break;
@@ -310,6 +307,15 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 		return -EINVAL;
 	}
 
+	/* This is an external PHY (xMII), so we need to enable the RGMII
+	 * block for the interface to work
+	 */
+	if (priv->ext_phy) {
+		reg = bcmgenet_ext_readl(priv, EXT_RGMII_OOB_CTRL);
+		reg |= RGMII_MODE_EN | id_mode_dis;
+		bcmgenet_ext_writel(priv, reg, EXT_RGMII_OOB_CTRL);
+	}
+
 	if (init)
 		dev_info(kdev, "configuring instance for %s\n", phy_name);
 
-- 
2.1.0

Re: [PATCH net] net: bcmgenet: power on MII block for all MII modes

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 26 Jun 2015 10:38:31 -0700

> The RGMII block is currently only powered on when using RGMII or
> RGMII_NO_ID, which is not correct when using the GENET interface in MII
> or Reverse MII modes. We always need to power on the RGMII interface for
> this block to properly work, regardless of the MII mode in which we
> operate.
> 
> Fixes: aa09677cba423 ("net: bcmgenet: add MDIO routines")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> David,
> 
> I am targetting "net" here since this is a bug fix, however, we have few
> people using MII or Reverse MII with GENET, such that there is no need to
> queue this for -stable unless you want to.

This does not apply cleanly to the net tree, please respin.

Re: [PATCH net] net: bcmgenet: power on MII block for all MII modes

[Florian Fainelli]

On 28/06/15 20:27, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Fri, 26 Jun 2015 10:38:31 -0700
> 
>> The RGMII block is currently only powered on when using RGMII or
>> RGMII_NO_ID, which is not correct when using the GENET interface in MII
>> or Reverse MII modes. We always need to power on the RGMII interface for
>> this block to properly work, regardless of the MII mode in which we
>> operate.
>>
>> Fixes: aa09677cba423 ("net: bcmgenet: add MDIO routines")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> David,
>>
>> I am targetting "net" here since this is a bug fix, however, we have few
>> people using MII or Reverse MII with GENET, such that there is no need to
>> queue this for -stable unless you want to.
> 
> This does not apply cleanly to the net tree, please respin.

You can discard this patch, it was applied earlier and was mistakenly
sent again. Thanks!
-- 
Florian

[PATCH net 1/3] net: phy: bcm7xxx: workaround MDIO management controller initial read

[Florian Fainelli]

The initial MDIO read or write towards the BCM7xxx integrated PHY may
fail, workaround this by inserting a dummy MII_BMSR read to force the
MDIO management controller to see at least one valid transaction and get
out of stuck state out of reset.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/bcm7xxx.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 4dea85bfc545..6b701b3ded74 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -246,6 +246,13 @@ static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
 	pr_info_once("%s: %s PHY revision: 0x%02x, patch: %d\n",
 		     dev_name(&phydev->dev), phydev->drv->name, rev, patch);
 
+	/* Dummy read to a register to workaround an issue upon reset where the
+	 * internal inverter may not allow the first MDIO transaction to pass
+	 * the MDIO management controller and make us return 0xffff for such
+	 * reads.
+	 */
+	phy_read(phydev, MII_BMSR);
+
 	switch (rev) {
 	case 0xb0:
 		ret = bcm7xxx_28nm_b0_afe_config_init(phydev);
-- 
2.1.0

[PATCH net 2/3] net: bcmgenet: workaround initial read failures for integrated PHYs

[Florian Fainelli]

All BCM7xxx integrated Gigabit PHYs have an issue in their MDIO
management controller which will make the initial read or write to them
to fail and return 0xffff. This is a real issue as the typical first
thing we do is read from MII_PHYSID1 and MII_PHYSID2 from get_phy_id()
to register a driver for these PHYs.

Coupled with the workaround in drivers/net/phy/bcm7xxx.c, this
workaround for the MDIO bus controller consists in scanning the list of
PHYs to do this initial read workaround for as part of the MDIO bus
reset routine which is invoked prior to mdiobus_scan().

Once we have a proper PHY driver/device registered, all workarounds are
located there (e.g: power management suspend/resume calls).

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 54 ++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 6f2887a5e0be..6159deab8c98 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -594,6 +594,7 @@ struct bcmgenet_priv {
 	wait_queue_head_t wq;
 	struct phy_device *phydev;
 	struct device_node *phy_dn;
+	struct device_node *mdio_dn;
 	struct mii_bus *mii_bus;
 	u16 gphy_rev;
 	struct clk *clk_eee;
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 6bef04e2f735..adf23d2ac488 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -408,6 +408,52 @@ static int bcmgenet_mii_probe(struct net_device *dev)
 	return 0;
 }
 
+/* Workaround for integrated BCM7xxx Gigabit PHYs which have a problem with
+ * their internal MDIO management controller making them fail to successfully
+ * be read from or written to for the first transaction.  We insert a dummy
+ * BMSR read here to make sure that phy_get_device() and get_phy_id() can
+ * correctly read the PHY MII_PHYSID1/2 registers and successfully register a
+ * PHY device for this peripheral.
+ *
+ * Once the PHY driver is registered, we can workaround subsequent reads from
+ * there (e.g: during system-wide power management).
+ *
+ * bus->reset is invoked before mdiobus_scan during mdiobus_register and is
+ * therefore the right location to stick that workaround. Since we do not want
+ * to read from non-existing PHYs, we either use bus->phy_mask or do a manual
+ * Device Tree scan to limit the search area.
+ */
+static int bcmgenet_mii_bus_reset(struct mii_bus *bus)
+{
+	struct net_device *dev = bus->priv;
+	struct bcmgenet_priv *priv = netdev_priv(dev);
+	struct device_node *np = priv->mdio_dn;
+	struct device_node *child = NULL;
+	u32 read_mask = 0;
+	int addr = 0;
+
+	if (!np) {
+		read_mask = 1 << priv->phy_addr;
+	} else {
+		for_each_available_child_of_node(np, child) {
+			addr = of_mdio_parse_addr(&dev->dev, child);
+			if (addr < 0)
+				continue;
+
+			read_mask |= 1 << addr;
+		}
+	}
+
+	for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
+		if (read_mask & 1 << addr) {
+			dev_dbg(&dev->dev, "Workaround for PHY @ %d\n", addr);
+			mdiobus_read(bus, addr, MII_BMSR);
+		}
+	}
+
+	return 0;
+}
+
 static int bcmgenet_mii_alloc(struct bcmgenet_priv *priv)
 {
 	struct mii_bus *bus;
@@ -427,6 +473,7 @@ static int bcmgenet_mii_alloc(struct bcmgenet_priv *priv)
 	bus->parent = &priv->pdev->dev;
 	bus->read = bcmgenet_mii_read;
 	bus->write = bcmgenet_mii_write;
+	bus->reset = bcmgenet_mii_bus_reset;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d",
 		 priv->pdev->name, priv->pdev->id);
 
@@ -443,7 +490,6 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
 {
 	struct device_node *dn = priv->pdev->dev.of_node;
 	struct device *kdev = &priv->pdev->dev;
-	struct device_node *mdio_dn;
 	char *compat;
 	int ret;
 
@@ -451,14 +497,14 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
 	if (!compat)
 		return -ENOMEM;
 
-	mdio_dn = of_find_compatible_node(dn, NULL, compat);
+	priv->mdio_dn = of_find_compatible_node(dn, NULL, compat);
 	kfree(compat);
-	if (!mdio_dn) {
+	if (!priv->mdio_dn) {
 		dev_err(kdev, "unable to find MDIO bus node\n");
 		return -ENODEV;
 	}
 
-	ret = of_mdiobus_register(priv->mii_bus, mdio_dn);
+	ret = of_mdiobus_register(priv->mii_bus, priv->mdio_dn);
 	if (ret) {
 		dev_err(kdev, "failed to register MDIO bus\n");
 		return ret;
-- 
2.1.0

Re: [PATCH net 0/3] net: phy: bcm7xxx initial read/write workaround

[Florian Fainelli]

On 26/06/15 10:38, Florian Fainelli wrote:
> Hi David,
> 
> This patch series fixes occasional BCM7xxx PHY driver binding failure due
> to a harware bug where the first read or write does not come out of the PHY
> MDIO management controller.
> 
> Since we have two different MDIO controllers using this PHY, a similar
> need to be replicated in GENET and UniMAC MDIO.

Mistankely sent a previous patch as part of this series, the latest
submission is correct.
-- 
Florian

[PATCH net] net: bcmgenet: power on MII block for all MII modes

[Florian Fainelli]

The RGMII block is currently only powered on when using RGMII or
RGMII_NO_ID, which is not correct when using the GENET interface in MII
or Reverse MII modes. We always need to power on the RGMII interface for
this block to properly work, regardless of the MII mode in which we
operate.

Fixes: aa09677cba423 ("net: bcmgenet: add MDIO routines")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
David,

I am targetting "net" here since this is a bug fix, however, we have few
people using MII or Reverse MII with GENET, such that there is no need to
queue this for -stable unless you want to.

Thanks!

 drivers/net/ethernet/broadcom/genet/bcmmii.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index e7651b3c6c57..420949cc55aa 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -299,9 +299,6 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 			phy_name = "external RGMII (no delay)";
 		else
 			phy_name = "external RGMII (TX delay)";
-		reg = bcmgenet_ext_readl(priv, EXT_RGMII_OOB_CTRL);
-		reg |= RGMII_MODE_EN | id_mode_dis;
-		bcmgenet_ext_writel(priv, reg, EXT_RGMII_OOB_CTRL);
 		bcmgenet_sys_writel(priv,
 				    PORT_MODE_EXT_GPHY, SYS_PORT_CTRL);
 		break;
@@ -310,6 +307,15 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 		return -EINVAL;
 	}
 
+	/* This is an external PHY (xMII), so we need to enable the RGMII
+	 * block for the interface to work
+	 */
+	if (priv->ext_phy) {
+		reg = bcmgenet_ext_readl(priv, EXT_RGMII_OOB_CTRL);
+		reg |= RGMII_MODE_EN | id_mode_dis;
+		bcmgenet_ext_writel(priv, reg, EXT_RGMII_OOB_CTRL);
+	}
+
 	if (init)
 		dev_info(kdev, "configuring instance for %s\n", phy_name);
 
-- 
2.1.0

Re: [PATCH net] net: bcmgenet: power on MII block for all MII modes

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon,  8 Jun 2015 10:47:57 -0700

> The RGMII block is currently only powered on when using RGMII or
> RGMII_NO_ID, which is not correct when using the GENET interface in MII
> or Reverse MII modes. We always need to power on the RGMII interface for
> this block to properly work, regardless of the MII mode in which we
> operate.
> 
> Fixes: aa09677cba423 ("net: bcmgenet: add MDIO routines")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> David,
> 
> I am targetting "net" here since this is a bug fix, however, we have few
> people using MII or Reverse MII with GENET, such that there is no need to
> queue this for -stable unless you want to.

Ok, applied, thanks Florian.