[PATCH 1/3] Revert "e1000e: Additional PHY power saving in S0ix"

[PATCH 1/3] Revert "e1000e: Additional PHY power saving in S0ix"

[Kai-Heng Feng]

This reverts commit 3ad3e28cb203309fb29022dea41cd65df0583632.

The s0ix series makes e1000e on TGL and ADL fails to work after s2idle
resume.

There doesn't seem to be any solution soon, so revert the whole series.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=214821
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 44e2dc8328a22..e16b7c0d98089 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6380,16 +6380,10 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
 		ew32(CTRL_EXT, mac_data);
 
 		/* DFT control: PHY bit: page769_20[0] = 1
-		 * page769_20[7] - PHY PLL stop
-		 * page769_20[8] - PHY go to the electrical idle
-		 * page769_20[9] - PHY serdes disable
 		 * Gate PPW via EXTCNF_CTRL - set 0x0F00[7] = 1
 		 */
 		e1e_rphy(hw, I82579_DFT_CTRL, &phy_data);
 		phy_data |= BIT(0);
-		phy_data |= BIT(7);
-		phy_data |= BIT(8);
-		phy_data |= BIT(9);
 		e1e_wphy(hw, I82579_DFT_CTRL, phy_data);
 
 		mac_data = er32(EXTCNF_CTRL);
-- 
2.32.0

[PATCH 2/3] Revert "e1000e: Add polling mechanism to indicate CSME DPG exit"

[Kai-Heng Feng]

This reverts commit ef407b86d3cc7ab7ad37658c1c7a094cb8f3b6b4.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=214821
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/net/ethernet/intel/e1000e/ich8lan.h |  1 -
 drivers/net/ethernet/intel/e1000e/netdev.c  | 24 ---------------------
 drivers/net/ethernet/intel/e1000e/regs.h    |  1 -
 3 files changed, 26 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.h b/drivers/net/ethernet/intel/e1000e/ich8lan.h
index 2504b11c3169f..1dfa1d28dae64 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.h
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.h
@@ -41,7 +41,6 @@
 #define E1000_FWSM_WLOCK_MAC_MASK	0x0380
 #define E1000_FWSM_WLOCK_MAC_SHIFT	7
 #define E1000_FWSM_ULP_CFG_DONE		0x00000400	/* Low power cfg done */
-#define E1000_EXFWSM_DPG_EXIT_DONE	0x00000001
 
 /* Shared Receive Address Registers */
 #define E1000_SHRAL_PCH_LPT(_i)		(0x05408 + ((_i) * 8))
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e16b7c0d98089..242314809e59c 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6483,10 +6483,8 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
 static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
-	bool firmware_bug = false;
 	u32 mac_data;
 	u16 phy_data;
