[PATCH net 0/4] net: bcmgenet: Wake-on-LAN and suspend fixes

[PATCH net 0/4] net: bcmgenet: Wake-on-LAN and suspend fixes

[Florian Fainelli]

Hi David,

This patch series fixes some mistakes that were introduced during the driver
changes adding support suspend/resume and Wake-on-LAN.

Thanks!

Florian Fainelli (4):
  net: bcmgenet: request and enable main clock earlier
  net: bcmgenet: correctly suspend and resume PHY device
  net: bcmgenet: update UMAC_CMD only when link is detected
  net: bcmgenet: correctly resume adapter from Wake-on-LAN

 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 37 +++++++++++++++++---------
 drivers/net/ethernet/broadcom/genet/bcmmii.c   |  8 ++++--
 2 files changed, 30 insertions(+), 15 deletions(-)

-- 
1.9.1

[PATCH net 1/4] net: bcmgenet: request and enable main clock earlier

[Florian Fainelli]

bcmgenet_set_hw_params() will read the hardware version and compare it
with the one we are getting from Device Tree. Due to the clock being
enabled too late, bcmgenet_set_hw_params() will cause bus errors since
the GENET hardware block is still gated off by the time
bcmgenet_set_hw_params() is called, this will also make us fail the
version check since we will read the value 0 from the hardware.

Fix this by requesting the clock before the first piece of code that
needs to access hardware register.

Fixes: 1c1008c793fa4 ("net: bcmgenet: add main driver file")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index ce455aed5a2f..7a3661c090ee 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2533,6 +2533,13 @@ static int bcmgenet_probe(struct platform_device *pdev)
 	priv->pdev = pdev;
 	priv->version = (enum bcmgenet_version)of_id->data;
 
+	priv->clk = devm_clk_get(&priv->pdev->dev, "enet");
+	if (IS_ERR(priv->clk))
+		dev_warn(&priv->pdev->dev, "failed to get enet clock\n");
+
+	if (!IS_ERR(priv->clk))
+		clk_prepare_enable(priv->clk);
+
 	bcmgenet_set_hw_params(priv);
 
 	/* Mii wait queue */
@@ -2541,17 +2548,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
 	priv->rx_buf_len = RX_BUF_LENGTH;
 	INIT_WORK(&priv->bcmgenet_irq_work, bcmgenet_irq_task);
 
-	priv->clk = devm_clk_get(&priv->pdev->dev, "enet");
-	if (IS_ERR(priv->clk))
-		dev_warn(&priv->pdev->dev, "failed to get enet clock\n");
-
 	priv->clk_wol = devm_clk_get(&priv->pdev->dev, "enet-wol");
 	if (IS_ERR(priv->clk_wol))
 		dev_warn(&priv->pdev->dev, "failed to get enet-wol clock\n");
 
-	if (!IS_ERR(priv->clk))
-		clk_prepare_enable(priv->clk);
-
 	err = reset_umac(priv);
 	if (err)
 		goto err_clk_disable;
-- 
1.9.1

[PATCH net 2/4] net: bcmgenet: correctly suspend and resume PHY device

[Florian Fainelli]

Make sure that we properly suspend and resume the PHY device when we
enter low power modes. We had two calls to bcmgenet_mii_reset() which
will issue a software-reset to the PHY without using the PHY library,
get rid of them since they are completely bogus and mess up with the PHY
library state. Make sure that we reset the PHY library cached values
(link, pause and duplex) to allow the link adjustment callback to be
invoked when needed.

