Add Todd. Todd found a regression caused by this commit. https://bugzilla.kernel.org/show_bug.cgi?id=218940 I think we should make that clear first. thanks, rui On Thu, 2024-06-06 at 16:06 +0200, Greg Kroah-Hartman wrote: > 6.6-stable review patch. If anyone has any objections, please let me > know. > > ------------------ > > From: Hui Wang <hui.wang@xxxxxxxxxxxxx> > > [ Upstream commit bfd546a552e140b0a4c8a21527c39d6d21addb28 ] > > The commit 861e8086029e ("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: 861e8086029e ("e1000e: move force SMBUS from enable ulp > function to avoid PHY loss issue") > Signed-off-by: Hui Wang <hui.wang@xxxxxxxxxxxxx> > Acked-by: Vitaly Lifshits <vitaly.lifshits@xxxxxxxxx> > Tested-by: Naama Meir <naamax.meir@xxxxxxxxxxxxxxx> > Reviewed-by: Simon Horman <horms@xxxxxxxxxx> > Reviewed-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@xxxxxxxxx> > Tested-by: Zhang Rui <rui.zhang@xxxxxxxxx> > Signed-off-by: Jacob Keller <jacob.e.keller@xxxxxxxxx> > Link: > https://lore.kernel.org/r/20240528-net-2024-05-28-intel-net-fixes-v1-1-dc8593d2bbc6@xxxxxxxxx > Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> > --- > drivers/net/ethernet/intel/e1000e/ich8lan.c | 22 > +++++++++++++++++++++ > drivers/net/ethernet/intel/e1000e/netdev.c | 18 ----------------- > 2 files changed, 22 insertions(+), 18 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c > b/drivers/net/ethernet/intel/e1000e/ich8lan.c > index 4d83c9a0c023a..d678ca0254651 100644 > --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c > +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c > @@ -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) > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c > b/drivers/net/ethernet/intel/e1000e/netdev.c > index 3692fce201959..cc8c531ec3dff 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -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