-	u32 i = 0;
 
 	if (er32(FWSM) & E1000_ICH_FWSM_FW_VALID) {
 		/* Request ME unconfigure the device from S0ix */
@@ -6494,28 +6492,6 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
 		mac_data &= ~E1000_H2ME_START_DPG;
 		mac_data |= E1000_H2ME_EXIT_DPG;
 		ew32(H2ME, mac_data);
-
-		/* Poll up to 2.5 seconds for ME to unconfigure DPG.
-		 * If this takes more than 1 second, show a warning indicating a
-		 * firmware bug
-		 */
-		while (!(er32(EXFWSM) & E1000_EXFWSM_DPG_EXIT_DONE)) {
-			if (i > 100 && !firmware_bug)
-				firmware_bug = true;
-
-			if (i++ == 250) {
-				e_dbg("Timeout (firmware bug): %d msec\n",
-				      i * 10);
-				break;
-			}
-
-			usleep_range(10000, 11000);
-		}
-		if (firmware_bug)
-			e_warn("DPG_EXIT_DONE took %d msec. This is a firmware bug\n",
-			       i * 10);
-		else
-			e_dbg("DPG_EXIT_DONE cleared after %d msec\n", i * 10);
 	} else {
 		/* Request driver unconfigure the device from S0ix */
 
diff --git a/drivers/net/ethernet/intel/e1000e/regs.h b/drivers/net/ethernet/intel/e1000e/regs.h
index 6c0cd8cab3ef2..8165ba2619a4d 100644
--- a/drivers/net/ethernet/intel/e1000e/regs.h
+++ b/drivers/net/ethernet/intel/e1000e/regs.h
@@ -213,7 +213,6 @@
 #define E1000_FACTPS	0x05B30	/* Function Active and Power State to MNG */
 #define E1000_SWSM	0x05B50	/* SW Semaphore */
 #define E1000_FWSM	0x05B54	/* FW Semaphore */
-#define E1000_EXFWSM	0x05B58	/* Extended FW Semaphore */
 /* Driver-only SW semaphore (not used by BOOT agents) */
 #define E1000_SWSM2	0x05B58
 #define E1000_FFLT_DBG	0x05F04	/* Debug Register */
-- 
2.32.0

[PATCH 3/3] Revert "e1000e: Add handshake with the CSME to support S0ix"

[Kai-Heng Feng]

This reverts commit 3e55d231716ea361b1520b801c6778c4c48de102.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=214821
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/net/ethernet/intel/e1000e/ich8lan.h |   2 -
 drivers/net/ethernet/intel/e1000e/netdev.c  | 328 +++++++++-----------
 2 files changed, 154 insertions(+), 176 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.h b/drivers/net/ethernet/intel/e1000e/ich8lan.h
index 1dfa1d28dae64..8f2a8f4ce0ee4 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.h
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.h
@@ -47,8 +47,6 @@
 #define E1000_SHRAH_PCH_LPT(_i)		(0x0540C + ((_i) * 8))
 
 #define E1000_H2ME		0x05B50	/* Host to ME */
-#define E1000_H2ME_START_DPG	0x00000001	/* indicate the ME of DPG */
-#define E1000_H2ME_EXIT_DPG	0x00000002	/* indicate the ME exit DPG */
 #define E1000_H2ME_ULP		0x00000800	/* ULP Indication Bit */
 #define E1000_H2ME_ENFORCE_SETTINGS	0x00001000	/* Enforce Settings */
 
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 242314809e59c..52c91c52b971b 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6345,105 +6345,43 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
 	u32 mac_data;
 	u16 phy_data;
 
-	if (er32(FWSM) & E1000_ICH_FWSM_FW_VALID) {
-		/* Request ME configure the device for S0ix */
-		mac_data = er32(H2ME);
-		mac_data |= E1000_H2ME_START_DPG;
-		mac_data &= ~E1000_H2ME_EXIT_DPG;
-		ew32(H2ME, mac_data);
-	} else {
-		/* Request driver configure the device to S0ix */
-		/* Disable the periodic inband message,
-		 * don't request PCIe clock in K1 page770_17[10:9] = 10b
-		 */
-		e1e_rphy(hw, HV_PM_CTRL, &phy_data);
-		phy_data &= ~HV_PM_CTRL_K1_CLK_REQ;
-		phy_data |= BIT(10);
-		e1e_wphy(hw, HV_PM_CTRL, phy_data);
-
-		/* Make sure we don't exit K1 every time a new packet arrives
-		 * 772_29[5] = 1 CS_Mode_Stay_In_K1
-		 */
-		e1e_rphy(hw, I217_CGFREG, &phy_data);
-		phy_data |= BIT(5);
-		e1e_wphy(hw, I217_CGFREG, phy_data);
-
-		/* Change the MAC/PHY interface to SMBus
-		 * Force the SMBus in PHY page769_23[0] = 1
-		 * Force the SMBus in MAC CTRL_EXT[11] = 1
-		 */
-		e1e_rphy(hw, CV_SMB_CTRL, &phy_data);
-		phy_data |= CV_SMB_CTRL_FORCE_SMBUS;
-		e1e_wphy(hw, CV_SMB_CTRL, phy_data);
-		mac_data = er32(CTRL_EXT);
-		mac_data |= E1000_CTRL_EXT_FORCE_SMBUS;
-		ew32(CTRL_EXT, mac_data);
-
-		/* DFT control: PHY bit: page769_20[0] = 1
-		 * Gate PPW via EXTCNF_CTRL - set 0x0F00[7] = 1
-		 */
-		e1e_rphy(hw, I82579_DFT_CTRL, &phy_data);
-		phy_data |= BIT(0);
-		e1e_wphy(hw, I82579_DFT_CTRL, phy_data);
-
-		mac_data = er32(EXTCNF_CTRL);
-		mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
-		ew32(EXTCNF_CTRL, mac_data);
-
-		/* Enable the Dynamic Power Gating in the MAC */
-		mac_data = er32(FEXTNVM7);
-		mac_data |= BIT(22);
-		ew32(FEXTNVM7, mac_data);
-
-		/* Disable disconnected cable conditioning for Power Gating */
-		mac_data = er32(DPGFR);
-		mac_data |= BIT(2);
-		ew32(DPGFR, mac_data);
-
-		/* Don't wake from dynamic Power Gating with clock request */
-		mac_data = er32(FEXTNVM12);
-		mac_data |= BIT(12);
-		ew32(FEXTNVM12, mac_data);
-
-		/* Ungate PGCB clock */
-		mac_data = er32(FEXTNVM9);
-		mac_data &= ~BIT(28);
-		ew32(FEXTNVM9, mac_data);
-
-		/* Enable K1 off to enable mPHY Power Gating */
-		mac_data = er32(FEXTNVM6);
-		mac_data |= BIT(31);
-		ew32(FEXTNVM6, mac_data);
-
-		/* Enable mPHY power gating for any link and speed */
-		mac_data = er32(FEXTNVM8);
-		mac_data |= BIT(9);
-		ew32(FEXTNVM8, mac_data);
-
-		/* Enable the Dynamic Clock Gating in the DMA and MAC */
-		mac_data = er32(CTRL_EXT);
-		mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
-		ew32(CTRL_EXT, mac_data);
-
-		/* No MAC DPG gating SLP_S0 in modern standby
-		 * Switch the logic of the lanphypc to use PMC counter
-		 */
-		mac_data = er32(FEXTNVM5);
-		mac_data |= BIT(7);
-		ew32(FEXTNVM5, mac_data);
-	}
+	/* Disable the periodic inband message,
+	 * don't request PCIe clock in K1 page770_17[10:9] = 10b
+	 */
+	e1e_rphy(hw, HV_PM_CTRL, &phy_data);
+	phy_data &= ~HV_PM_CTRL_K1_CLK_REQ;
+	phy_data |= BIT(10);
+	e1e_wphy(hw, HV_PM_CTRL, phy_data);
 
-	/* Disable the time synchronization clock */
-	mac_data = er32(FEXTNVM7);
-	mac_data |= BIT(31);
-	mac_data &= ~BIT(0);
-	ew32(FEXTNVM7, mac_data);
+	/* Make sure we don't exit K1 every time a new packet arrives
+	 * 772_29[5] = 1 CS_Mode_Stay_In_K1
+	 */
+	e1e_rphy(hw, I217_CGFREG, &phy_data);
+	phy_data |= BIT(5);
+	e1e_wphy(hw, I217_CGFREG, phy_data);
 
-	/* Dynamic Power Gating Enable */
+	/* Change the MAC/PHY interface to SMBus
+	 * Force the SMBus in PHY page769_23[0] = 1
+	 * Force the SMBus in MAC CTRL_EXT[11] = 1
+	 */
+	e1e_rphy(hw, CV_SMB_CTRL, &phy_data);
+	phy_data |= CV_SMB_CTRL_FORCE_SMBUS;
+	e1e_wphy(hw, CV_SMB_CTRL, phy_data);
 	mac_data = er32(CTRL_EXT);
-	mac_data |= BIT(3);
+	mac_data |= E1000_CTRL_EXT_FORCE_SMBUS;
 	ew32(CTRL_EXT, mac_data);
 
+	/* DFT control: PHY bit: page769_20[0] = 1
+	 * Gate PPW via EXTCNF_CTRL - set 0x0F00[7] = 1
+	 */
+	e1e_rphy(hw, I82579_DFT_CTRL, &phy_data);
+	phy_data |= BIT(0);
+	e1e_wphy(hw, I82579_DFT_CTRL, phy_data);
+
+	mac_data = er32(EXTCNF_CTRL);
+	mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
+	ew32(EXTCNF_CTRL, mac_data);
+
 	/* Check MAC Tx/Rx packet buffer pointers.
 	 * Reset MAC Tx/Rx packet buffer pointers to suppress any
 	 * pending traffic indication that would prevent power gating.
@@ -6478,6 +6416,59 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
 	mac_data = er32(RDFPC);
 	if (mac_data)
 		ew32(RDFPC, 0);
+
+	/* Enable the Dynamic Power Gating in the MAC */
+	mac_data = er32(FEXTNVM7);
+	mac_data |= BIT(22);
+	ew32(FEXTNVM7, mac_data);
+
+	/* Disable the time synchronization clock */
+	mac_data = er32(FEXTNVM7);
+	mac_data |= BIT(31);
+	mac_data &= ~BIT(0);
+	ew32(FEXTNVM7, mac_data);
+
+	/* Dynamic Power Gating Enable */
+	mac_data = er32(CTRL_EXT);
+	mac_data |= BIT(3);
+	ew32(CTRL_EXT, mac_data);
+
+	/* Disable disconnected cable conditioning for Power Gating */
+	mac_data = er32(DPGFR);
+	mac_data |= BIT(2);
+	ew32(DPGFR, mac_data);
+
+	/* Don't wake from dynamic Power Gating with clock request */
+	mac_data = er32(FEXTNVM12);
+	mac_data |= BIT(12);
+	ew32(FEXTNVM12, mac_data);
+
+	/* Ungate PGCB clock */
+	mac_data = er32(FEXTNVM9);
+	mac_data &= ~BIT(28);
+	ew32(FEXTNVM9, mac_data);
+
+	/* Enable K1 off to enable mPHY Power Gating */
+	mac_data = er32(FEXTNVM6);
+	mac_data |= BIT(31);
+	ew32(FEXTNVM6, mac_data);
+
+	/* Enable mPHY power gating for any link and speed */
+	mac_data = er32(FEXTNVM8);
+	mac_data |= BIT(9);
+	ew32(FEXTNVM8, mac_data);
+
+	/* Enable the Dynamic Clock Gating in the DMA and MAC */
+	mac_data = er32(CTRL_EXT);
+	mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
+	ew32(CTRL_EXT, mac_data);
+
+	/* No MAC DPG gating SLP_S0 in modern standby
+	 * Switch the logic of the lanphypc to use PMC counter
+	 */
+	mac_data = er32(FEXTNVM5);
+	mac_data |= BIT(7);
+	ew32(FEXTNVM5, mac_data);
 }
 
 static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
@@ -6486,98 +6477,87 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
 	u32 mac_data;
 	u16 phy_data;
 
-	if (er32(FWSM) & E1000_ICH_FWSM_FW_VALID) {
-		/* Request ME unconfigure the device from S0ix */
-		mac_data = er32(H2ME);
-		mac_data &= ~E1000_H2ME_START_DPG;
-		mac_data |= E1000_H2ME_EXIT_DPG;
-		ew32(H2ME, mac_data);
-	} else {
-		/* Request driver unconfigure the device from S0ix */
-
-		/* Disable the Dynamic Power Gating in the MAC */
-		mac_data = er32(FEXTNVM7);
-		mac_data &= 0xFFBFFFFF;
-		ew32(FEXTNVM7, mac_data);
-
-		/* Disable mPHY power gating for any link and speed */
-		mac_data = er32(FEXTNVM8);
-		mac_data &= ~BIT(9);
-		ew32(FEXTNVM8, mac_data);
-
-		/* Disable K1 off */
-		mac_data = er32(FEXTNVM6);
-		mac_data &= ~BIT(31);
-		ew32(FEXTNVM6, mac_data);
-
-		/* Disable Ungate PGCB clock */
-		mac_data = er32(FEXTNVM9);
-		mac_data |= BIT(28);
-		ew32(FEXTNVM9, mac_data);
-
-		/* Cancel not waking from dynamic
-		 * Power Gating with clock request
-		 */
-		mac_data = er32(FEXTNVM12);
-		mac_data &= ~BIT(12);
-		ew32(FEXTNVM12, mac_data);
+	/* Disable the Dynamic Power Gating in the MAC */
+	mac_data = er32(FEXTNVM7);
+	mac_data &= 0xFFBFFFFF;
+	ew32(FEXTNVM7, mac_data);
 
-		/* Cancel disable disconnected cable conditioning
-		 * for Power Gating
-		 */
-		mac_data = er32(DPGFR);
-		mac_data &= ~BIT(2);
-		ew32(DPGFR, mac_data);
+	/* Enable the time synchronization clock */
+	mac_data = er32(FEXTNVM7);
+	mac_data |= BIT(0);
+	ew32(FEXTNVM7, mac_data);
 
-		/* Disable the Dynamic Clock Gating in the DMA and MAC */
-		mac_data = er32(CTRL_EXT);
-		mac_data &= 0xFFF7FFFF;
-		ew32(CTRL_EXT, mac_data);
+	/* Disable mPHY power gating for any link and speed */
+	mac_data = er32(FEXTNVM8);
+	mac_data &= ~BIT(9);
+	ew32(FEXTNVM8, mac_data);
 
-		/* Revert the lanphypc logic to use the internal Gbe counter
-		 * and not the PMC counter
-		 */
-		mac_data = er32(FEXTNVM5);
-		mac_data &= 0xFFFFFF7F;
-		ew32(FEXTNVM5, mac_data);
+	/* Disable K1 off */
+	mac_data = er32(FEXTNVM6);
+	mac_data &= ~BIT(31);
+	ew32(FEXTNVM6, mac_data);
 
-		/* Enable the periodic inband message,
-		 * Request PCIe clock in K1 page770_17[10:9] =01b
-		 */
-		e1e_rphy(hw, HV_PM_CTRL, &phy_data);
-		phy_data &= 0xFBFF;
-		phy_data |= HV_PM_CTRL_K1_CLK_REQ;
-		e1e_wphy(hw, HV_PM_CTRL, phy_data);
+	/* Disable Ungate PGCB clock */
+	mac_data = er32(FEXTNVM9);
+	mac_data |= BIT(28);
+	ew32(FEXTNVM9, mac_data);
 
-		/* Return back configuration
-		 * 772_29[5] = 0 CS_Mode_Stay_In_K1
-		 */
-		e1e_rphy(hw, I217_CGFREG, &phy_data);
-		phy_data &= 0xFFDF;
-		e1e_wphy(hw, I217_CGFREG, phy_data);
+	/* Cancel not waking from dynamic
+	 * Power Gating with clock request
+	 */
+	mac_data = er32(FEXTNVM12);
+	mac_data &= ~BIT(12);
+	ew32(FEXTNVM12, mac_data);
 
-		/* Change the MAC/PHY interface to Kumeran
-		 * Unforce the SMBus in PHY page769_23[0] = 0
-		 * Unforce the SMBus in MAC CTRL_EXT[11] = 0
-		 */
-		e1e_rphy(hw, CV_SMB_CTRL, &phy_data);
-		phy_data &= ~CV_SMB_CTRL_FORCE_SMBUS;
-		e1e_wphy(hw, CV_SMB_CTRL, phy_data);
-		mac_data = er32(CTRL_EXT);
-		mac_data &= ~E1000_CTRL_EXT_FORCE_SMBUS;
-		ew32(CTRL_EXT, mac_data);
-	}
+	/* Cancel disable disconnected cable conditioning
+	 * for Power Gating
+	 */
+	mac_data = er32(DPGFR);
+	mac_data &= ~BIT(2);
+	ew32(DPGFR, mac_data);
 
 	/* Disable Dynamic Power Gating */
 	mac_data = er32(CTRL_EXT);
 	mac_data &= 0xFFFFFFF7;
 	ew32(CTRL_EXT, mac_data);
 