Fixes: b6e978e50444 ("net: bcmgenet: add suspend/resume callbacks")
Fixes: 1c1008c793fa4 ("net: bcmgenet: add main driver file")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 7a3661c090ee..90eb9c4b1b2c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -739,7 +739,6 @@ static void bcmgenet_power_down(struct bcmgenet_priv *priv,
 
 	case GENET_POWER_PASSIVE:
 		/* Power down LED */
-		bcmgenet_mii_reset(priv->dev);
 		if (priv->hw_params->flags & GENET_HAS_EXT) {
 			reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 			reg |= (EXT_PWR_DOWN_PHY |
@@ -779,7 +778,9 @@ static void bcmgenet_power_up(struct bcmgenet_priv *priv,
 	}
 
 	bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
-	bcmgenet_mii_reset(priv->dev);
+
+	if (mode == GENET_POWER_PASSIVE)
+		bcmgenet_mii_reset(priv->dev);
 }
 
 /* ioctl handle special commands that are not present in ethtool. */
@@ -2164,6 +2165,10 @@ static void bcmgenet_netif_stop(struct net_device *dev)
 	 * disabled no new work will be scheduled.
 	 */
 	cancel_work_sync(&priv->bcmgenet_irq_work);
+
+	priv->old_pause = -1;
+	priv->old_link = -1;
+	priv->old_duplex = -1;
 }
 
 static int bcmgenet_close(struct net_device *dev)
@@ -2611,6 +2616,8 @@ static int bcmgenet_suspend(struct device *d)
 
 	bcmgenet_netif_stop(dev);
 
+	phy_suspend(priv->phydev);
+
 	netif_device_detach(dev);
 
 	/* Disable MAC receive */
@@ -2693,6 +2700,8 @@ static int bcmgenet_resume(struct device *d)
 
 	netif_device_attach(dev);
 
+	phy_resume(priv->phydev);
+
 	bcmgenet_netif_start(dev);
 
 	return 0;
-- 
1.9.1

[PATCH net 3/4] net: bcmgenet: update UMAC_CMD only when link is detected

[Florian Fainelli]

When we bring the interface down, phy_stop() will schedule the PHY state
machine to call our link adjustment callback. By the time we do so, we
may have clock gated off the GENET hardware block, and this will cause
bus errors to happen in bcmgenet_mii_setup():

Make sure that we only touch the UMAC_CMD register when there is an
actual link. This is safe to do for two reasons:

- updating the Ethernet MAC registers only make sense when a physical
  link is present
- the PHY library state machine first set phydev->link = 0 before
  invoking phydev->adjust_link in the PHY_HALTED case

Fixes: 240524089d7a ("net: bcmgenet: only update UMAC_CMD if something changed")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmmii.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index b56f1bbb17bf..c88f7ae99636 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -129,7 +129,10 @@ static void bcmgenet_mii_setup(struct net_device *dev)
 			cmd_bits |= CMD_RX_PAUSE_IGNORE | CMD_TX_PAUSE_IGNORE;
 	}
 
-	if (status_changed) {
+	if (!status_changed)
+		return;
+
+	if (phydev->link) {
 		reg = bcmgenet_umac_readl(priv, UMAC_CMD);
 		reg &= ~((CMD_SPEED_MASK << CMD_SPEED_SHIFT) |
 			       CMD_HD_EN |
@@ -137,8 +140,9 @@ static void bcmgenet_mii_setup(struct net_device *dev)
 		reg |= cmd_bits;
 		bcmgenet_umac_writel(priv, reg, UMAC_CMD);
 
-		phy_print_status(phydev);
 	}
+
+	phy_print_status(phydev);
 }
 
 void bcmgenet_mii_reset(struct net_device *dev)
-- 
1.9.1

[PATCH net 4/4] net: bcmgenet: correctly resume adapter from Wake-on-LAN

[Florian Fainelli]

In case we configured the adapter to be a wake up source from
Wake-on-LAN, but we never actually woke up using Wake-on-LAN, we will
leave the adapter in MagicPacket matching mode, which prevents any other
type of packets from reaching the RX engine. Fix this by calling
bcmgenet_power_up() with GENET_POWER_WOL_MAGIC to restore the adapter
configuration in bcmgenet_resume().

The second problem we had was an imbalanced clock disabling in
bcmgenet_wol_resume(), the Wake-on-LAN slow clock is only enabled in
bcmgenet_suspend() if we configured Wake-on-LAN, yet we unconditionally
disabled the clock in bcmgenet_wol_resume().

Fixes: 8c90db72f926 ("net: bcmgenet: suspend and resume from Wake-on-LAN")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 90eb9c4b1b2c..3f9d4de8173c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1962,7 +1962,8 @@ static void bcmgenet_set_hw_addr(struct bcmgenet_priv *priv,
 static int bcmgenet_wol_resume(struct bcmgenet_priv *priv)
 {
 	/* From WOL-enabled suspend, switch to regular clock */
-	clk_disable_unprepare(priv->clk_wol);
+	if (priv->wolopts)
+		clk_disable_unprepare(priv->clk_wol);
 
 	phy_init_hw(priv->phydev);
 	/* Speed settings must be restored */
@@ -2668,9 +2669,7 @@ static int bcmgenet_resume(struct device *d)
 	if (ret)
 		goto out_clk_disable;
 
-	if (priv->wolopts)
-		ret = bcmgenet_wol_resume(priv);
-
+	ret = bcmgenet_wol_resume(priv);
 	if (ret)
 		goto out_clk_disable;
 
@@ -2685,6 +2684,9 @@ static int bcmgenet_resume(struct device *d)
 		bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
 	}
 
+	if (priv->wolopts)
+		bcmgenet_power_up(priv, GENET_POWER_WOL_MAGIC);
+
 	/* Disable RX/TX DMA and flush TX queues */
 	dma_ctrl = bcmgenet_dma_disable(priv);
 
-- 
1.9.1

Re: [PATCH net 0/4] net: bcmgenet: Wake-on-LAN and suspend fixes

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 11 Aug 2014 14:50:41 -0700

> This patch series fixes some mistakes that were introduced during the driver
> changes adding support suspend/resume and Wake-on-LAN.

Series applied, thanks.