[patch 02/10] forcedeth: power down phy when interface is down

[patch 02/10] forcedeth: power down phy when interface is down

[akpm]

From: "Ed Swierk" <eswierk@arastra.com>

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.

Signed-off-by: Ed Swierk <eswierk@arastra.com>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: Ayaz Abdulla <aabdulla@nvidia.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 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-down
+++ 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 */
_

RE: [patch 02/10] forcedeth: power down phy when interface is down

[Ayaz Abdulla]

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: akpm@linux-foundation.org [mailto:akpm@linux-foundation.org] 
Sent: Thursday, December 13, 2007 4:03 PM
To: jeff@garzik.org
Cc: netdev@vger.kernel.org; akpm@linux-foundation.org;
eswierk@arastra.com; Ayaz Abdulla
Subject: [patch 02/10] forcedeth: power down phy when interface is down


From: "Ed Swierk" <eswierk@arastra.com>

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.

Signed-off-by: Ed Swierk <eswierk@arastra.com>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: Ayaz Abdulla <aabdulla@nvidia.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 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.
-----------------------------------------------------------------------------------

Re: [patch 02/10] forcedeth: power down phy when interface is down

[Andrew Morton]

On Thu, 13 Dec 2007 16:11:55 -0800
"Ayaz Abdulla" <AAbdulla@nvidia.com> wrote:

> 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: akpm@linux-foundation.org [mailto:akpm@linux-foundation.org] 
> Sent: Thursday, December 13, 2007 4:03 PM
> To: jeff@garzik.org
> Cc: netdev@vger.kernel.org; akpm@linux-foundation.org;
> eswierk@arastra.com; Ayaz Abdulla
> Subject: [patch 02/10] forcedeth: power down phy when interface is down
> 
> 
> From: "Ed Swierk" <eswierk@arastra.com>
> 
> 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.

Does this patch actually fix any observeable problem?

RE: [patch 02/10] forcedeth: power down phy when interface is down

[Brandeburg, Jesse]

Andrew Morton wrote:
> On Thu, 13 Dec 2007 16:11:55 -0800
> "Ayaz Abdulla" <AAbdulla@nvidia.com> wrote:
> 
>> 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: akpm@linux-foundation.org [mailto:akpm@linux-foundation.org]
>> Sent: Thursday, December 13, 2007 4:03 PM
>> To: jeff@garzik.org
>> Cc: netdev@vger.kernel.org; akpm@linux-foundation.org;
>> eswierk@arastra.com; Ayaz Abdulla
>> Subject: [patch 02/10] forcedeth: power down phy when interface is
>> down 
>> 
>> 
>> From: "Ed Swierk" <eswierk@arastra.com>
>> 
>> 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.
> 
> Does this patch actually fix any observeable problem?

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

Re: [patch 02/10] forcedeth: power down phy when interface is down

[Ed Swierk]

On 12/13/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> 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

Re: [patch 02/10] forcedeth: power down phy when interface is down

[Andrew Morton]

On Thu, 13 Dec 2007 16:53:58 -0800
"Ed Swierk" <eswierk@arastra.com> wrote:

> On 12/13/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > 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.
> 

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.  

RE: [patch 02/10] forcedeth: power down phy when interface is down

[Ayaz Abdulla]

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:akpm@linux-foundation.org] 
Sent: Thursday, December 13, 2007 5:07 PM
To: Ed Swierk
Cc: Ayaz Abdulla; jeff@garzik.org; netdev@vger.kernel.org
Subject: Re: [patch 02/10] forcedeth: power down phy when interface is
down


On Thu, 13 Dec 2007 16:53:58 -0800
"Ed Swierk" <eswierk@arastra.com> wrote:

> On 12/13/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > 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.
> 

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.
-----------------------------------------------------------------------------------