From: Rohan G Thomas rohan.g.thomas@altera.com
[ Upstream commit deb105f49879dd50d595f7f55207d6e74dec34e6 ]
The 88e1510 PHY has an erratum where the phy downshift counter is not cleared after phy being suspended(BMCR_PDOWN set) and then later resumed(BMCR_PDOWN cleared). This can cause the gigabit link to intermittently downshift to a lower speed.
Disabling and re-enabling the downshift feature clears the counter, allowing the PHY to retry gigabit link negotiation up to the programmed retry count times before downshifting. This behavior has been observed on copper links.
Signed-off-by: Rohan G Thomas rohan.g.thomas@altera.com Reviewed-by: Matthew Gerlach matthew.gerlach@altera.com Reviewed-by: Andrew Lunn andrew@lunn.ch Link: https://patch.msgid.link/20250906-marvell_fix-v2-1-f6efb286937f@altera.com Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- Bug fixed and user impact: - The commit addresses a real erratum on 88E1510 where the PHY downshift counter is not cleared across suspend/resume, which can cause intermittent, user-visible downshift from gigabit to lower speeds on copper links.
- What the patch changes: - Adds a device-specific resume wrapper `m88e1510_resume()` which first performs the normal resume sequence and then clears the stale downshift counter by toggling the downshift feature off and back on with the existing configured retry count. - `drivers/net/phy/marvell.c:1915` defines `m88e1510_resume(struct phy_device *phydev)`: it calls `marvell_resume()` to do the standard fiber/copper resume, then reads the configured downshift count via `m88e1011_get_downshift()`. If non-zero, it disables and re-enables downshift with the same count to clear the counter. - `drivers/net/phy/marvell.c:1875` shows `marvell_resume(struct phy_device *phydev)`, which handles the dual-mode (fiber/copper) page sequencing and invokes `genphy_resume()`. `m88e1510_resume()` invokes this first to keep existing resume behavior intact. - `drivers/net/phy/marvell.c:1138` `m88e1011_get_downshift()` reads the current downshift configuration (returns 0 if disabled). - `drivers/net/phy/marvell.c:1154` `m88e1011_set_downshift()` programs the downshift count and performs a soft reset to apply the change, which is exactly what is needed to reliably clear the counter. - Hooks the new resume into the 88E1510 driver entry only: - `drivers/net/phy/marvell.c:3961` sets `.resume = m88e1510_resume` for `MARVELL_PHY_ID_88E1510`, replacing the generic `marvell_resume` only for that PHY.
- Why it’s safe and minimal: - Scope-limited: Only 88E1510’s `.resume` is changed; other Marvell PHYs keep their existing resume paths. - No API or architectural changes: The patch only introduces a small wrapper and uses existing helper functions already used elsewhere in this driver. - Preserves user configuration: It reads the current downshift setting and restores the same count, doing nothing if downshift is disabled (`cnt == 0`), so it does not override user-set policy. - Correct sequencing and pages: `m88e1510_resume()` defers to `marvell_resume()` first, which restores the page to copper before calling the downshift helpers. The helpers operate on the copper page registers. - Side effects are minimal and expected: `m88e1011_set_downshift()` performs a soft reset to apply changes; the wrapper may cause two quick resets (disable then re-enable), slightly delaying link bring- up on resume but preventing the intermittent low-speed fallback — a clear net improvement for users.
- Stable backport criteria: - Fixes a real, user-facing bug (intermittent downshift after resume). - Small, isolated change to a single driver with no cross-subsystem impact. - Low regression risk and no new features or behavior changes beyond clearing the erratum condition. - Aligns with existing driver patterns and uses proven helper functions.
Given the above, this is a good candidate for stable backporting.
drivers/net/phy/marvell.c | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 623292948fa70..0ea366c1217eb 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -1902,6 +1902,43 @@ static int marvell_resume(struct phy_device *phydev) return err; }
+/* m88e1510_resume + * + * The 88e1510 PHY has an erratum where the phy downshift counter is not cleared + * after phy being suspended(BMCR_PDOWN set) and then later resumed(BMCR_PDOWN + * cleared). This can cause the link to intermittently downshift to a lower speed. + * + * Disabling and re-enabling the downshift feature clears the counter, allowing + * the PHY to retry gigabit link negotiation up to the programmed retry count + * before downshifting. This behavior has been observed on copper links. + */ +static int m88e1510_resume(struct phy_device *phydev) +{ + int err; + u8 cnt = 0; + + err = marvell_resume(phydev); + if (err < 0) + return err; + + /* read downshift counter value */ + err = m88e1011_get_downshift(phydev, &cnt); + if (err < 0) + return err; + + if (cnt) { + /* downshift disabled */ + err = m88e1011_set_downshift(phydev, 0); + if (err < 0) + return err; + + /* downshift enabled, with previous counter value */ + err = m88e1011_set_downshift(phydev, cnt); + } + + return err; +} + static int marvell_aneg_done(struct phy_device *phydev) { int retval = phy_read(phydev, MII_M1011_PHY_STATUS); @@ -3923,7 +3960,7 @@ static struct phy_driver marvell_drivers[] = { .handle_interrupt = marvell_handle_interrupt, .get_wol = m88e1318_get_wol, .set_wol = m88e1318_set_wol, - .resume = marvell_resume, + .resume = m88e1510_resume, .suspend = marvell_suspend, .read_page = marvell_read_page, .write_page = marvell_write_page,