[PATCH] e1000: power off PHY after reset when interface is down

[PATCH] e1000: power off PHY after reset when interface is down

[prasanna.panchamukhi]

From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com>

Some Phys supported by the e1000 driver do not remain powered off across
a reset of the device when the interface is down, e.g. on 82546.
This patch powers down (only when WoL is disabled) the PHY after reset if
the interface is down and ethtool diagnostics are not currently running.

Similar problem was see on 82571 controller and was fixed in e1000e driver
by Bruce Allan.
Please refer commit 31dbe5b4ac6fca72dec946e4d0fa7f0913f1d9b1 for details.

Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com>
---
 drivers/net/e1000/e1000_ethtool.c |   27 +++++++++++++++++++--------
 drivers/net/e1000/e1000_main.c    |    7 +++++++
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/net/e1000/e1000_ethtool.c b/drivers/net/e1000/e1000_ethtool.c
index f4d0922..d3f18f5 100644
--- a/drivers/net/e1000/e1000_ethtool.c
+++ b/drivers/net/e1000/e1000_ethtool.c
@@ -1554,6 +1554,15 @@ static void e1000_diag_test(struct net_device *netdev,
 	bool if_running = netif_running(netdev);
 
 	set_bit(__E1000_TESTING, &adapter->flags);
+
+	if (!if_running) {
+		e1000_power_up_phy(adapter);
+
+		adapter->hw.wait_autoneg_complete = 1;
+		e1000_reset(adapter);
+		adapter->hw.wait_autoneg_complete = 0;
+	}
+
 	if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
 		/* Offline tests */
 
@@ -1572,8 +1581,6 @@ static void e1000_diag_test(struct net_device *netdev,
 		if (if_running)
 			/* indicate we're in test mode */
 			dev_close(netdev);
-		else
-			e1000_reset(adapter);
 
 		if (e1000_reg_test(adapter, &data[0]))
 			eth_test->flags |= ETH_TEST_FL_FAILED;
@@ -1587,8 +1594,6 @@ static void e1000_diag_test(struct net_device *netdev,
 			eth_test->flags |= ETH_TEST_FL_FAILED;
 
 		e1000_reset(adapter);
-		/* make sure the phy is powered up */
-		e1000_power_up_phy(adapter);
 		if (e1000_loopback_test(adapter, &data[3]))
 			eth_test->flags |= ETH_TEST_FL_FAILED;
 
@@ -1602,19 +1607,25 @@ static void e1000_diag_test(struct net_device *netdev,
 		if (if_running)
 			dev_open(netdev);
 	} else {
-		e_info(hw, "online testing starting\n");
 		/* Online tests */
-		if (e1000_link_test(adapter, &data[4]))
-			eth_test->flags |= ETH_TEST_FL_FAILED;
 
-		/* Online tests aren't run; pass by default */
+		e_info(hw, "online testing starting\n");
+
+		/* register, eeprom, intr and loopback tests not run online */
 		data[0] = 0;
 		data[1] = 0;
 		data[2] = 0;
 		data[3] = 0;
 
+		if (e1000_link_test(adapter, &data[4]))
+			eth_test->flags |= ETH_TEST_FL_FAILED;
+
 		clear_bit(__E1000_TESTING, &adapter->flags);
 	}
+
+	if (!if_running)
+		e1000_reset(adapter);
+
 	msleep_interruptible(4 * 1000);
 }
 
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index bfab140..beec573 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -704,6 +704,13 @@ void e1000_reset(struct e1000_adapter *adapter)
 	ew32(VET, ETHERNET_IEEE_VLAN_TYPE);
 
 	e1000_reset_adaptive(hw);
+
+	if (!netif_running(adapter->netdev) &&
+	    !test_bit(__E1000_TESTING, &adapter->flags)) {
+		e1000_power_down_phy(adapter);
+		return;
+	}
+
 	e1000_phy_get_info(hw, &adapter->phy_info);
 
 	e1000_release_manageability(adapter);
-- 
1.7.0.4


------------------------------------------------------------------------------
Free Software Download: Index, Search & Analyze Logs and other IT data in 
Real-Time with Splunk. Collect, index and harness all the fast moving IT data 
generated by your applications, servers and devices whether physical, virtual
or in the cloud. Deliver compliance at lower cost and gain new business 
insights. http://p.sf.net/sfu/splunk-dev2dev 
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

Re: [PATCH] e1000: power off PHY after reset when interface is down

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

On Tue, 2011-02-22 at 17:25 -0800, prasanna.panchamukhi@riverbed.com
wrote:
> From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com>
> 
> Some Phys supported by the e1000 driver do not remain powered off
> across
> a reset of the device when the interface is down, e.g. on 82546.
> This patch powers down (only when WoL is disabled) the PHY after reset
> if
> the interface is down and ethtool diagnostics are not currently
> running.
> 
> Similar problem was see on 82571 controller and was fixed in e1000e
> driver
> by Bruce Allan.
> Please refer commit 31dbe5b4ac6fca72dec946e4d0fa7f0913f1d9b1 for
> details.
> 
> Signed-off-by: Prasanna S. Panchamukhi
> <prasanna.panchamukhi@riverbed.com>
> ---
>  drivers/net/e1000/e1000_ethtool.c |   27 +++++++++++++++++++--------
>  drivers/net/e1000/e1000_main.c    |    7 +++++++
>  2 files changed, 26 insertions(+), 8 deletions(-) 

Thanks Prasanna!  I have added the patch to my queue of e1000 patches.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH] e1000: power off PHY after reset when interface is down

[Jeff Kirsher]

On Wed, Feb 23, 2011 at 16:02, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> On Tue, 2011-02-22 at 17:25 -0800, prasanna.panchamukhi@riverbed.com
> wrote:
>> From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com>
>>
>> Some Phys supported by the e1000 driver do not remain powered off
>> across
>> a reset of the device when the interface is down, e.g. on 82546.
>> This patch powers down (only when WoL is disabled) the PHY after reset
>> if
>> the interface is down and ethtool diagnostics are not currently
>> running.
>>
>> Similar problem was see on 82571 controller and was fixed in e1000e
>> driver
>> by Bruce Allan.
>> Please refer commit 31dbe5b4ac6fca72dec946e4d0fa7f0913f1d9b1 for
>> details.
>>
>> Signed-off-by: Prasanna S. Panchamukhi
>> <prasanna.panchamukhi@riverbed.com>
>> ---
>>  drivers/net/e1000/e1000_ethtool.c |   27 +++++++++++++++++++--------
>>  drivers/net/e1000/e1000_main.c    |    7 +++++++
>>  2 files changed, 26 insertions(+), 8 deletions(-)
>
> Thanks Prasanna!  I have added the patch to my queue of e1000 patches.
>

Prasanna-
Here is what we found during validating your patch:

The behavior of 82546 device(s) seems to be identical with/without this patch
applied. 82546GB (LOM), dev_id 1079 powers down (with wol disabled) after
ifdown, but powers back up after approx. 10 seconds. 82546EB (NIC), dev_id
1010 powers down (with wol disabled) after ifdown. Both of the above
behaviors are the same with and without the patch applied. Also, if this patch
DID work as expected, it should print a message after a reset, such as "Cannot
restart autonegotiation: Resource temporarily unavailable", which would mirror
the behavior of e1000e.

-- 
Cheers,
Jeff

Re: [PATCH] e1000: power off PHY after reset when interface is down

[Prasanna Panchamukhi]

On 03/04/2011 12:19 AM, Jeff Kirsher wrote:
> On Wed, Feb 23, 2011 at 16:02, Jeff Kirsher<jeffrey.t.kirsher@intel.com>  wrote:
>> On Tue, 2011-02-22 at 17:25 -0800, prasanna.panchamukhi@riverbed.com
>> wrote:
>>> From: Prasanna S. Panchamukhi<prasanna.panchamukhi@riverbed.com>
>>>
>>> Some Phys supported by the e1000 driver do not remain powered off
>>> across
>>> a reset of the device when the interface is down, e.g. on 82546.
>>> This patch powers down (only when WoL is disabled) the PHY after reset
>>> if
>>> the interface is down and ethtool diagnostics are not currently
>>> running.
>>>
>>> Similar problem was see on 82571 controller and was fixed in e1000e
>>> driver
>>> by Bruce Allan.
>>> Please refer commit 31dbe5b4ac6fca72dec946e4d0fa7f0913f1d9b1 for
>>> details.
>>>
>>> Signed-off-by: Prasanna S. Panchamukhi
>>> <prasanna.panchamukhi@riverbed.com>
>>> ---
>>>   drivers/net/e1000/e1000_ethtool.c |   27 +++++++++++++++++++--------
>>>   drivers/net/e1000/e1000_main.c    |    7 +++++++
>>>   2 files changed, 26 insertions(+), 8 deletions(-)
>> Thanks Prasanna!  I have added the patch to my queue of e1000 patches.
>>
> Prasanna-
> Here is what we found during validating your patch:
>
> The behavior of 82546 device(s) seems to be identical with/without this patch
> applied. 82546GB (LOM), dev_id 1079 powers down (with wol disabled) after
> ifdown, but powers back up after approx. 10 seconds. 82546EB (NIC), dev_id
> 1010 powers down (with wol disabled) after ifdown. Both of the above
> behaviors are the same with and without the patch applied. Also, if this patch
> DID work as expected, it should print a message after a reset, such as "Cannot
> restart autonegotiation: Resource temporarily unavailable", which would mirror
> the behavior of e1000e.
>
Hi Jeff,

Below is the test case we run verify this fix:

$ ethtool -s eth0 wol d
$ ifconfig eth0 up
$ mii-tool eth0
eth0: negotiated 100baseTx-FD, link ok
$ ifconfig eth0 down
$ mii-tool eth0
eth0: no link
$ ethtool -s eth0 autoneg on  (doesn't really matter what we do here)
$ mii-tool eth0
eth0: negotiated 100baseTx-FD, link ok (this should be: eth0: no link)

I will re-run the test&  check if does not fix this.

Thanks
Prasanna

Re: [PATCH] e1000: power off PHY after reset when interface is down

[Prasanna Panchamukhi]

On 03/04/2011 10:03 AM, Prasanna Panchamukhi wrote:
> On 03/04/2011 12:19 AM, Jeff Kirsher wrote:
>> On Wed, Feb 23, 2011 at 16:02, Jeff Kirsher<jeffrey.t.kirsher@intel.com>   wrote:
>>> On Tue, 2011-02-22 at 17:25 -0800, prasanna.panchamukhi@riverbed.com
>>> wrote:
>>>> From: Prasanna S. Panchamukhi<prasanna.panchamukhi@riverbed.com>
>>>>
>>>> Some Phys supported by the e1000 driver do not remain powered off
>>>> across
>>>> a reset of the device when the interface is down, e.g. on 82546.
>>>> This patch powers down (only when WoL is disabled) the PHY after reset
>>>> if
>>>> the interface is down and ethtool diagnostics are not currently
>>>> running.
>>>>
>>>> Similar problem was see on 82571 controller and was fixed in e1000e
>>>> driver
>>>> by Bruce Allan.
>>>> Please refer commit 31dbe5b4ac6fca72dec946e4d0fa7f0913f1d9b1 for
>>>> details.
>>>>
>>>> Signed-off-by: Prasanna S. Panchamukhi
>>>> <prasanna.panchamukhi@riverbed.com>
>>>> ---
>>>>    drivers/net/e1000/e1000_ethtool.c |   27 +++++++++++++++++++--------
>>>>    drivers/net/e1000/e1000_main.c    |    7 +++++++
>>>>    2 files changed, 26 insertions(+), 8 deletions(-)
>>> Thanks Prasanna!  I have added the patch to my queue of e1000 patches.
>>>
>> Prasanna-
>> Here is what we found during validating your patch:
>>
>> The behavior of 82546 device(s) seems to be identical with/without this patch
>> applied. 82546GB (LOM), dev_id 1079 powers down (with wol disabled) after
>> ifdown, but powers back up after approx. 10 seconds. 82546EB (NIC), dev_id
>> 1010 powers down (with wol disabled) after ifdown. Both of the above
>> behaviors are the same with and without the patch applied. Also, if this patch
>> DID work as expected, it should print a message after a reset, such as "Cannot
>> restart autonegotiation: Resource temporarily unavailable", which would mirror
>> the behavior of e1000e.
>>
> Hi Jeff,
>
> Below is the test case we run verify this fix:
>
> $ ethtool -s eth0 wol d
> $ ifconfig eth0 up
> $ mii-tool eth0
> eth0: negotiated 100baseTx-FD, link ok
> $ ifconfig eth0 down
> $ mii-tool eth0
> eth0: no link
> $ ethtool -s eth0 autoneg on  (doesn't really matter what we do here)
> $ mii-tool eth0
> eth0: negotiated 100baseTx-FD, link ok (this should be: eth0: no link)
>
> I will re-run the test&   check if does not fix this.
Hi Jeff,

I re-verified it & confirm that, this patch fixes the problem.
Please let me know if you have any issues.

Thanks
Prasanna

> Thanks
> Prasanna
>
>
>