Commit bfd546a5 authored by Hui Wang's avatar Hui Wang Committed by Jakub Kicinski

e1000e: move force SMBUS near the end of enable_ulp function

The commit 861e8086 ("e1000e: move force SMBUS from enable ulp
function to avoid PHY loss issue") introduces a regression on
PCH_MTP_I219_LM18 (PCIID: 0x8086550A). Without the referred commit, the
ethernet works well after suspend and resume, but after applying the
commit, the ethernet couldn't work anymore after the resume and the
dmesg shows that the NIC link changes to 10Mbps (1000Mbps originally):

    [   43.305084] e1000e 0000:00:1f.6 enp0s31f6: NIC Link is Up 10 Mbps Full Duplex, Flow Control: Rx/Tx

Without the commit, the force SMBUS code will not be executed if
"return 0" or "goto out" is executed in the enable_ulp(), and in my
case, the "goto out" is executed since FWSM_FW_VALID is set. But after
applying the commit, the force SMBUS code will be ran unconditionally.

Here move the force SMBUS code back to enable_ulp() and put it
immediately ahead of hw->phy.ops.release(hw), this could allow the
longest settling time as possible for interface in this function and
doesn't change the original code logic.

The issue was found on a Lenovo laptop with the ethernet hw as below:
00:1f.6 Ethernet controller [0200]: Intel Corporation Device [8086:550a]
(rev 20).

And this patch is verified (cable plug and unplug, system suspend
and resume) on Lenovo laptops with ethernet hw: [8086:550a],
[8086:550b], [8086:15bb], [8086:15be], [8086:1a1f], [8086:1a1c] and
[8086:0dc7].

Fixes: 861e8086 ("e1000e: move force SMBUS from enable ulp function to avoid PHY loss issue")
Signed-off-by: default avatarHui Wang <hui.wang@canonical.com>
Acked-by: default avatarVitaly Lifshits <vitaly.lifshits@intel.com>
Tested-by: default avatarNaama Meir <naamax.meir@linux.intel.com>
Reviewed-by: default avatarSimon Horman <horms@kernel.org>
Reviewed-by: default avatarPaul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
Tested-by: default avatarZhang Rui <rui.zhang@intel.com>
Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Link: https://lore.kernel.org/r/20240528-net-2024-05-28-intel-net-fixes-v1-1-dc8593d2bbc6@intel.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 278d65cc
......@@ -1225,6 +1225,28 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool to_sx)
}
release:
/* Switching PHY interface always returns MDI error
* so disable retry mechanism to avoid wasting time
*/
e1000e_disable_phy_retry(hw);
/* Force SMBus mode in PHY */
ret_val = e1000_read_phy_reg_hv_locked(hw, CV_SMB_CTRL, &phy_reg);
if (ret_val) {
e1000e_enable_phy_retry(hw);
hw->phy.ops.release(hw);
goto out;
}
phy_reg |= CV_SMB_CTRL_FORCE_SMBUS;
e1000_write_phy_reg_hv_locked(hw, CV_SMB_CTRL, phy_reg);
e1000e_enable_phy_retry(hw);
/* Force SMBus mode in MAC */
mac_reg = er32(CTRL_EXT);
mac_reg |= E1000_CTRL_EXT_FORCE_SMBUS;
ew32(CTRL_EXT, mac_reg);
hw->phy.ops.release(hw);
out:
if (ret_val)
......
......@@ -6623,7 +6623,6 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool runtime)
struct e1000_hw *hw = &adapter->hw;
u32 ctrl, ctrl_ext, rctl, status, wufc;
int retval = 0;
u16 smb_ctrl;
/* Runtime suspend should only enable wakeup for link changes */
if (runtime)
......@@ -6697,23 +6696,6 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool runtime)
if (retval)
return retval;
}
/* Force SMBUS to allow WOL */
/* Switching PHY interface always returns MDI error
* so disable retry mechanism to avoid wasting time
*/
e1000e_disable_phy_retry(hw);
e1e_rphy(hw, CV_SMB_CTRL, &smb_ctrl);
smb_ctrl |= CV_SMB_CTRL_FORCE_SMBUS;
e1e_wphy(hw, CV_SMB_CTRL, smb_ctrl);
e1000e_enable_phy_retry(hw);
/* Force SMBus mode in MAC */
ctrl_ext = er32(CTRL_EXT);
ctrl_ext |= E1000_CTRL_EXT_FORCE_SMBUS;
ew32(CTRL_EXT, ctrl_ext);
}
/* Ensure that the appropriate bits are set in LPI_CTRL
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment