Re: [PATCH 6.8 117/273] e1000e: Workaround for sporadic MDI error on Meteor Lake systems

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08. 04. 24, 14:56, Greg Kroah-Hartman wrote:
6.8-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Vitaly Lifshits <vitaly.lifshits@xxxxxxxxx>

commit 6dbdd4de0362c37e54e8b049781402e5a409e7d0 upstream.

On some Meteor Lake systems accessing the PHY via the MDIO interface may
result in an MDI error. This issue happens sporadically and in most cases
a second access to the PHY via the MDIO interface results in success.

As a workaround, introduce a retry counter which is set to 3 on Meteor
Lake systems. The driver will only return an error if 3 consecutive PHY
access attempts fail. The retry mechanism is disabled in specific flows,
where MDI errors are expected.
...
--- a/drivers/net/ethernet/intel/e1000e/phy.c
+++ b/drivers/net/ethernet/intel/e1000e/phy.c
@@ -107,6 +107,16 @@ s32 e1000e_phy_reset_dsp(struct e1000_hw
  	return e1e_wphy(hw, M88E1000_PHY_GEN_CONTROL, 0);
  }
+void e1000e_disable_phy_retry(struct e1000_hw *hw)
+{
+	hw->phy.retry_enabled = false;
+}
+
+void e1000e_enable_phy_retry(struct e1000_hw *hw)
+{
+	hw->phy.retry_enabled = true;
+}
+
  /**
   *  e1000e_read_phy_reg_mdic - Read MDI control register
   *  @hw: pointer to the HW structure
@@ -118,55 +128,73 @@ s32 e1000e_phy_reset_dsp(struct e1000_hw
   **/
  s32 e1000e_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
  {
+	u32 i, mdic = 0, retry_counter, retry_max;
  	struct e1000_phy_info *phy = &hw->phy;
-	u32 i, mdic = 0;
+	bool success;
if (offset > MAX_PHY_REG_ADDRESS) {
  		e_dbg("PHY Address %d is out of range\n", offset);
  		return -E1000_ERR_PARAM;
  	}
+ retry_max = phy->retry_enabled ? phy->retry_count : 0;
+
  	/* Set up Op-code, Phy Address, and register offset in the MDI
  	 * Control register.  The MAC will take care of interfacing with the
  	 * PHY to retrieve the desired data.
  	 */
-	mdic = ((offset << E1000_MDIC_REG_SHIFT) |
-		(phy->addr << E1000_MDIC_PHY_SHIFT) |
-		(E1000_MDIC_OP_READ));
-
-	ew32(MDIC, mdic);
-
-	/* Poll the ready bit to see if the MDI read completed
-	 * Increasing the time out as testing showed failures with
-	 * the lower time out
-	 */
-	for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
-		udelay(50);
-		mdic = er32(MDIC);
-		if (mdic & E1000_MDIC_READY)
-			break;
-	}
-	if (!(mdic & E1000_MDIC_READY)) {
-		e_dbg("MDI Read PHY Reg Address %d did not complete\n", offset);
-		return -E1000_ERR_PHY;
-	}
-	if (mdic & E1000_MDIC_ERROR) {
-		e_dbg("MDI Read PHY Reg Address %d Error\n", offset);
-		return -E1000_ERR_PHY;
-	}
-	if (FIELD_GET(E1000_MDIC_REG_MASK, mdic) != offset) {
-		e_dbg("MDI Read offset error - requested %d, returned %d\n",
-		      offset, FIELD_GET(E1000_MDIC_REG_MASK, mdic));
-		return -E1000_ERR_PHY;
+	for (retry_counter = 0; retry_counter <= retry_max; retry_counter++) {
+		success = true;
+
+		mdic = ((offset << E1000_MDIC_REG_SHIFT) |
+			(phy->addr << E1000_MDIC_PHY_SHIFT) |
+			(E1000_MDIC_OP_READ));
+
+		ew32(MDIC, mdic);
+
+		/* Poll the ready bit to see if the MDI read completed
+		 * Increasing the time out as testing showed failures with
+		 * the lower time out
+		 */
+		for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
+			usleep_range(50, 60);

This crashes the kernel as a spinlock is held upper in the call stack in e1000_watchdog_task():
  spin_lock(&adapter->stats64_lock);
  e1000e_update_stats(adapter);
  -> e1000e_update_phy_stats()
     -> e1000e_read_phy_reg_mdic()
        -> usleep_range() ----> Boom.

It was reported to our bugzilla:
https://bugzilla.suse.com/show_bug.cgi?id=1223109

I believe, the mainline has the same bug.

Any ideas?

thanks,
--
js
suse labs





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux