[PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes

[PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes

[Oleksij Rempel]

This patch series improves the reliability of the Microchip LAN88xx
PHYs, particularly in edge cases involving fixed link configurations or
forced speed modes.

Patch 1 assigns genphy_soft_reset() to the .soft_reset hook to ensure
that stale link partner advertisement (LPA) bits are properly cleared
during reconfiguration. Without this, outdated autonegotiation bits may
remain visible in some parallel detection cases.

Patch 2 restricts the 100 Mbps workaround (originally intended to handle
cable length switching) to only run when the link transitions to the
PHY_NOLINK state. This prevents repeated toggling that can confuse
autonegotiating link partners such as the Intel i350, leading to
unstable link cycles.

Both patches were tested on a LAN7850 (with integrated LAN88xx PHY)
against an Intel I350 NIC. The full test suite - autonegotiation, fixed
link, and parallel detection - passed successfully.

Oleksij Rempel (2):
  net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits
  net: phy: microchip: limit 100M workaround to link-down events on
    LAN88xx

 drivers/net/phy/microchip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--
2.39.5

[PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits

[Oleksij Rempel]

Enable .soft_reset for the LAN88xx PHY driver by assigning
genphy_soft_reset() to ensure that the phylib core performs a proper
soft reset during reconfiguration.

Previously, the driver left .soft_reset unimplemented, so calls to
phy_init_hw() (e.g., from lan88xx_link_change_notify()) did not fully
reset the PHY. As a result, stale contents in the Link Partner Ability
(LPA) register could persist, causing the PHY to incorrectly report
that the link partner advertised autonegotiation even when it did not.

Using genphy_soft_reset() guarantees a clean reset of the PHY and
corrects the false autoneg reporting in these scenarios.

Fixes: ccb989e4d1ef ("net: phy: microchip: Reset LAN88xx PHY to ensure clean link state on LAN7800/7850")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/microchip.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
index 13570f628aa5..5e590b0a75e5 100644
--- a/drivers/net/phy/microchip.c
+++ b/drivers/net/phy/microchip.c
@@ -488,6 +488,7 @@ static struct phy_driver microchip_phy_driver[] = {
 	.config_init	= lan88xx_config_init,
 	.config_aneg	= lan88xx_config_aneg,
 	.link_change_notify = lan88xx_link_change_notify,
+	.soft_reset	= genphy_soft_reset,
 
 	/* Interrupt handling is broken, do not define related
 	 * functions to force polling.
-- 
2.39.5

Re: [PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits

[Andrew Lunn]

On Wed, Jul 09, 2025 at 03:07:52PM +0200, Oleksij Rempel wrote:
> Enable .soft_reset for the LAN88xx PHY driver by assigning
> genphy_soft_reset() to ensure that the phylib core performs a proper
> soft reset during reconfiguration.
> 
> Previously, the driver left .soft_reset unimplemented, so calls to
> phy_init_hw() (e.g., from lan88xx_link_change_notify()) did not fully
> reset the PHY. As a result, stale contents in the Link Partner Ability
> (LPA) register could persist, causing the PHY to incorrectly report
> that the link partner advertised autonegotiation even when it did not.
> 
> Using genphy_soft_reset() guarantees a clean reset of the PHY and
> corrects the false autoneg reporting in these scenarios.
> 
> Fixes: ccb989e4d1ef ("net: phy: microchip: Reset LAN88xx PHY to ensure clean link state on LAN7800/7850")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net v1 2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx

[Oleksij Rempel]

Restrict the 100Mbit forced-mode workaround to link-down transitions
only, to prevent repeated link reset cycles in certain configurations.

The workaround was originally introduced to improve signal reliability
when switching cables between long and short distances. It temporarily
forces the PHY into 10 Mbps before returning to 100 Mbps.

However, when used with autonegotiating link partners (e.g., Intel i350),
executing this workaround on every link change can confuse the partner
and cause constant renegotiation loops. This results in repeated link
down/up transitions and the PHY never reaching a stable state.

Limit the workaround to only run during the PHY_NOLINK state. This ensures
it is triggered only once per link drop, avoiding disruptive toggling
while still preserving its intended effect.

Note: I am not able to reproduce the original issue that this workaround
addresses. I can only confirm that 100 Mbit mode works correctly in my
test setup. Based on code inspection, I assume the workaround aims to
reset some internal state machine or signal block by toggling speeds.
However, a PHY reset is already performed earlier in the function via
phy_init_hw(), which may achieve a similar effect. Without a reproducer,
I conservatively keep the workaround but restrict its conditions.

Fixes: e57cf3639c32 ("net: lan78xx: fix accessing the LAN7800's internal phy specific registers from the MAC driver")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/microchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
index 5e590b0a75e5..dc8634e7bcbe 100644
--- a/drivers/net/phy/microchip.c
+++ b/drivers/net/phy/microchip.c
@@ -332,7 +332,7 @@ static void lan88xx_link_change_notify(struct phy_device *phydev)
 	 * As workaround, set to 10 before setting to 100
 	 * at forced 100 F/H mode.
 	 */
-	if (!phydev->autoneg && phydev->speed == 100) {
+	if (phydev->state == PHY_NOLINK && !phydev->autoneg && phydev->speed == 100) {
 		/* disable phy interrupt */
 		temp = phy_read(phydev, LAN88XX_INT_MASK);
 		temp &= ~LAN88XX_INT_MASK_MDINTPIN_EN_;
-- 
2.39.5

Re: [PATCH net v1 2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx

[Andrew Lunn]

On Wed, Jul 09, 2025 at 03:07:53PM +0200, Oleksij Rempel wrote:
> Restrict the 100Mbit forced-mode workaround to link-down transitions
> only, to prevent repeated link reset cycles in certain configurations.
> 
> The workaround was originally introduced to improve signal reliability
> when switching cables between long and short distances. It temporarily
> forces the PHY into 10 Mbps before returning to 100 Mbps.
> 
> However, when used with autonegotiating link partners (e.g., Intel i350),
> executing this workaround on every link change can confuse the partner
> and cause constant renegotiation loops. This results in repeated link
> down/up transitions and the PHY never reaching a stable state.
> 
> Limit the workaround to only run during the PHY_NOLINK state. This ensures
> it is triggered only once per link drop, avoiding disruptive toggling
> while still preserving its intended effect.
> 
> Note: I am not able to reproduce the original issue that this workaround
> addresses. I can only confirm that 100 Mbit mode works correctly in my
> test setup. Based on code inspection, I assume the workaround aims to
> reset some internal state machine or signal block by toggling speeds.
> However, a PHY reset is already performed earlier in the function via
> phy_init_hw(), which may achieve a similar effect. Without a reproducer,
> I conservatively keep the workaround but restrict its conditions.
> 
> Fixes: e57cf3639c32 ("net: lan78xx: fix accessing the LAN7800's internal phy specific registers from the MAC driver")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  9 Jul 2025 15:07:51 +0200 you wrote:
> This patch series improves the reliability of the Microchip LAN88xx
> PHYs, particularly in edge cases involving fixed link configurations or
> forced speed modes.
> 
> Patch 1 assigns genphy_soft_reset() to the .soft_reset hook to ensure
> that stale link partner advertisement (LPA) bits are properly cleared
> during reconfiguration. Without this, outdated autonegotiation bits may
> remain visible in some parallel detection cases.
> 
> [...]

Here is the summary with links:
  - [net,v1,1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits
    https://git.kernel.org/netdev/net/c/b4517c363e0e
  - [net,v1,2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx
    https://git.kernel.org/netdev/net/c/dd4360c0e850

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html