[PATCH net-next 0/7] net: bcmgenet: integrated GPHY power up/down

[PATCH net-next 0/7] net: bcmgenet: integrated GPHY power up/down

[Florian Fainelli]

Hi David,

This patch series implements integrated Gigabit PHY power up/down, which allows
us to save close to 300mW on some designs when the Gigabit PHY is known to be
unused (e.g: during bcmgenet_close or bcmgenet_suspend not doing Wake-on-LAN).

Thanks!

Florian Fainelli (7):
  net: bcmgenet: propagate errors from bcmgenet_power_down
  net: bcmgenet: update bcmgenet_ephy_power_up to clear CK25_DIS bit
  net: bcmgenet: rename bcmgenet_ephy_power_up
  net: bcmgenet: fix GPHY power-up sequence
  net: bcmgenet: implement GPHY power down sequence
  net: bcmgenet: power up and down integrated GPHY when unused
  net: bcmgenet: power down and up GPHY during suspend/resume

 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 34 +++++++++++++++++++-------
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  2 ++
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 32 ++++++++++++++++--------
 3 files changed, 49 insertions(+), 19 deletions(-)

-- 
2.1.0

[PATCH net-next 1/7] net: bcmgenet: propagate errors from bcmgenet_power_down

[Florian Fainelli]

If bcmgenet_power_down() fails, we would want to propagate a return
value from bcmgenet_wol_power_down_cfg() to know about this.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 12956b143b11..f86837f1656a 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -847,9 +847,10 @@ static struct ethtool_ops bcmgenet_ethtool_ops = {
 };
 
 /* Power down the unimac, based on mode. */
-static void bcmgenet_power_down(struct bcmgenet_priv *priv,
+static int bcmgenet_power_down(struct bcmgenet_priv *priv,
 				enum bcmgenet_power_mode mode)
 {
+	int ret = 0;
 	u32 reg;
 
 	switch (mode) {
@@ -858,7 +859,7 @@ static void bcmgenet_power_down(struct bcmgenet_priv *priv,
 		break;
 
 	case GENET_POWER_WOL_MAGIC:
-		bcmgenet_wol_power_down_cfg(priv, mode);
+		ret = bcmgenet_wol_power_down_cfg(priv, mode);
 		break;
 
 	case GENET_POWER_PASSIVE:
@@ -873,6 +874,8 @@ static void bcmgenet_power_down(struct bcmgenet_priv *priv,
 	default:
 		break;
 	}
+
+	return 0;
 }
 
 static void bcmgenet_power_up(struct bcmgenet_priv *priv,
@@ -2606,12 +2609,12 @@ static int bcmgenet_close(struct net_device *dev)
 	free_irq(priv->irq1, priv);
 
 	if (phy_is_internal(priv->phydev))
-		bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
+		ret = bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
 
 	if (!IS_ERR(priv->clk))
 		clk_disable_unprepare(priv->clk);
 
-	return 0;
+	return ret;
 }
 
 static void bcmgenet_timeout(struct net_device *dev)
@@ -3097,14 +3100,14 @@ static int bcmgenet_suspend(struct device *d)
 
 	/* Prepare the device for Wake-on-LAN and switch to the slow clock */
 	if (device_may_wakeup(d) && priv->wolopts) {
-		bcmgenet_power_down(priv, GENET_POWER_WOL_MAGIC);
+		ret = bcmgenet_power_down(priv, GENET_POWER_WOL_MAGIC);
 		clk_prepare_enable(priv->clk_wol);
 	}
 
 	/* Turn off the clocks */
 	clk_disable_unprepare(priv->clk);
 
-	return 0;
+	return ret;
 }
 
 static int bcmgenet_resume(struct device *d)
-- 
2.1.0

[PATCH net-next 2/7] net: bcmgenet: update bcmgenet_ephy_power_up to clear CK25_DIS bit

[Florian Fainelli]

The CK25_DIS bit controls whether a 25Mhz clock is fed to the GPHY or
not, in preparation for powering down the integrated GPHY when relevant,
make sure we clear that bit.

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

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 1ea838946318..a27ef777cc81 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -354,6 +354,7 @@ struct bcmgenet_mib_counters {
 #define EXT_GPHY_CTRL			0x1C
 #define  EXT_CFG_IDDQ_BIAS		(1 << 0)
 #define  EXT_CFG_PWR_DOWN		(1 << 1)
+#define  EXT_CK25_DIS			(1 << 4)
 #define  EXT_GPHY_RESET			(1 << 5)
 
 /* DMA rings size */
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 446889cc3c6a..f7d9d2753141 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -178,7 +178,7 @@ static void bcmgenet_ephy_power_up(struct net_device *dev)
 		return;
 
 	reg = bcmgenet_ext_readl(priv, EXT_GPHY_CTRL);
-	reg &= ~(EXT_CFG_IDDQ_BIAS | EXT_CFG_PWR_DOWN);
+	reg &= ~(EXT_CFG_IDDQ_BIAS | EXT_CFG_PWR_DOWN | EXT_CK25_DIS);
 	reg |= EXT_GPHY_RESET;
 	bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
 	mdelay(2);
-- 
2.1.0

[PATCH net-next 3/7] net: bcmgenet: rename bcmgenet_ephy_power_up

[Florian Fainelli]

In preparation for implementing the power down GPHY sequence, rename
bcmgenet_ephy_power_up to illustrate that it is not EPHY specific but
PHY agnostic, and add an "enable" argument.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmmii.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index f7d9d2753141..3fdbf57208a3 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -168,7 +168,7 @@ void bcmgenet_mii_reset(struct net_device *dev)
 	}
 }
 
-static void bcmgenet_ephy_power_up(struct net_device *dev)
+static void bcmgenet_phy_power_set(struct net_device *dev, bool enable)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
 	u32 reg = 0;
@@ -177,15 +177,17 @@ static void bcmgenet_ephy_power_up(struct net_device *dev)
 	if (!GENET_IS_V4(priv))
 		return;
 
-	reg = bcmgenet_ext_readl(priv, EXT_GPHY_CTRL);
-	reg &= ~(EXT_CFG_IDDQ_BIAS | EXT_CFG_PWR_DOWN | EXT_CK25_DIS);
-	reg |= EXT_GPHY_RESET;
-	bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
-	mdelay(2);
+	if (enable) {
+		reg = bcmgenet_ext_readl(priv, EXT_GPHY_CTRL);
+		reg &= ~(EXT_CFG_IDDQ_BIAS | EXT_CFG_PWR_DOWN | EXT_CK25_DIS);
+		reg |= EXT_GPHY_RESET;
+		bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
+		mdelay(2);
 
-	reg &= ~EXT_GPHY_RESET;
-	bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
-	udelay(20);
+		reg &= ~EXT_GPHY_RESET;
+		bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
+		udelay(20);
+	}
 }
 
 static void bcmgenet_internal_phy_setup(struct net_device *dev)
