git: df34ccfc913d - main - e1000: Retry transient MDIC failures on modern PCH
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 11 Aug 2026 20:20:02 UTC
The branch main has been updated by kbowling:
URL: https://cgit.FreeBSD.org/src/commit/?id=df34ccfc913d6635e957c685d7452a05911eab89
commit df34ccfc913d6635e957c685d7452a05911eab89
Author: Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-11 16:15:28 +0000
Commit: Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-08-11 20:19:19 +0000
e1000: Retry transient MDIC failures on modern PCH
Some Meteor Lake and newer systems sporadically fail an MDIC PHY
transaction while the MAC and PHY clocks synchronize. Retry twice
before reporting the transaction failure.
Disable retries around PHY interface transitions where an MDI error
is expected. Preserve and restore the configured retry count on every
exit from those flows.
This follows DPDK commit bdca22d62ff0, extended to the PTP and NVP PCH
types.
MFC after: 2 weeks
---
sys/dev/e1000/e1000_hw.h | 1 +
sys/dev/e1000/e1000_ich8lan.c | 26 ++++++
sys/dev/e1000/e1000_phy.c | 196 +++++++++++++++++++++++++-----------------
sys/dev/e1000/e1000_phy.h | 4 +
4 files changed, 148 insertions(+), 79 deletions(-)
diff --git a/sys/dev/e1000/e1000_hw.h b/sys/dev/e1000/e1000_hw.h
index 0bd99778ba01..75f8953f2fdb 100644
--- a/sys/dev/e1000/e1000_hw.h
+++ b/sys/dev/e1000/e1000_hw.h
@@ -886,6 +886,7 @@ struct e1000_phy_info {
u32 id;
u32 reset_delay_us; /* in usec */
u32 revision;
+ u32 current_retry_counter;
enum e1000_media_type media_type;
diff --git a/sys/dev/e1000/e1000_ich8lan.c b/sys/dev/e1000/e1000_ich8lan.c
index 1a67f4d8f42d..6090b8995718 100644
--- a/sys/dev/e1000/e1000_ich8lan.c
+++ b/sys/dev/e1000/e1000_ich8lan.c
@@ -202,6 +202,7 @@ static bool e1000_phy_is_accessible_pchlan(struct e1000_hw *hw)
{
u16 phy_reg = 0;
u32 phy_id = 0;
+ u32 phy_retries;
s32 ret_val = 0;
u16 retry_count;
u32 mac_reg = 0;
@@ -248,11 +249,18 @@ out:
/* Only unforce SMBus if ME is not active */
if (!(E1000_READ_REG(hw, E1000_FWSM) &
E1000_ICH_FWSM_FW_VALID)) {
+ /* Switching the PHY interface returns an expected MDI
+ * error. Do not retry that transaction.
+ */
+ e1000_disable_phy_retry_mechanism(hw, &phy_retries);
+
/* Unforce SMBus mode in PHY */
hw->phy.ops.read_reg_locked(hw, CV_SMB_CTRL, &phy_reg);
phy_reg &= ~CV_SMB_CTRL_FORCE_SMBUS;
hw->phy.ops.write_reg_locked(hw, CV_SMB_CTRL, phy_reg);
+ e1000_enable_phy_retry_mechanism(hw, phy_retries);
+
/* Unforce SMBus mode in MAC */
mac_reg = E1000_READ_REG(hw, E1000_CTRL_EXT);
mac_reg &= ~E1000_CTRL_EXT_FORCE_SMBUS;
@@ -317,6 +325,7 @@ static void e1000_toggle_lanphypc_pch_lpt(struct e1000_hw *hw)
static s32 e1000_init_phy_workarounds_pchlan(struct e1000_hw *hw)
{
u32 mac_reg, fwsm = E1000_READ_REG(hw, E1000_FWSM);
+ u32 phy_retries;
s32 ret_val;
DEBUGFUNC("e1000_init_phy_workarounds_pchlan");
@@ -340,6 +349,9 @@ static s32 e1000_init_phy_workarounds_pchlan(struct e1000_hw *hw)
goto out;
}
+ /* The PHY might be inaccessible while its interface is changing. */
+ e1000_disable_phy_retry_mechanism(hw, &phy_retries);
+
/* The MAC-PHY interconnect may be in SMBus mode. If the PHY is
* inaccessible and resetting the PHY is not blocked, toggle the
* LANPHYPC Value bit to force the interconnect to PCIe mode.
@@ -409,6 +421,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct e1000_hw *hw)
break;
}
+ e1000_enable_phy_retry_mechanism(hw, phy_retries);
hw->phy.ops.release(hw);
if (!ret_val) {
@@ -484,6 +497,9 @@ static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
phy->autoneg_mask = AUTONEG_ADVERTISE_SPEED_DEFAULT;
phy->id = e1000_phy_unknown;
+ if (hw->mac.type >= e1000_pch_mtp &&
+ hw->mac.type < e1000_82575)
+ phy->current_retry_counter = 2;
ret_val = e1000_init_phy_workarounds_pchlan(hw);
if (ret_val)
@@ -1275,6 +1291,7 @@ static s32 e1000_set_obff_timer_pch_lpt(struct e1000_hw *hw, u32 itr)
s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool to_sx)
{
u32 mac_reg;
+ u32 phy_retries;
s32 ret_val = E1000_SUCCESS;
u16 phy_reg;
u16 oem_reg = 0;
@@ -1322,6 +1339,9 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool to_sx)
if (ret_val)
goto out;
+ /* Switching the PHY interface returns an expected MDI error. */
+ e1000_disable_phy_retry_mechanism(hw, &phy_retries);
+
/* Force SMBus mode in PHY */
ret_val = e1000_read_phy_reg_hv_locked(hw, CV_SMB_CTRL, &phy_reg);
if (ret_val)
@@ -1394,6 +1414,7 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool to_sx)
}
release:
+ e1000_enable_phy_retry_mechanism(hw, phy_retries);
hw->phy.ops.release(hw);
out:
if (ret_val)
@@ -1424,6 +1445,7 @@ s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force)
s32 ret_val = E1000_SUCCESS;
u8 ulp_exit_timeout = 30;
u32 mac_reg;
+ u32 phy_retries;
u16 phy_reg;
int i = 0;
@@ -1480,6 +1502,9 @@ s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force)
/* Toggle LANPHYPC Value bit */
e1000_toggle_lanphypc_pch_lpt(hw);
+ /* Switching the PHY interface returns an expected MDI error. */
+ e1000_disable_phy_retry_mechanism(hw, &phy_retries);
+
/* Unforce SMBus mode in PHY */
ret_val = e1000_read_phy_reg_hv_locked(hw, CV_SMB_CTRL, &phy_reg);
if (ret_val) {
@@ -1538,6 +1563,7 @@ s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force)
E1000_WRITE_REG(hw, E1000_FEXTNVM7, mac_reg);
release:
+ e1000_enable_phy_retry_mechanism(hw, phy_retries);
hw->phy.ops.release(hw);
if (force) {
hw->phy.ops.reset(hw);
diff --git a/sys/dev/e1000/e1000_phy.c b/sys/dev/e1000/e1000_phy.c
index 5fec60e797b3..e3c8383d012e 100644
--- a/sys/dev/e1000/e1000_phy.c
+++ b/sys/dev/e1000/e1000_phy.c
@@ -278,6 +278,27 @@ s32 e1000_phy_reset_dsp_generic(struct e1000_hw *hw)
return hw->phy.ops.write_reg(hw, M88E1000_PHY_GEN_CONTROL, 0);
}
+void
+e1000_disable_phy_retry_mechanism(struct e1000_hw *hw,
+ u32 *phy_retries_original)
+{
+
+ DEBUGFUNC("e1000_disable_phy_retry_mechanism");
+
+ *phy_retries_original = hw->phy.current_retry_counter;
+ hw->phy.current_retry_counter = 0;
+}
+
+void
+e1000_enable_phy_retry_mechanism(struct e1000_hw *hw,
+ u32 phy_retries_original)
+{
+
+ DEBUGFUNC("e1000_enable_phy_retry_mechanism");
+
+ hw->phy.current_retry_counter = phy_retries_original;
+}
+
/**
* e1000_read_phy_reg_mdic - Read MDI control register
* @hw: pointer to the HW structure
@@ -290,7 +311,8 @@ s32 e1000_phy_reset_dsp_generic(struct e1000_hw *hw)
s32 e1000_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
{
struct e1000_phy_info *phy = &hw->phy;
- u32 i, mdic = 0;
+ u32 i, mdic = 0, retry_counter;
+ bool success;
DEBUGFUNC("e1000_read_phy_reg_mdic");
@@ -299,49 +321,59 @@ s32 e1000_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
return -E1000_ERR_PARAM;
}
- /* 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.
+ /* Set up and execute the transaction once, plus any configured
+ * retries. Newer PCH generations can transiently fail MDIC
+ * transactions while the MAC and PHY clocks synchronize.
*/
- mdic = ((offset << E1000_MDIC_REG_SHIFT) |
- (phy->addr << E1000_MDIC_PHY_SHIFT) |
- (E1000_MDIC_OP_READ));
+ for (retry_counter = 0;
+ retry_counter <= phy->current_retry_counter; retry_counter++) {
+ success = true;
+ mdic = ((offset << E1000_MDIC_REG_SHIFT) |
+ (phy->addr << E1000_MDIC_PHY_SHIFT) |
+ E1000_MDIC_OP_READ);
+ E1000_WRITE_REG(hw, E1000_MDIC, mdic);
+
+ /* Poll the ready bit to see if the MDI read completed.
+ * Increasing the timeout avoided failures seen in testing.
+ */
+ for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
+ usec_delay_irq(50);
+ mdic = E1000_READ_REG(hw, E1000_MDIC);
+ if (mdic & E1000_MDIC_READY)
+ break;
+ }
+ if (!(mdic & E1000_MDIC_READY)) {
+ DEBUGOUT("MDI Read did not complete\n");
+ success = false;
+ }
+ if (mdic & E1000_MDIC_ERROR) {
+ DEBUGOUT("MDI Error\n");
+ success = false;
+ }
+ if (((mdic & E1000_MDIC_REG_MASK) >>
+ E1000_MDIC_REG_SHIFT) != offset) {
+ DEBUGOUT2("MDI Read offset error - requested %d, "
+ "returned %d\n", offset,
+ (mdic & E1000_MDIC_REG_MASK) >>
+ E1000_MDIC_REG_SHIFT);
+ success = false;
+ }
- E1000_WRITE_REG(hw, E1000_MDIC, mdic);
+ /* Avoid duplicate data in the next MDIC transaction. */
+ if (hw->mac.type == e1000_pch2lan)
+ usec_delay_irq(100);
- /* 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++) {
- usec_delay_irq(50);
- mdic = E1000_READ_REG(hw, E1000_MDIC);
- if (mdic & E1000_MDIC_READY)
- break;
- }
- if (!(mdic & E1000_MDIC_READY)) {
- DEBUGOUT("MDI Read did not complete\n");
- return -E1000_ERR_PHY;
- }
- if (mdic & E1000_MDIC_ERROR) {
- DEBUGOUT("MDI Error\n");
- return -E1000_ERR_PHY;
- }
- if (((mdic & E1000_MDIC_REG_MASK) >> E1000_MDIC_REG_SHIFT) != offset) {
- DEBUGOUT2("MDI Read offset error - requested %d, returned %d\n",
- offset,
- (mdic & E1000_MDIC_REG_MASK) >> E1000_MDIC_REG_SHIFT);
- return -E1000_ERR_PHY;
+ if (success) {
+ *data = (u16)mdic;
+ return E1000_SUCCESS;
+ }
+ if (retry_counter != phy->current_retry_counter) {
+ DEBUGOUT("Retrying PHY transaction\n");
+ msec_delay_irq(10);
+ }
}
- *data = (u16) mdic;
-
- /* Allow some time after each MDIC transaction to avoid
- * reading duplicate data in the next MDIC transaction.
- */
- if (hw->mac.type == e1000_pch2lan)
- usec_delay_irq(100);
- return E1000_SUCCESS;
+ return -E1000_ERR_PHY;
}
/**
@@ -355,7 +387,8 @@ s32 e1000_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
s32 e1000_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data)
{
struct e1000_phy_info *phy = &hw->phy;
- u32 i, mdic = 0;
+ u32 i, mdic = 0, retry_counter;
+ bool success;
DEBUGFUNC("e1000_write_phy_reg_mdic");
@@ -364,49 +397,54 @@ s32 e1000_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data)
return -E1000_ERR_PARAM;
}
- /* 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 = (((u32)data) |
- (offset << E1000_MDIC_REG_SHIFT) |
- (phy->addr << E1000_MDIC_PHY_SHIFT) |
- (E1000_MDIC_OP_WRITE));
+ for (retry_counter = 0;
+ retry_counter <= phy->current_retry_counter; retry_counter++) {
+ success = true;
+ mdic = ((u32)data |
+ (offset << E1000_MDIC_REG_SHIFT) |
+ (phy->addr << E1000_MDIC_PHY_SHIFT) |
+ E1000_MDIC_OP_WRITE);
+ E1000_WRITE_REG(hw, E1000_MDIC, mdic);
+
+ /* Poll the ready bit to see if the MDI write completed.
+ * Increasing the timeout avoided failures seen in testing.
+ */
+ for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
+ usec_delay_irq(50);
+ mdic = E1000_READ_REG(hw, E1000_MDIC);
+ if (mdic & E1000_MDIC_READY)
+ break;
+ }
+ if (!(mdic & E1000_MDIC_READY)) {
+ DEBUGOUT("MDI Write did not complete\n");
+ success = false;
+ }
+ if (mdic & E1000_MDIC_ERROR) {
+ DEBUGOUT("MDI Error\n");
+ success = false;
+ }
+ if (((mdic & E1000_MDIC_REG_MASK) >>
+ E1000_MDIC_REG_SHIFT) != offset) {
+ DEBUGOUT2("MDI Write offset error - requested %d, "
+ "returned %d\n", offset,
+ (mdic & E1000_MDIC_REG_MASK) >>
+ E1000_MDIC_REG_SHIFT);
+ success = false;
+ }
- E1000_WRITE_REG(hw, E1000_MDIC, mdic);
+ /* Avoid duplicate data in the next MDIC transaction. */
+ if (hw->mac.type == e1000_pch2lan)
+ usec_delay_irq(100);
- /* 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++) {
- usec_delay_irq(50);
- mdic = E1000_READ_REG(hw, E1000_MDIC);
- if (mdic & E1000_MDIC_READY)
- break;
- }
- if (!(mdic & E1000_MDIC_READY)) {
- DEBUGOUT("MDI Write did not complete\n");
- return -E1000_ERR_PHY;
- }
- if (mdic & E1000_MDIC_ERROR) {
- DEBUGOUT("MDI Error\n");
- return -E1000_ERR_PHY;
- }
- if (((mdic & E1000_MDIC_REG_MASK) >> E1000_MDIC_REG_SHIFT) != offset) {
- DEBUGOUT2("MDI Write offset error - requested %d, returned %d\n",
- offset,
- (mdic & E1000_MDIC_REG_MASK) >> E1000_MDIC_REG_SHIFT);
- return -E1000_ERR_PHY;
+ if (success)
+ return E1000_SUCCESS;
+ if (retry_counter != phy->current_retry_counter) {
+ DEBUGOUT("Retrying PHY transaction\n");
+ msec_delay_irq(10);
+ }
}
- /* Allow some time after each MDIC transaction to avoid
- * reading duplicate data in the next MDIC transaction.
- */
- if (hw->mac.type == e1000_pch2lan)
- usec_delay_irq(100);
-
- return E1000_SUCCESS;
+ return -E1000_ERR_PHY;
}
/**
diff --git a/sys/dev/e1000/e1000_phy.h b/sys/dev/e1000/e1000_phy.h
index 147e542dfe0b..6020e260dcb3 100644
--- a/sys/dev/e1000/e1000_phy.h
+++ b/sys/dev/e1000/e1000_phy.h
@@ -70,6 +70,10 @@ s32 e1000_phy_sw_reset_generic(struct e1000_hw *hw);
void e1000_phy_force_speed_duplex_setup(struct e1000_hw *hw, u16 *phy_ctrl);
s32 e1000_phy_hw_reset_generic(struct e1000_hw *hw);
s32 e1000_phy_reset_dsp_generic(struct e1000_hw *hw);
+void e1000_disable_phy_retry_mechanism(struct e1000_hw *hw,
+ u32 *phy_retries_original);
+void e1000_enable_phy_retry_mechanism(struct e1000_hw *hw,
+ u32 phy_retries_original);
s32 e1000_read_kmrn_reg_generic(struct e1000_hw *hw, u32 offset, u16 *data);
s32 e1000_read_kmrn_reg_locked(struct e1000_hw *hw, u32 offset, u16 *data);
s32 e1000_set_page_igp(struct e1000_hw *hw, u16 page);