forcedeth: power down phy when interface is down

From: "Ed Swierk" <[email protected]>

Bring the physical link down when the interface is down by placing the PHY in power-down state, unless WOL is enabled. This mirrors the behavior of other drivers including e1000 and tg3.

---

 drivers/net/forcedeth.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff -puN drivers/net/forcedeth.c~forcedeth-power-down-phy-when-interface-is-down drivers/net/forcedeth.c
@@ -1312,9 +1312,9 @@ static int phy_init(struct net_device *d
 	/* some phys clear out pause advertisment on reset, set it back */
 	mii_rw(dev, np->phyaddr, MII_ADVERTISE, reg);
 
-	/* restart auto negotiation */
+	/* restart auto negotiation, power down phy */
 	mii_control = mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ);
-	mii_control |= (BMCR_ANRESTART | BMCR_ANENABLE);
+	mii_control |= (BMCR_ANRESTART | BMCR_ANENABLE | BMCR_PDOWN);
 	if (mii_rw(dev, np->phyaddr, MII_BMCR, mii_control)) {
 		return PHY_ERROR;
 	}

@@ -4798,6 +4798,10 @@ static int nv_open(struct net_device *de
 
 	dprintk(KERN_DEBUG "nv_open: begin\n");
 
+	/* power up phy */
+	mii_rw(dev, np->phyaddr, MII_BMCR,
+	       mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ) & ~BMCR_PDOWN);
+
 	/* erase previous misconfiguration */
 	if (np->driver_data & DEV_HAS_POWER_CNTRL)
 		nv_mac_reset(dev);

@@ -4980,6 +4984,10 @@ static int nv_close(struct net_device *d
 	if (np->wolenabled) {
 		writel(NVREG_PFF_ALWAYS|NVREG_PFF_MYADDR, base + NvRegPacketFilterFlags);
 		nv_start_rx(dev);
+	} else {
+		/* power down phy */
+		mii_rw(dev, np->phyaddr, MII_BMCR,
+		       mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ)|BMCR_PDOWN);
 	}
 
 	/* FIXME: power down nic */

_

I would not include this patch until further testing is performed. NVIDIA MCP chips use 3rd party PHY vendors. By powering down the phy, it could have adverse affects on certain phys.

Ayaz

-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Thursday, December 13, 2007 4:03 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; Ayaz Abdulla
Subject: [patch 02/10] forcedeth: power down phy when interface is down

From: "Ed Swierk" <[email protected]>

Bring the physical link down when the interface is down by placing the PHY
in power-down state, unless WOL is enabled. This mirrors the behavior of
other drivers including e1000 and tg3.

---

 drivers/net/forcedeth.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff -puN drivers/net/forcedeth.c~forcedeth-power-down-phy-when-interface-is-down drivers/net/forcedeth.c
---
a/drivers/net/forcedeth.c~forcedeth-power-down-phy-when-interface-is-dow n
+++ a/drivers/net/forcedeth.c
@@ -1312,9 +1312,9 @@ static int phy_init(struct net_device *d
/* some phys clear out pause advertisment on reset, set it back
*/
mii_rw(dev, np->phyaddr, MII_ADVERTISE, reg);

- /* restart auto negotiation */
+ /* restart auto negotiation, power down phy */
mii_control = mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ);

-	mii_control |= (BMCR_ANRESTART | BMCR_ANENABLE);
+	mii_control |= (BMCR_ANRESTART | BMCR_ANENABLE | BMCR_PDOWN);

if (mii_rw(dev, np->phyaddr, MII_BMCR, mii_control)) {
return PHY_ERROR;
}
@@ -4798,6 +4798,10 @@ static int nv_open(struct net_device *de

dprintk(KERN_DEBUG "nv_open: begin\n");

+ /* power up phy */
+ mii_rw(dev, np->phyaddr, MII_BMCR,
+ mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ) &
~BMCR_PDOWN);
+
/* erase previous misconfiguration */
if (np->driver_data & DEV_HAS_POWER_CNTRL)
nv_mac_reset(dev);
@@ -4980,6 +4984,10 @@ static int nv_close(struct net_device *d
if (np->wolenabled) {
writel(NVREG_PFF_ALWAYS|NVREG_PFF_MYADDR, base +
NvRegPacketFilterFlags);
nv_start_rx(dev);
+ } else {
+ /* power down phy */
+ mii_rw(dev, np->phyaddr, MII_BMCR,
+ mii_rw(dev, np->phyaddr, MII_BMCR,
MII_READ)|BMCR_PDOWN);
}

/* FIXME: power down nic */
_
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Does this patch actually fix any observeable problem?

Without the patch, ifconfig down leaves the physical link up, which confuses datacenter users who expect the link lights both on the NIC and the switch to go out when they bring an interface down.

Furthermore, even though the phy is powered on, autonegotiation stops working, so a normally gigabit link might suddenly become 100 Mbit half-duplex when the interface goes down, and become gigabit when it comes up again.

--Ed

OK, thanks, I added that text to the changelog along with Ayaz's objection and shall continue to bug people with it until we have a fix merged.

Ed,

You mention that the phy will become 100Mbit half duplex, but during nv_close, the phy setting is not modified. This might be a separate issue.

Ayaz

-----Original Message-----
From: Andrew Morton [mailto:[email protected]]
Sent: Thursday, December 13, 2007 5:07 PM
To: Ed Swierk
Cc: Ayaz Abdulla; [email protected]; [email protected]
Subject: Re: [patch 02/10] forcedeth: power down phy when interface is down

OK, thanks, I added that text to the changelog along with Ayaz's objection
and shall continue to bug people with it until we have a fix merged.
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

as a side note this one feature has caused more bugs than almost any feature I can think of in e1000. If Linux had a decent power manager that handled PCI device D-states (D0/D3) then most hardware that is able to be certified in M$ OSes would already implement power savings just by going to D3 when it was not being used. In e1000's case all the "power down phy" logic would become useless because the hardware already has a huge dependency tree that it walks when going to D3 to decide if it can power down the link, or go to a lower power link (like 100Mb or 10Mb)

/rant
Jesse