@@ -193,8 +195,8 @@ static void bcmgenet_internal_phy_setup(struct net_device *dev)
 	struct bcmgenet_priv *priv = netdev_priv(dev);
 	u32 reg;
 
-	/* Power up EPHY */
-	bcmgenet_ephy_power_up(dev);
+	/* Power up PHY */
+	bcmgenet_phy_power_set(dev, true);
 	/* enable APD */
 	reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 	reg |= EXT_PWR_DN_EN_LD;
-- 
2.1.0

[PATCH net-next 4/7] net: bcmgenet: fix GPHY power-up sequence

[Florian Fainelli]

We were missing a number of extra steps and delays to power-up the GPHY, update
the sequence to reflect the proper procedure here.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmmii.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 3fdbf57208a3..c26c0757d755 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -179,14 +179,18 @@ static void bcmgenet_phy_power_set(struct net_device *dev, bool enable)
 
 	if (enable) {
 		reg = bcmgenet_ext_readl(priv, EXT_GPHY_CTRL);
-		reg &= ~(EXT_CFG_IDDQ_BIAS | EXT_CFG_PWR_DOWN | EXT_CK25_DIS);
+		reg &= ~EXT_CK25_DIS;
+		bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
+		mdelay(1);
+
+		reg &= ~(EXT_CFG_IDDQ_BIAS | EXT_CFG_PWR_DOWN);
 		reg |= EXT_GPHY_RESET;
 		bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
-		mdelay(2);
+		mdelay(1);
 
 		reg &= ~EXT_GPHY_RESET;
 		bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
-		udelay(20);
+		udelay(60);
 	}
 }
 
-- 
2.1.0

[PATCH net-next 5/7] net: bcmgenet: implement GPHY power down sequence

[Florian Fainelli]

Implement the GPHY power down sequence by setting all power down bits, putting
the GPHY in reset, and finally cutting the 25Mhz reference clock.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmmii.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index c26c0757d755..77d6a18707c3 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -177,6 +177,7 @@ static void bcmgenet_phy_power_set(struct net_device *dev, bool enable)
 	if (!GENET_IS_V4(priv))
 		return;
 
+	reg = bcmgenet_ext_readl(priv, EXT_GPHY_CTRL);
 	if (enable) {
 		reg = bcmgenet_ext_readl(priv, EXT_GPHY_CTRL);
 		reg &= ~EXT_CK25_DIS;
@@ -189,9 +190,14 @@ static void bcmgenet_phy_power_set(struct net_device *dev, bool enable)
 		mdelay(1);
 
 		reg &= ~EXT_GPHY_RESET;
+	} else {
+		reg |= EXT_CFG_IDDQ_BIAS | EXT_CFG_PWR_DOWN | EXT_GPHY_RESET;
 		bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
-		udelay(60);
+		mdelay(1);
+		reg |= EXT_CK25_DIS;
 	}
+	bcmgenet_ext_writel(priv, reg, EXT_GPHY_CTRL);
+	udelay(60);
 }
 
 static void bcmgenet_internal_phy_setup(struct net_device *dev)
-- 
2.1.0

[PATCH net-next 6/7] net: bcmgenet: power up and down integrated GPHY when unused

[Florian Fainelli]

