| Submitter | prasanna.panchamukhi@riverbed.com |
|---|---|
| Date | Feb. 23, 2011, 1:25 a.m. |
| Message ID | <1298424313-9840-1-git-send-email-prasanna.panchamukhi@riverbed.com> |
| Download | mbox | patch |
| Permalink | /patch/84040/ |
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
Comments
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.
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.
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
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);