-	/* Enable the time synchronization clock */
-	mac_data = er32(FEXTNVM7);
-	mac_data &= ~BIT(31);
-	mac_data |= BIT(0);
-	ew32(FEXTNVM7, mac_data);
+	/* Disable the Dynamic Clock Gating in the DMA and MAC */
+	mac_data = er32(CTRL_EXT);
+	mac_data &= 0xFFF7FFFF;
+	ew32(CTRL_EXT, mac_data);
+
+	/* Revert the lanphypc logic to use the internal Gbe counter
+	 * and not the PMC counter
+	 */
+	mac_data = er32(FEXTNVM5);
+	mac_data &= 0xFFFFFF7F;
+	ew32(FEXTNVM5, mac_data);
+
+	/* Enable the periodic inband message,
+	 * Request PCIe clock in K1 page770_17[10:9] =01b
+	 */
+	e1e_rphy(hw, HV_PM_CTRL, &phy_data);
+	phy_data &= 0xFBFF;
+	phy_data |= HV_PM_CTRL_K1_CLK_REQ;
+	e1e_wphy(hw, HV_PM_CTRL, phy_data);
+
+	/* Return back configuration
+	 * 772_29[5] = 0 CS_Mode_Stay_In_K1
+	 */
+	e1e_rphy(hw, I217_CGFREG, &phy_data);
+	phy_data &= 0xFFDF;
+	e1e_wphy(hw, I217_CGFREG, phy_data);
+
+	/* Change the MAC/PHY interface to Kumeran
+	 * Unforce the SMBus in PHY page769_23[0] = 0
+	 * Unforce the SMBus in MAC CTRL_EXT[11] = 0
+	 */
+	e1e_rphy(hw, CV_SMB_CTRL, &phy_data);
+	phy_data &= ~CV_SMB_CTRL_FORCE_SMBUS;
+	e1e_wphy(hw, CV_SMB_CTRL, phy_data);
+	mac_data = er32(CTRL_EXT);
+	mac_data &= ~E1000_CTRL_EXT_FORCE_SMBUS;
+	ew32(CTRL_EXT, mac_data);
 }
 
 static int e1000e_pm_freeze(struct device *dev)
-- 
2.32.0

Re: [PATCH 3/3] Revert "e1000e: Add handshake with the CSME to support S0ix"

[Sasha Neftin]