Power up the GPHY while we are bringing-up the network interface, and
conversely, upon bring down, power the GPHY down. In order to avoid
creating hardware hazards, make sure that the GPHY gets powered on
during bcmgenet_open() prior to the UniMAC being reset as the UniMAC may
start creating activity towards the GPHY if we reverse the steps.

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

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index f86837f1656a..c6b15c3cf023 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -869,6 +869,8 @@ static int bcmgenet_power_down(struct bcmgenet_priv *priv,
 			reg |= (EXT_PWR_DOWN_PHY |
 				EXT_PWR_DOWN_DLL | EXT_PWR_DOWN_BIAS);
 			bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
+
+			bcmgenet_phy_power_set(priv->dev, false);
 		}
 		break;
 	default:
@@ -2465,9 +2467,6 @@ static void bcmgenet_netif_start(struct net_device *dev)
 
 	umac_enable_set(priv, CMD_TX_EN | CMD_RX_EN, true);
 
-	if (phy_is_internal(priv->phydev))
-		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
-
 	netif_tx_start_all_queues(dev);
 
 	phy_start(priv->phydev);
@@ -2486,6 +2485,12 @@ static int bcmgenet_open(struct net_device *dev)
 	if (!IS_ERR(priv->clk))
 		clk_prepare_enable(priv->clk);
 
+	/* If this is an internal GPHY, power it back on now, before UniMAC is
+	 * brought out of reset as absolutely no UniMAC activity is allowed
+	 */
+	if (phy_is_internal(priv->phydev))
+		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
+
 	/* take MAC out of reset */
 	bcmgenet_umac_reset(priv);
 
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index a27ef777cc81..7a59879d441f 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -664,6 +664,7 @@ int bcmgenet_mii_init(struct net_device *dev);
 int bcmgenet_mii_config(struct net_device *dev, bool init);
 void bcmgenet_mii_exit(struct net_device *dev);
 void bcmgenet_mii_reset(struct net_device *dev);
+void bcmgenet_phy_power_set(struct net_device *dev, bool enable);
 void bcmgenet_mii_setup(struct net_device *dev);
 
 /* Wake-on-LAN routines */
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 77d6a18707c3..5d110880f7c1 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -168,7 +168,7 @@ void bcmgenet_mii_reset(struct net_device *dev)
 	}
 }
 
-static void bcmgenet_phy_power_set(struct net_device *dev, bool enable)
+void bcmgenet_phy_power_set(struct net_device *dev, bool enable)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
 	u32 reg = 0;
-- 
2.1.0

[PATCH net-next 7/7] net: bcmgenet: power down and up GPHY during suspend/resume

[Florian Fainelli]

In case the interface is not used, power down the integrated GPHY during
suspend. Similarly to bcmgenet_open(), bcmgenet_resume() powers on the GPHY
prior to any UniMAC activity.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index c6b15c3cf023..c38d5429e27a 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3107,6 +3107,8 @@ static int bcmgenet_suspend(struct device *d)
 	if (device_may_wakeup(d) && priv->wolopts) {
 		ret = bcmgenet_power_down(priv, GENET_POWER_WOL_MAGIC);
 		clk_prepare_enable(priv->clk_wol);
+	} else if (phy_is_internal(priv->phydev)) {
+		ret = bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
 	}
 
 	/* Turn off the clocks */
@@ -3131,6 +3133,12 @@ static int bcmgenet_resume(struct device *d)
 	if (ret)
 		return ret;
 
+	/* If this is an internal GPHY, power it back on now, before UniMAC is
+	 * brought out of reset as absolutely no UniMAC activity is allowed
+	 */
+	if (phy_is_internal(priv->phydev))
+		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
+
 	bcmgenet_umac_reset(priv);
 
 	ret = init_umac(priv);
-- 
2.1.0

Re: [PATCH net-next 0/7] net: bcmgenet: integrated GPHY power up/down

[Florian Fainelli]

On 23/03/15 15:03, Florian Fainelli wrote:
> Hi David,
> 
> This patch series implements integrated Gigabit PHY power up/down, which allows
> us to save close to 300mW on some designs when the Gigabit PHY is known to be
> unused (e.g: during bcmgenet_close or bcmgenet_suspend not doing Wake-on-LAN).

I will resubmit shortly with a trivial fix for "net: bcmgenet: implement
GPHY power down sequence"), sorry about that.

> 
> Thanks!
> 
> Florian Fainelli (7):
>   net: bcmgenet: propagate errors from bcmgenet_power_down
>   net: bcmgenet: update bcmgenet_ephy_power_up to clear CK25_DIS bit
>   net: bcmgenet: rename bcmgenet_ephy_power_up
>   net: bcmgenet: fix GPHY power-up sequence
>   net: bcmgenet: implement GPHY power down sequence
>   net: bcmgenet: power up and down integrated GPHY when unused
>   net: bcmgenet: power down and up GPHY during suspend/resume
> 
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 34 +++++++++++++++++++-------
>  drivers/net/ethernet/broadcom/genet/bcmgenet.h |  2 ++
>  drivers/net/ethernet/broadcom/genet/bcmmii.c   | 32 ++++++++++++++++--------
>  3 files changed, 49 insertions(+), 19 deletions(-)
> 


-- 
Florian