netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes
@ 2025-07-09 13:07 Oleksij Rempel
  2025-07-09 13:07 ` [PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits Oleksij Rempel
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Oleksij Rempel @ 2025-07-09 13:07 UTC (permalink / raw)
  To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli,
	Jakub Kicinski, Paolo Abeni, Woojung Huh, Arun Ramadoss,
	Heiner Kallweit, Russell King, Yuiko Oshino
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
	Phil Elwell

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits
  2025-07-09 13:07 [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes Oleksij Rempel
@ 2025-07-09 13:07 ` Oleksij Rempel
  2025-07-09 14:16   ` Andrew Lunn
  2025-07-09 13:07 ` [PATCH net v1 2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx Oleksij Rempel
  2025-07-11  1:20 ` [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Oleksij Rempel @ 2025-07-09 13:07 UTC (permalink / raw)
  To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli,
	Jakub Kicinski, Paolo Abeni, Woojung Huh, Arun Ramadoss,
	Heiner Kallweit, Russell King, Yuiko Oshino
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
	Phil Elwell

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net v1 2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx
  2025-07-09 13:07 [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes Oleksij Rempel
  2025-07-09 13:07 ` [PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits Oleksij Rempel
@ 2025-07-09 13:07 ` Oleksij Rempel
  2025-07-09 14:17   ` Andrew Lunn
  2025-07-11  1:20 ` [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Oleksij Rempel @ 2025-07-09 13:07 UTC (permalink / raw)
  To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli,
	Jakub Kicinski, Paolo Abeni, Woojung Huh, Arun Ramadoss,
	Heiner Kallweit, Russell King, Yuiko Oshino
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
	Phil Elwell

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits
  2025-07-09 13:07 ` [PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits Oleksij Rempel
@ 2025-07-09 14:16   ` Andrew Lunn
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2025-07-09 14:16 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
	Paolo Abeni, Woojung Huh, Arun Ramadoss, Heiner Kallweit,
	Russell King, Yuiko Oshino, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v1 2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx
  2025-07-09 13:07 ` [PATCH net v1 2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx Oleksij Rempel
@ 2025-07-09 14:17   ` Andrew Lunn
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2025-07-09 14:17 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
	Paolo Abeni, Woojung Huh, Arun Ramadoss, Heiner Kallweit,
	Russell King, Yuiko Oshino, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes
  2025-07-09 13:07 [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes Oleksij Rempel
  2025-07-09 13:07 ` [PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits Oleksij Rempel
  2025-07-09 13:07 ` [PATCH net v1 2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx Oleksij Rempel
@ 2025-07-11  1:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-07-11  1:20 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: davem, andrew, edumazet, f.fainelli, kuba, pabeni, woojung.huh,
	arun.ramadoss, hkallweit1, linux, yuiko.oshino, kernel,
	linux-kernel, netdev, UNGLinuxDriver, phil

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-11  1:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-09 13:07 [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes Oleksij Rempel
2025-07-09 13:07 ` [PATCH net v1 1/2] net: phy: microchip: Use genphy_soft_reset() to purge stale LPA bits Oleksij Rempel
2025-07-09 14:16   ` Andrew Lunn
2025-07-09 13:07 ` [PATCH net v1 2/2] net: phy: microchip: limit 100M workaround to link-down events on LAN88xx Oleksij Rempel
2025-07-09 14:17   ` Andrew Lunn
2025-07-11  1:20 ` [PATCH net v1 0/2] net: phy: microchip: LAN88xx reliability fixes patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).