On 11/22/2021 18:19, Kai-Heng Feng wrote:
> This reverts commit 3e55d231716ea361b1520b801c6778c4c48de102.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=214821
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>   drivers/net/ethernet/intel/e1000e/ich8lan.h |   2 -
>   drivers/net/ethernet/intel/e1000e/netdev.c  | 328 +++++++++-----------
>   2 files changed, 154 insertions(+), 176 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.h b/drivers/net/ethernet/intel/e1000e/ich8lan.h
> index 1dfa1d28dae64..8f2a8f4ce0ee4 100644
> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.h
> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.h
> @@ -47,8 +47,6 @@
>   #define E1000_SHRAH_PCH_LPT(_i)		(0x0540C + ((_i) * 8))
>   
>   #define E1000_H2ME		0x05B50	/* Host to ME */
> -#define E1000_H2ME_START_DPG	0x00000001	/* indicate the ME of DPG */
> -#define E1000_H2ME_EXIT_DPG	0x00000002	/* indicate the ME exit DPG */
>   #define E1000_H2ME_ULP		0x00000800	/* ULP Indication Bit */
>   #define E1000_H2ME_ENFORCE_SETTINGS	0x00001000	/* Enforce Settings */
>   
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 242314809e59c..52c91c52b971b 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -6345,105 +6345,43 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
>   	u32 mac_data;
>   	u16 phy_data;
>   
> -	if (er32(FWSM) & E1000_ICH_FWSM_FW_VALID) {
> -		/* Request ME configure the device for S0ix */
> -		mac_data = er32(H2ME);
> -		mac_data |= E1000_H2ME_START_DPG;
> -		mac_data &= ~E1000_H2ME_EXIT_DPG;
> -		ew32(H2ME, mac_data);
> -	} else {
> -		/* Request driver configure the device to S0ix */
> -		/* Disable the periodic inband message,
> -		 * don't request PCIe clock in K1 page770_17[10:9] = 10b
> -		 */
> -		e1e_rphy(hw, HV_PM_CTRL, &phy_data);
> -		phy_data &= ~HV_PM_CTRL_K1_CLK_REQ;
> -		phy_data |= BIT(10);
> -		e1e_wphy(hw, HV_PM_CTRL, phy_data);
> -
> -		/* Make sure we don't exit K1 every time a new packet arrives
> -		 * 772_29[5] = 1 CS_Mode_Stay_In_K1
> -		 */
> -		e1e_rphy(hw, I217_CGFREG, &phy_data);
> -		phy_data |= BIT(5);
> -		e1e_wphy(hw, I217_CGFREG, phy_data);
> -
> -		/* Change the MAC/PHY interface to SMBus
> -		 * Force the SMBus in PHY page769_23[0] = 1
> -		 * Force the SMBus in MAC CTRL_EXT[11] = 1
> -		 */
> -		e1e_rphy(hw, CV_SMB_CTRL, &phy_data);
> -		phy_data |= CV_SMB_CTRL_FORCE_SMBUS;
> -		e1e_wphy(hw, CV_SMB_CTRL, phy_data);
> -		mac_data = er32(CTRL_EXT);
> -		mac_data |= E1000_CTRL_EXT_FORCE_SMBUS;
> -		ew32(CTRL_EXT, mac_data);
> -
> -		/* DFT control: PHY bit: page769_20[0] = 1
> -		 * Gate PPW via EXTCNF_CTRL - set 0x0F00[7] = 1
> -		 */
> -		e1e_rphy(hw, I82579_DFT_CTRL, &phy_data);
> -		phy_data |= BIT(0);
> -		e1e_wphy(hw, I82579_DFT_CTRL, phy_data);
> -
> -		mac_data = er32(EXTCNF_CTRL);
> -		mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
> -		ew32(EXTCNF_CTRL, mac_data);
> -
> -		/* Enable the Dynamic Power Gating in the MAC */
> -		mac_data = er32(FEXTNVM7);
> -		mac_data |= BIT(22);
> -		ew32(FEXTNVM7, mac_data);
> -
> -		/* Disable disconnected cable conditioning for Power Gating */
> -		mac_data = er32(DPGFR);
> -		mac_data |= BIT(2);
> -		ew32(DPGFR, mac_data);
> -
> -		/* Don't wake from dynamic Power Gating with clock request */
> -		mac_data = er32(FEXTNVM12);
> -		mac_data |= BIT(12);
> -		ew32(FEXTNVM12, mac_data);
> -
> -		/* Ungate PGCB clock */
> -		mac_data = er32(FEXTNVM9);
> -		mac_data &= ~BIT(28);
> -		ew32(FEXTNVM9, mac_data);
> -
> -		/* Enable K1 off to enable mPHY Power Gating */
> -		mac_data = er32(FEXTNVM6);
> -		mac_data |= BIT(31);
> -		ew32(FEXTNVM6, mac_data);
> -
> -		/* Enable mPHY power gating for any link and speed */
> -		mac_data = er32(FEXTNVM8);
> -		mac_data |= BIT(9);
> -		ew32(FEXTNVM8, mac_data);
> -
> -		/* Enable the Dynamic Clock Gating in the DMA and MAC */
> -		mac_data = er32(CTRL_EXT);
> -		mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
> -		ew32(CTRL_EXT, mac_data);
> -
> -		/* No MAC DPG gating SLP_S0 in modern standby
> -		 * Switch the logic of the lanphypc to use PMC counter
> -		 */
> -		mac_data = er32(FEXTNVM5);
> -		mac_data |= BIT(7);
> -		ew32(FEXTNVM5, mac_data);
> -	}
> +	/* Disable the periodic inband message,
> +	 * don't request PCIe clock in K1 page770_17[10:9] = 10b
> +	 */
> +	e1e_rphy(hw, HV_PM_CTRL, &phy_data);
> +	phy_data &= ~HV_PM_CTRL_K1_CLK_REQ;
> +	phy_data |= BIT(10);
> +	e1e_wphy(hw, HV_PM_CTRL, phy_data);
>   
> -	/* Disable the time synchronization clock */
> -	mac_data = er32(FEXTNVM7);
> -	mac_data |= BIT(31);
> -	mac_data &= ~BIT(0);
> -	ew32(FEXTNVM7, mac_data);
> +	/* Make sure we don't exit K1 every time a new packet arrives
> +	 * 772_29[5] = 1 CS_Mode_Stay_In_K1
> +	 */
> +	e1e_rphy(hw, I217_CGFREG, &phy_data);
> +	phy_data |= BIT(5);
> +	e1e_wphy(hw, I217_CGFREG, phy_data);
>   
> -	/* Dynamic Power Gating Enable */
> +	/* Change the MAC/PHY interface to SMBus
> +	 * Force the SMBus in PHY page769_23[0] = 1
> +	 * Force the SMBus in MAC CTRL_EXT[11] = 1
> +	 */
> +	e1e_rphy(hw, CV_SMB_CTRL, &phy_data);
> +	phy_data |= CV_SMB_CTRL_FORCE_SMBUS;
> +	e1e_wphy(hw, CV_SMB_CTRL, phy_data);
>   	mac_data = er32(CTRL_EXT);
> -	mac_data |= BIT(3);
> +	mac_data |= E1000_CTRL_EXT_FORCE_SMBUS;
>   	ew32(CTRL_EXT, mac_data);
>   
> +	/* DFT control: PHY bit: page769_20[0] = 1
> +	 * Gate PPW via EXTCNF_CTRL - set 0x0F00[7] = 1
> +	 */
> +	e1e_rphy(hw, I82579_DFT_CTRL, &phy_data);
> +	phy_data |= BIT(0);
> +	e1e_wphy(hw, I82579_DFT_CTRL, phy_data);
> +
> +	mac_data = er32(EXTCNF_CTRL);
> +	mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
> +	ew32(EXTCNF_CTRL, mac_data);
> +
>   	/* Check MAC Tx/Rx packet buffer pointers.
>   	 * Reset MAC Tx/Rx packet buffer pointers to suppress any
>   	 * pending traffic indication that would prevent power gating.
> @@ -6478,6 +6416,59 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
>   	mac_data = er32(RDFPC);
>   	if (mac_data)
>   		ew32(RDFPC, 0);
> +
> +	/* Enable the Dynamic Power Gating in the MAC */
> +	mac_data = er32(FEXTNVM7);
> +	mac_data |= BIT(22);
> +	ew32(FEXTNVM7, mac_data);
> +
> +	/* Disable the time synchronization clock */
> +	mac_data = er32(FEXTNVM7);
> +	mac_data |= BIT(31);
> +	mac_data &= ~BIT(0);
> +	ew32(FEXTNVM7, mac_data);
> +
> +	/* Dynamic Power Gating Enable */
> +	mac_data = er32(CTRL_EXT);
> +	mac_data |= BIT(3);
> +	ew32(CTRL_EXT, mac_data);
> +
> +	/* Disable disconnected cable conditioning for Power Gating */
> +	mac_data = er32(DPGFR);
> +	mac_data |= BIT(2);
> +	ew32(DPGFR, mac_data);
> +
> +	/* Don't wake from dynamic Power Gating with clock request */
> +	mac_data = er32(FEXTNVM12);
> +	mac_data |= BIT(12);
> +	ew32(FEXTNVM12, mac_data);
> +
> +	/* Ungate PGCB clock */
> +	mac_data = er32(FEXTNVM9);
> +	mac_data &= ~BIT(28);
> +	ew32(FEXTNVM9, mac_data);
> +
> +	/* Enable K1 off to enable mPHY Power Gating */
> +	mac_data = er32(FEXTNVM6);
> +	mac_data |= BIT(31);
> +	ew32(FEXTNVM6, mac_data);
> +
> +	/* Enable mPHY power gating for any link and speed */
> +	mac_data = er32(FEXTNVM8);
> +	mac_data |= BIT(9);
> +	ew32(FEXTNVM8, mac_data);
> +
> +	/* Enable the Dynamic Clock Gating in the DMA and MAC */
> +	mac_data = er32(CTRL_EXT);
> +	mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
> +	ew32(CTRL_EXT, mac_data);
> +
> +	/* No MAC DPG gating SLP_S0 in modern standby
> +	 * Switch the logic of the lanphypc to use PMC counter
> +	 */
> +	mac_data = er32(FEXTNVM5);
> +	mac_data |= BIT(7);
> +	ew32(FEXTNVM5, mac_data);
>   }
>   
>   static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
> @@ -6486,98 +6477,87 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
>   	u32 mac_data;
>   	u16 phy_data;
>   
> -	if (er32(FWSM) & E1000_ICH_FWSM_FW_VALID) {
> -		/* Request ME unconfigure the device from S0ix */
> -		mac_data = er32(H2ME);
> -		mac_data &= ~E1000_H2ME_START_DPG;
> -		mac_data |= E1000_H2ME_EXIT_DPG;
> -		ew32(H2ME, mac_data);
> -	} else {
> -		/* Request driver unconfigure the device from S0ix */
> -
> -		/* Disable the Dynamic Power Gating in the MAC */
> -		mac_data = er32(FEXTNVM7);
> -		mac_data &= 0xFFBFFFFF;
> -		ew32(FEXTNVM7, mac_data);
> -
> -		/* Disable mPHY power gating for any link and speed */
> -		mac_data = er32(FEXTNVM8);
> -		mac_data &= ~BIT(9);
> -		ew32(FEXTNVM8, mac_data);
> -
> -		/* Disable K1 off */
> -		mac_data = er32(FEXTNVM6);
> -		mac_data &= ~BIT(31);
> -		ew32(FEXTNVM6, mac_data);
> -
> -		/* Disable Ungate PGCB clock */
> -		mac_data = er32(FEXTNVM9);
> -		mac_data |= BIT(28);
> -		ew32(FEXTNVM9, mac_data);
> -
> -		/* Cancel not waking from dynamic
> -		 * Power Gating with clock request
> -		 */
> -		mac_data = er32(FEXTNVM12);
> -		mac_data &= ~BIT(12);
> -		ew32(FEXTNVM12, mac_data);
> +	/* Disable the Dynamic Power Gating in the MAC */
> +	mac_data = er32(FEXTNVM7);
> +	mac_data &= 0xFFBFFFFF;
> +	ew32(FEXTNVM7, mac_data);
>   
> -		/* Cancel disable disconnected cable conditioning
> -		 * for Power Gating
> -		 */
> -		mac_data = er32(DPGFR);
> -		mac_data &= ~BIT(2);
> -		ew32(DPGFR, mac_data);
> +	/* Enable the time synchronization clock */
> +	mac_data = er32(FEXTNVM7);
> +	mac_data |= BIT(0);
> +	ew32(FEXTNVM7, mac_data);
>   
> -		/* Disable the Dynamic Clock Gating in the DMA and MAC */
> -		mac_data = er32(CTRL_EXT);
> -		mac_data &= 0xFFF7FFFF;
> -		ew32(CTRL_EXT, mac_data);
> +	/* Disable mPHY power gating for any link and speed */
> +	mac_data = er32(FEXTNVM8);
> +	mac_data &= ~BIT(9);
> +	ew32(FEXTNVM8, mac_data);
>   
> -		/* Revert the lanphypc logic to use the internal Gbe counter
> -		 * and not the PMC counter
> -		 */
> -		mac_data = er32(FEXTNVM5);
> -		mac_data &= 0xFFFFFF7F;
> -		ew32(FEXTNVM5, mac_data);
> +	/* Disable K1 off */
> +	mac_data = er32(FEXTNVM6);
> +	mac_data &= ~BIT(31);
> +	ew32(FEXTNVM6, mac_data);
>   
> -		/* Enable the periodic inband message,
> -		 * Request PCIe clock in K1 page770_17[10:9] =01b
> -		 */
> -		e1e_rphy(hw, HV_PM_CTRL, &phy_data);
> -		phy_data &= 0xFBFF;
> -		phy_data |= HV_PM_CTRL_K1_CLK_REQ;
> -		e1e_wphy(hw, HV_PM_CTRL, phy_data);
> +	/* Disable Ungate PGCB clock */
> +	mac_data = er32(FEXTNVM9);
> +	mac_data |= BIT(28);
> +	ew32(FEXTNVM9, mac_data);
>   
> -		/* Return back configuration
> -		 * 772_29[5] = 0 CS_Mode_Stay_In_K1
> -		 */
> -		e1e_rphy(hw, I217_CGFREG, &phy_data);
> -		phy_data &= 0xFFDF;
> -		e1e_wphy(hw, I217_CGFREG, phy_data);
> +	/* Cancel not waking from dynamic
> +	 * Power Gating with clock request
> +	 */
> +	mac_data = er32(FEXTNVM12);
> +	mac_data &= ~BIT(12);
> +	ew32(FEXTNVM12, mac_data);
>   
> -		/* Change the MAC/PHY interface to Kumeran
> -		 * Unforce the SMBus in PHY page769_23[0] = 0
> -		 * Unforce the SMBus in MAC CTRL_EXT[11] = 0
> -		 */
> -		e1e_rphy(hw, CV_SMB_CTRL, &phy_data);
> -		phy_data &= ~CV_SMB_CTRL_FORCE_SMBUS;
> -		e1e_wphy(hw, CV_SMB_CTRL, phy_data);
> -		mac_data = er32(CTRL_EXT);
> -		mac_data &= ~E1000_CTRL_EXT_FORCE_SMBUS;
> -		ew32(CTRL_EXT, mac_data);
> -	}
> +	/* Cancel disable disconnected cable conditioning
> +	 * for Power Gating
> +	 */
> +	mac_data = er32(DPGFR);
> +	mac_data &= ~BIT(2);
> +	ew32(DPGFR, mac_data);
>   
>   	/* Disable Dynamic Power Gating */
>   	mac_data = er32(CTRL_EXT);
>   	mac_data &= 0xFFFFFFF7;
>   	ew32(CTRL_EXT, mac_data);
>   
> -	/* Enable the time synchronization clock */
> -	mac_data = er32(FEXTNVM7);
> -	mac_data &= ~BIT(31);
> -	mac_data |= BIT(0);
> -	ew32(FEXTNVM7, mac_data);
> +	/* Disable the Dynamic Clock Gating in the DMA and MAC */
> +	mac_data = er32(CTRL_EXT);
> +	mac_data &= 0xFFF7FFFF;
> +	ew32(CTRL_EXT, mac_data);
> +
> +	/* Revert the lanphypc logic to use the internal Gbe counter
> +	 * and not the PMC counter
> +	 */
> +	mac_data = er32(FEXTNVM5);
> +	mac_data &= 0xFFFFFF7F;
> +	ew32(FEXTNVM5, mac_data);
> +
> +	/* Enable the periodic inband message,
> +	 * Request PCIe clock in K1 page770_17[10:9] =01b
> +	 */
> +	e1e_rphy(hw, HV_PM_CTRL, &phy_data);
> +	phy_data &= 0xFBFF;
> +	phy_data |= HV_PM_CTRL_K1_CLK_REQ;
> +	e1e_wphy(hw, HV_PM_CTRL, phy_data);
> +
> +	/* Return back configuration
> +	 * 772_29[5] = 0 CS_Mode_Stay_In_K1
> +	 */
> +	e1e_rphy(hw, I217_CGFREG, &phy_data);
> +	phy_data &= 0xFFDF;
> +	e1e_wphy(hw, I217_CGFREG, phy_data);
> +
> +	/* Change the MAC/PHY interface to Kumeran
> +	 * Unforce the SMBus in PHY page769_23[0] = 0
> +	 * Unforce the SMBus in MAC CTRL_EXT[11] = 0
> +	 */
> +	e1e_rphy(hw, CV_SMB_CTRL, &phy_data);
> +	phy_data &= ~CV_SMB_CTRL_FORCE_SMBUS;
> +	e1e_wphy(hw, CV_SMB_CTRL, phy_data);
> +	mac_data = er32(CTRL_EXT);
> +	mac_data &= ~E1000_CTRL_EXT_FORCE_SMBUS;
> +	ew32(CTRL_EXT, mac_data);
>   }
>   
>   static int e1000e_pm_freeze(struct device *dev)
> 
Hello Kai-Heng,
I believe it is the wrong approach. Reverting this patch will put 
corporate systems in an unpredictable state. SW will perform s0ix flow 
independent to CSME. (The CSME firmware will continue run 
independently.) LAN controller could be in an unknown state.
Please, afford us to continue to debug the problem (it is could be 
incredible complexity)

