| From a3a3b72d964bf7c593bf393c83ceb25727ec1ade Mon Sep 17 00:00:00 2001 |
| From: Arvind Sankar <niveditas98@gmail.com> |
| Date: Sat, 2 Mar 2019 11:01:17 -0500 |
| Subject: igb: Fix WARN_ONCE on runtime suspend |
| |
| [ Upstream commit dabb8338be533c18f50255cf39ff4f66d4dabdbe ] |
| |
| The runtime_suspend device callbacks are not supposed to save |
| configuration state or change the power state. Commit fb29f76cc566 |
| ("igb: Fix an issue that PME is not enabled during runtime suspend") |
| changed the driver to not save configuration state during runtime |
| suspend, however the driver callback still put the device into a |
| low-power state. This causes a warning in the pci pm core and results in |
| pci_pm_runtime_suspend not calling pci_save_state or pci_finish_runtime_suspend. |
| |
| Fix this by not changing the power state either, leaving that to pci pm |
| core, and make the same change for suspend callback as well. |
| |
| Also move a couple of defines into the appropriate header file instead |
| of inline in the .c file. |
| |
| Fixes: fb29f76cc566 ("igb: Fix an issue that PME is not enabled during runtime suspend") |
| Signed-off-by: Arvind Sankar <niveditas98@gmail.com> |
| Reviewed-by: Kai-Heng Feng <kai.heng.feng@canonical.com> |
| Tested-by: Aaron Brown <aaron.f.brown@intel.com> |
| Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> |
| Signed-off-by: Sasha Levin (Microsoft) <sashal@kernel.org> |
| --- |
| .../net/ethernet/intel/igb/e1000_defines.h | 2 + |
| drivers/net/ethernet/intel/igb/igb_main.c | 57 +++---------------- |
| 2 files changed, 10 insertions(+), 49 deletions(-) |
| |
| diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h |
| index 01fcfc6f3415..d2e2c50ce257 100644 |
| --- a/drivers/net/ethernet/intel/igb/e1000_defines.h |
| +++ b/drivers/net/ethernet/intel/igb/e1000_defines.h |
| @@ -194,6 +194,8 @@ |
| /* enable link status from external LINK_0 and LINK_1 pins */ |
| #define E1000_CTRL_SWDPIN0 0x00040000 /* SWDPIN 0 value */ |
| #define E1000_CTRL_SWDPIN1 0x00080000 /* SWDPIN 1 value */ |
| +#define E1000_CTRL_ADVD3WUC 0x00100000 /* D3 WUC */ |
| +#define E1000_CTRL_EN_PHY_PWR_MGMT 0x00200000 /* PHY PM enable */ |
| #define E1000_CTRL_SDP0_DIR 0x00400000 /* SDP0 Data direction */ |
| #define E1000_CTRL_SDP1_DIR 0x00800000 /* SDP1 Data direction */ |
| #define E1000_CTRL_RST 0x04000000 /* Global reset */ |
| diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c |
| index 7137e7f9c7f3..21ccadb720d1 100644 |
| --- a/drivers/net/ethernet/intel/igb/igb_main.c |
| +++ b/drivers/net/ethernet/intel/igb/igb_main.c |
| @@ -8755,9 +8755,7 @@ static int __igb_shutdown(struct pci_dev *pdev, bool *enable_wake, |
| struct e1000_hw *hw = &adapter->hw; |
| u32 ctrl, rctl, status; |
| u32 wufc = runtime ? E1000_WUFC_LNKC : adapter->wol; |
| -#ifdef CONFIG_PM |
| - int retval = 0; |
| -#endif |
| + bool wake; |
| |
| rtnl_lock(); |
| netif_device_detach(netdev); |
| @@ -8770,14 +8768,6 @@ static int __igb_shutdown(struct pci_dev *pdev, bool *enable_wake, |
| igb_clear_interrupt_scheme(adapter); |
| rtnl_unlock(); |
| |
| -#ifdef CONFIG_PM |
| - if (!runtime) { |
| - retval = pci_save_state(pdev); |
| - if (retval) |
| - return retval; |
| - } |
| -#endif |
| - |
| status = rd32(E1000_STATUS); |
| if (status & E1000_STATUS_LU) |
| wufc &= ~E1000_WUFC_LNKC; |
| @@ -8794,10 +8784,6 @@ static int __igb_shutdown(struct pci_dev *pdev, bool *enable_wake, |
| } |
| |
| ctrl = rd32(E1000_CTRL); |
| - /* advertise wake from D3Cold */ |
| - #define E1000_CTRL_ADVD3WUC 0x00100000 |
| - /* phy power management enable */ |
| - #define E1000_CTRL_EN_PHY_PWR_MGMT 0x00200000 |
| ctrl |= E1000_CTRL_ADVD3WUC; |
| wr32(E1000_CTRL, ctrl); |
| |
| @@ -8811,12 +8797,15 @@ static int __igb_shutdown(struct pci_dev *pdev, bool *enable_wake, |
| wr32(E1000_WUFC, 0); |
| } |
| |
| - *enable_wake = wufc || adapter->en_mng_pt; |
| - if (!*enable_wake) |
| + wake = wufc || adapter->en_mng_pt; |
| + if (!wake) |
| igb_power_down_link(adapter); |
| else |
| igb_power_up_link(adapter); |
| |
| + if (enable_wake) |
| + *enable_wake = wake; |
| + |
| /* Release control of h/w to f/w. If f/w is AMT enabled, this |
| * would have already happened in close and is redundant. |
| */ |
| @@ -8859,22 +8848,7 @@ static void igb_deliver_wake_packet(struct net_device *netdev) |
| |
| static int __maybe_unused igb_suspend(struct device *dev) |
| { |
| - int retval; |
| - bool wake; |
| - struct pci_dev *pdev = to_pci_dev(dev); |
| - |
| - retval = __igb_shutdown(pdev, &wake, 0); |
| - if (retval) |
| - return retval; |
| - |
| - if (wake) { |
| - pci_prepare_to_sleep(pdev); |
| - } else { |
| - pci_wake_from_d3(pdev, false); |
| - pci_set_power_state(pdev, PCI_D3hot); |
| - } |
| - |
| - return 0; |
| + return __igb_shutdown(to_pci_dev(dev), NULL, 0); |
| } |
| |
| static int __maybe_unused igb_resume(struct device *dev) |
| @@ -8945,22 +8919,7 @@ static int __maybe_unused igb_runtime_idle(struct device *dev) |
| |
| static int __maybe_unused igb_runtime_suspend(struct device *dev) |
| { |
| - struct pci_dev *pdev = to_pci_dev(dev); |
| - int retval; |
| - bool wake; |
| - |
| - retval = __igb_shutdown(pdev, &wake, 1); |
| - if (retval) |
| - return retval; |
| - |
| - if (wake) { |
| - pci_prepare_to_sleep(pdev); |
| - } else { |
| - pci_wake_from_d3(pdev, false); |
| - pci_set_power_state(pdev, PCI_D3hot); |
| - } |
| - |
| - return 0; |
| + return __igb_shutdown(to_pci_dev(dev), NULL, 1); |
| } |
| |
| static int __maybe_unused igb_runtime_resume(struct device *dev) |
| -- |
| 2.20.1 |
| |