You always can skip the s0ix flow on problematic corporate systems by 
using privilege flag: ethtool --set-priv-flags enp0s31f6 s0ix-enabled off

Also, there is no impact on consumer systems.
Sasha

Re: [External] Re: [PATCH 3/3] Revert "e1000e: Add handshake with the CSME to support S0ix"

[Mark Pearson]

Hi Sasha

On 2021-11-28 08:23, Sasha Neftin wrote:
> On 11/22/2021 18:19, Kai-Heng Feng wrote:
>> This reverts commit 3e55d231716ea361b1520b801c6778c4c48de102.
>>
>> Bugzilla:
>> https://bugzilla.kernel.org/show_bug.cgi?id=214821>>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
<snip>
>>
> Hello Kai-Heng,
> I believe it is the wrong approach. Reverting this patch will put
> corporate systems in an unpredictable state. SW will perform s0ix flow
> independent to CSME. (The CSME firmware will continue run
> independently.) LAN controller could be in an unknown state.
> Please, afford us to continue to debug the problem (it is could be
> incredible complexity)
> 
> You always can skip the s0ix flow on problematic corporate systems by
> using privilege flag: ethtool --set-priv-flags enp0s31f6 s0ix-enabled off
> 
> Also, there is no impact on consumer systems.
> Sasha

I know we've discussed this offline, and your team are working on the
correct fix but I wanted to check based on your comments above that "it
was complex". I thought, and maybe misunderstood, that it was going to
be relatively simple to disable the change for older CPUs - which is the
biggest problem caused by the patch.

Right now it's breaking networking for folk who happen to have a vPro
Tigerlake (and I believe even potentially Cometlake or older) system. I
think the impact of that could potentially be quite severe.

I understand not wanting to revert the change for the ADL platforms I
believe this is targeting and to fix this instead - but your comment
made me nervous that Linux users on older Intel based platforms are in
for a long and painful wait - it is likely a lot of users....

Can you or Dima confirm the fix for older platforms will be available
soon? I appreciate the ADL platform might take a bit more work and time
to get right.

Thanks
Mark

Re: [External] Re: [PATCH 3/3] Revert "e1000e: Add handshake with the CSME to support S0ix"

[Ruinskiy, Dima]

On 30/11/2021 17:52, Mark Pearson wrote:
> Hi Sasha
> 
> On 2021-11-28 08:23, Sasha Neftin wrote:
>> On 11/22/2021 18:19, Kai-Heng Feng wrote:
>>> This reverts commit 3e55d231716ea361b1520b801c6778c4c48de102.
>>>
>>> Bugzilla:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=214821>>>
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> ---
> <snip>
>>>
>> Hello Kai-Heng,
>> I believe it is the wrong approach. Reverting this patch will put
>> corporate systems in an unpredictable state. SW will perform s0ix flow
>> independent to CSME. (The CSME firmware will continue run
>> independently.) LAN controller could be in an unknown state.
>> Please, afford us to continue to debug the problem (it is could be
>> incredible complexity)
>>
>> You always can skip the s0ix flow on problematic corporate systems by
>> using privilege flag: ethtool --set-priv-flags enp0s31f6 s0ix-enabled off
>>
>> Also, there is no impact on consumer systems.
>> Sasha
> 
> I know we've discussed this offline, and your team are working on the
> correct fix but I wanted to check based on your comments above that "it
> was complex". I thought, and maybe misunderstood, that it was going to
> be relatively simple to disable the change for older CPUs - which is the
> biggest problem caused by the patch.
> 
> Right now it's breaking networking for folk who happen to have a vPro
> Tigerlake (and I believe even potentially Cometlake or older) system. I
> think the impact of that could potentially be quite severe.
> 
> I understand not wanting to revert the change for the ADL platforms I
> believe this is targeting and to fix this instead - but your comment
> made me nervous that Linux users on older Intel based platforms are in
> for a long and painful wait - it is likely a lot of users....
> 
> Can you or Dima confirm the fix for older platforms will be available
> soon? I appreciate the ADL platform might take a bit more work and time
> to get right.
> 
> Thanks
> Mark
> 
Hi Mark,

What we currently see is that the issue manifests itself similarly on 
ADL and TGL platforms. Thus, the fix will likely be the same for both.

If we cannot find a proper fix soon, we will provide a workaround (for 
example by temporary disabling the feature on vPro platforms until we do 
have a fix).

This can be done without reverting the patch series, and I don't see 
much value in selectively disabling it for CML/TGL while leaving it on 
for ADL, unless our ongoing debug shows otherwise.

--Dima

Re: [External] Re: [PATCH 3/3] Revert "e1000e: Add handshake with the CSME to support S0ix"

[Mark Pearson]

On 2021-12-01 11:38, Ruinskiy, Dima wrote:
> On 30/11/2021 17:52, Mark Pearson wrote:
>> Hi Sasha
>>
>> On 2021-11-28 08:23, Sasha Neftin wrote:
>>> On 11/22/2021 18:19, Kai-Heng Feng wrote:
>>>> This reverts commit 3e55d231716ea361b1520b801c6778c4c48de102.
>>>>
>>>> Bugzilla:
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=214821>>>>>
>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>>> ---
>> <snip>
>>>>
>>> Hello Kai-Heng,
>>> I believe it is the wrong approach. Reverting this patch will put
>>> corporate systems in an unpredictable state. SW will perform s0ix flow
>>> independent to CSME. (The CSME firmware will continue run
>>> independently.) LAN controller could be in an unknown state.
>>> Please, afford us to continue to debug the problem (it is could be
>>> incredible complexity)
>>>
>>> You always can skip the s0ix flow on problematic corporate systems by
>>> using privilege flag: ethtool --set-priv-flags enp0s31f6 s0ix-enabled
>>> off
>>>
>>> Also, there is no impact on consumer systems.
>>> Sasha
>>
>> I know we've discussed this offline, and your team are working on the
>> correct fix but I wanted to check based on your comments above that "it
>> was complex". I thought, and maybe misunderstood, that it was going to
>> be relatively simple to disable the change for older CPUs - which is the
>> biggest problem caused by the patch.
>>
>> Right now it's breaking networking for folk who happen to have a vPro
>> Tigerlake (and I believe even potentially Cometlake or older) system. I
>> think the impact of that could potentially be quite severe.
>>
>> I understand not wanting to revert the change for the ADL platforms I
>> believe this is targeting and to fix this instead - but your comment
>> made me nervous that Linux users on older Intel based platforms are in
>> for a long and painful wait - it is likely a lot of users....
>>
>> Can you or Dima confirm the fix for older platforms will be available
>> soon? I appreciate the ADL platform might take a bit more work and time
>> to get right.
>>
>> Thanks
>> Mark
>>
> Hi Mark,
> 
> What we currently see is that the issue manifests itself similarly on
> ADL and TGL platforms. Thus, the fix will likely be the same for both.
> 
> If we cannot find a proper fix soon, we will provide a workaround (for
> example by temporary disabling the feature on vPro platforms until we do
> have a fix).
> 
> This can be done without reverting the patch series, and I don't see
> much value in selectively disabling it for CML/TGL while leaving it on
> for ADL, unless our ongoing debug shows otherwise.
> 
Got it - thanks Dima.

As a note - the obvious advantage of selectively disabling for CML/TGL
is there is a ton of those platforms out there in users hands, whereas
the ADL platforms won't be landing for a few more months (at least in
our case). I'm OK if the fixes take a touch longer with ADL (though
we'll want them soon so they have time to make it upstream and down into
the distro's) - but there's going to be a lot of unhappy Intel users as
soon as they start picking up the updates (that are landing in some
distro's) and finding that networking is broken. I'd expect TGL/CML to
be a priority...

Keep us posted when the fix is ready please.

Mark

Re: [Intel-wired-lan] [PATCH 1/3] Revert "e1000e: Additional PHY power saving in S0ix"

[Kai-Heng Feng]

Hi, Vitaly,

On Tue, Nov 23, 2021 at 11:22 PM Lifshits, Vitaly
<vitaly.lifshits@intel.com> wrote:
>
> Hello Kai,
>
>
> We believe that simply reverting these patches is not a good idea. It will cause the driver to behave on a corporate system as if the CSME firmware is not there. This can lead to an unpredictable behavior in the long run.

I really don't want to revert the series either.

>
>
> The issue exposed by these patches is currently under active debug. We would like to find the root cause and fix it in a way that will still enable S0ix power savings on both corporate and consumer systems.

I am aware. But we've been waiting for the fix for a while, so I guess
it's better to revert the series now, and re-apply them when the fix
is ready.

Kai-Heng

>
>
> On 11/22/2021 18:19, Kai-Heng Feng wrote:
>
> This reverts commit 3ad3e28cb203309fb29022dea41cd65df0583632.
>
> The s0ix series makes e1000e on TGL and ADL fails to work after s2idle
> resume.
>
> There doesn't seem to be any solution soon, so revert the whole series.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=214821
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 44e2dc8328a22..e16b7c0d98089 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -6380,16 +6380,10 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
>   ew32(CTRL_EXT, mac_data);
>
>   /* DFT control: PHY bit: page769_20[0] = 1
> - * page769_20[7] - PHY PLL stop
> - * page769_20[8] - PHY go to the electrical idle
> - * page769_20[9] - PHY serdes disable
>   * Gate PPW via EXTCNF_CTRL - set 0x0F00[7] = 1
>   */
>   e1e_rphy(hw, I82579_DFT_CTRL, &phy_data);
>   phy_data |= BIT(0);
> - phy_data |= BIT(7);
> - phy_data |= BIT(8);
> - phy_data |= BIT(9);
>   e1e_wphy(hw, I82579_DFT_CTRL, phy_data);
>
>   mac_data = er32(EXTCNF_CTRL);

Re: [Intel-wired-lan] [PATCH 1/3] Revert "e1000e: Additional PHY power saving in S0ix"

[Sasha Neftin]

On 11/24/2021 08:34, Kai-Heng Feng wrote:
> Hi, Vitaly,
> 
> On Tue, Nov 23, 2021 at 11:22 PM Lifshits, Vitaly
> <vitaly.lifshits@intel.com> wrote:
>>
>> Hello Kai,
>>
>>
>> We believe that simply reverting these patches is not a good idea. It will cause the driver to behave on a corporate system as if the CSME firmware is not there. This can lead to an unpredictable behavior in the long run.
> 
> I really don't want to revert the series either.
> 
>>
>>
>> The issue exposed by these patches is currently under active debug. We would like to find the root cause and fix it in a way that will still enable S0ix power savings on both corporate and consumer systems.
> 
> I am aware. But we've been waiting for the fix for a while, so I guess
> it's better to revert the series now, and re-apply them when the fix
> is ready.
> 
> Kai-Heng
Hello Kai-Heng,
In this patch, we addressed additional PHY power saving. There is no 
point in trying to revert it.
> 
>>
>>
>> On 11/22/2021 18:19, Kai-Heng Feng wrote:
>>
>> This reverts commit 3ad3e28cb203309fb29022dea41cd65df0583632.
>>
>> The s0ix series makes e1000e on TGL and ADL fails to work after s2idle
>> resume.
>>
>> There doesn't seem to be any solution soon, so revert the whole series.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=214821
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>>   drivers/net/ethernet/intel/e1000e/netdev.c | 6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
>> index 44e2dc8328a22..e16b7c0d98089 100644
>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>> @@ -6380,16 +6380,10 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
>>    ew32(CTRL_EXT, mac_data);
>>
>>    /* DFT control: PHY bit: page769_20[0] = 1
>> - * page769_20[7] - PHY PLL stop
>> - * page769_20[8] - PHY go to the electrical idle
>> - * page769_20[9] - PHY serdes disable
>>    * Gate PPW via EXTCNF_CTRL - set 0x0F00[7] = 1
>>    */
>>    e1e_rphy(hw, I82579_DFT_CTRL, &phy_data);
>>    phy_data |= BIT(0);
>> - phy_data |= BIT(7);
>> - phy_data |= BIT(8);
>> - phy_data |= BIT(9);
>>    e1e_wphy(hw, I82579_DFT_CTRL, phy_data);
>>
>>    mac_data = er32(EXTCNF_CTRL);
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> 
Sasha