[PATCH net v4 0/3] net: phy: smsc: use IRQ + relaxed polling to fix missed link-up

[PATCH net v4 0/3] net: phy: smsc: use IRQ + relaxed polling to fix missed link-up

[Oleksij Rempel]

This series makes the SMSC LAN8700 (as used in LAN9512 and similar USB
adapters) reliable again in configurations where it is forced to 10 Mb/s
and the link partner still advertises autonegotiation.

In this scenario, the PHY may miss the final link-up interrupt, causing
the network interface to remain down even though a valid link is
present.

To address this:

Patch 1 – phylib: Enable polling if the driver implements
get_next_update_time(). This ensures the state machine is active even
without update_stats().

Patch 2 – phylib: Allow drivers to return PHY_STATE_IRQ to explicitly
disable polling.

Patch 3 – smsc: Implement get_next_update_time() with adaptive 1 Hz
polling for up to 30 seconds after the last interrupt in the affected
10M autoneg-off mode.  All other configurations rely on IRQs only.

Testing:

The LAN9512 (LAN8700 core) was tested against an Intel I350 NIC using
baseline, parallel-detection, and advertisement test suites. All
relevant tests passed.

Changes in v4:
- address -Wformat-security for WARN_ONCE()

Changes in v3:
- handle conflicting configuration if update_stats are supported

Changes in v2:
- Introduced explicit disable polling via PHY_STATE_IRQ
- Changed the workaround logic to apply 1 Hz polling only for 30 seconds
  after the last IRQ
- Dropped relaxed 30s polling while link is up
- Reworded commit messages and comments to reflect updated logic
- Split core changes into two separate patches for clarity

Thanks,
Oleksij Rempel

Oleksij Rempel (3):
  net: phy: enable polling when driver implements get_next_update_time
  net: phy: allow drivers to disable polling via get_next_update_time()
  net: phy: smsc: recover missed link-up IRQs on LAN8700 with adaptive
    polling

 drivers/net/phy/phy.c  | 27 ++++++++++++++++++++-------
 drivers/net/phy/smsc.c | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/phy.h    | 17 +++++++++++++++--
 3 files changed, 75 insertions(+), 9 deletions(-)

--
2.39.5

[PATCH net v4 1/3] net: phy: enable polling when driver implements get_next_update_time

[Oleksij Rempel]

Currently, phy_polling_mode() enables polling only if:
- the PHY is in interrupt-less mode, or
- the driver provides an update_stats() callback.

This excludes drivers that implement get_next_update_time()
to support adaptive polling but do not provide update_stats().
As a result, the state machine timer will not run, and the
get_next_update_time() callback is never used.

This patch extends the polling condition to include drivers that
implement get_next_update_time(). This change is required to support
adaptive polling in the SMSC LAN9512/LAN8700 PHY family, which cannot
reliably use interrupts.

No in-tree drivers rely on this mechanism yet, so existing behavior is
unchanged. If any out-of-tree driver incorrectly implements
get_next_update_time(), enabling polling is still the correct behavior.

Fixes: 8bf47e4d7b87 ("net: phy: Add support for driver-specific next update time")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
changes v2:
- update commit message
---
 include/linux/phy.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 4c2b8b6e7187..2688c0435b9b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1636,7 +1636,7 @@ static inline bool phy_polling_mode(struct phy_device *phydev)
 		if (phydev->drv->flags & PHY_POLL_CABLE_TEST)
 			return true;
 
-	if (phydev->drv->update_stats)
+	if (phydev->drv->update_stats || phydev->drv->get_next_update_time)
 		return true;
 
 	return phydev->irq == PHY_POLL;
-- 
2.39.5

[PATCH net v4 2/3] net: phy: allow drivers to disable polling via get_next_update_time()

[Oleksij Rempel]

Some PHY drivers can reliably report link-down events via IRQs,
but may fail to generate reliable link-up IRQs. To support such
cases, polling is often needed - but only selectively.

Extend get_next_update_time() so drivers can return PHY_STATE_IRQ
to indicate that polling is not needed and IRQs are sufficient.
This allows finer control over PHY state machine behavior.

Introduce PHY_STATE_IRQ (UINT_MAX) as a sentinel value, and move
PHY_STATE_TIME to phy.h to allow consistent use across the codebase.

This change complements the previous patch enabling polling when
get_next_update_time() is present.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v4:
- address -Wformat-security for WARN_ONCE()
changes v3:
- handle conflicting configuration if update_stats are supported
changes v2:
- this patch is added
---
 drivers/net/phy/phy.c | 27 ++++++++++++++++++++-------
 include/linux/phy.h   | 15 ++++++++++++++-
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 13df28445f02..8f6d5a95c777 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -39,8 +39,6 @@
 #include "phylib-internal.h"
 #include "phy-caps.h"
 
-#define PHY_STATE_TIME	HZ
-
 #define PHY_STATE_STR(_state)			\
 	case PHY_##_state:			\
 		return __stringify(_state);	\
@@ -1575,16 +1573,31 @@ static enum phy_state_work _phy_state_machine(struct phy_device *phydev)
 	phy_process_state_change(phydev, old_state);
 
 	/* Only re-schedule a PHY state machine change if we are polling the
-	 * PHY, if PHY_MAC_INTERRUPT is set, then we will be moving
-	 * between states from phy_mac_interrupt().
+	 * PHY. If PHY_MAC_INTERRUPT is set or get_next_update_time() returns
+	 * PHY_STATE_IRQ, then we rely on interrupts for state changes.
 	 *
 	 * In state PHY_HALTED the PHY gets suspended, so rescheduling the
 	 * state machine would be pointless and possibly error prone when
 	 * called from phy_disconnect() synchronously.
 	 */
-	if (phy_polling_mode(phydev) && phy_is_started(phydev))
-		phy_queue_state_machine(phydev,
-					phy_get_next_update_time(phydev));
+	if (phy_polling_mode(phydev) && phy_is_started(phydev)) {
+		unsigned int next_time = phy_get_next_update_time(phydev);
+
+		if (next_time == PHY_STATE_IRQ) {
+			/* A driver requesting IRQ mode while also needing
+			 * polling for stats has a conflicting configuration.
+			 * Warn about this buggy driver and fall back to
+			 * polling to ensure stats are updated.
+			 */
+			if (phydev->drv->update_stats) {
+				WARN_ONCE(1, "phy: %s: driver requested IRQ mode but needs polling for stats\n",
+					  phydev_name(phydev));
+				phy_queue_state_machine(phydev, PHY_STATE_TIME);
+			}
+		} else {
+			phy_queue_state_machine(phydev, next_time);
+		}
+	}
 
 	return state_work;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2688c0435b9b..673477700416 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -66,6 +66,11 @@ extern const int phy_basic_ports_array[3];
 #define PHY_ALWAYS_CALL_SUSPEND	0x00000008
 #define MDIO_DEVICE_IS_PHY	0x80000000
 
+/* Default polling interval */
+#define PHY_STATE_TIME		HZ
+/* disable polling, rely on IRQs */
+#define PHY_STATE_IRQ		UINT_MAX
+
 /**
  * enum phy_interface_t - Interface Mode definitions
  *
@@ -1261,7 +1266,15 @@ struct phy_driver {
 	 * dynamically adjust polling intervals based on link state or other
 	 * conditions.
 	 *
-	 * Returns the time in jiffies until the next update event.
+	 * Returning PHY_STATE_IRQ disables polling for link state changes,
+	 * indicating that the driver relies solely on IRQs for this purpose.
+	 * Note that polling may continue at a default rate if needed for
+	 * other callbacks like update_stats(). Requesting IRQ mode while
+	 * implementing update_stats() is considered a driver bug and will
+	 * trigger a kernel warning.
+	 *
+	 * Returns the time in jiffies until the next update event, or
+	 * PHY_STATE_IRQ to disable polling.
 	 */
 	unsigned int (*get_next_update_time)(struct phy_device *dev);
 };
-- 
2.39.5

[PATCH net v4 3/3] net: phy: smsc: recover missed link-up IRQs on LAN8700 with adaptive polling

[Oleksij Rempel]

Fix unreliable link detection on the LAN8700 PHY (integrated in LAN9512
and related USB adapters) when configured for 10 Mbit/s half- or
full-duplex with autonegotiation disabled, and connected to a link
partner that still advertises autonegotiation.

In this scenario, the PHY may emit several link-down interrupts during
negotiation but fail to raise a final link-up interrupt. As a result,
phylib never observes the transition and the kernel keeps the network
interface down, even though the link is actually up.

To handle this, add a get_next_update_time() callback that performs 1 Hz
polling for up to 30 seconds after the last interrupt, but only while
the PHY is in this problematic configuration and the link is still down.
This ensures link-up detection without unnecessary long delays or
full-time polling.

After 30 seconds with no further interrupt, the driver switches back to
IRQ-only mode. In all other configurations, IRQ-only mode is used
immediately.

This patch depends on:
- commit 8bf47e4d7b87 ("net: phy: Add support for driver-specific next
  update time")
- a prior patch in this series:
  net: phy: enable polling when driver implements get_next_update_time
  net: phy: allow drivers to disable polling via get_next_update_time()

Fixes: 1ce8b37241ed ("usbnet: smsc95xx: Forward PHY interrupts to PHY driver to avoid polling")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Lukas Wunner <lukas@wunner.de>
---
changes v2:
- Switch to hybrid approach: 1 Hz polling for 30 seconds after last IRQ
  instead of relaxed 30s polling while link is up
- Only enable polling in problematic 10M autoneg-off mode while link is down
- Return PHY_STATE_IRQ in all other configurations
- Updated commit message and comments to reflect new logic
---
 drivers/net/phy/smsc.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index b6489da5cfcd..88eb15700dbd 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -39,6 +39,9 @@
 /* interval between phylib state machine runs in ms */
 #define PHY_STATE_MACH_MS		1000
 
+/* max retry window for missed link-up */
+#define SMSC_IRQ_MAX_POLLING_TIME	secs_to_jiffies(30)
+
 struct smsc_hw_stat {
 	const char *string;
 	u8 reg;
@@ -54,6 +57,7 @@ struct smsc_phy_priv {
 	unsigned int edpd_mode_set_by_user:1;
 	unsigned int edpd_max_wait_ms;
 	bool wol_arp;
+	unsigned long last_irq;
 };
 
 static int smsc_phy_ack_interrupt(struct phy_device *phydev)
@@ -100,6 +104,7 @@ static int smsc_phy_config_edpd(struct phy_device *phydev)
 
 irqreturn_t smsc_phy_handle_interrupt(struct phy_device *phydev)
 {
+	struct smsc_phy_priv *priv = phydev->priv;
 	int irq_status;
 
 	irq_status = phy_read(phydev, MII_LAN83C185_ISF);
@@ -113,6 +118,8 @@ irqreturn_t smsc_phy_handle_interrupt(struct phy_device *phydev)
 	if (!(irq_status & MII_LAN83C185_ISF_INT_PHYLIB_EVENTS))
 		return IRQ_NONE;
 
+	WRITE_ONCE(priv->last_irq, jiffies);
+
 	phy_trigger_machine(phydev);
 
 	return IRQ_HANDLED;
@@ -684,6 +691,38 @@ int smsc_phy_probe(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(smsc_phy_probe);
 
+static unsigned int smsc_phy_get_next_update(struct phy_device *phydev)
+{
+	struct smsc_phy_priv *priv = phydev->priv;
+
+	/* If interrupts are disabled, fall back to default polling */
+	if (phydev->irq == PHY_POLL)
+		return PHY_STATE_TIME;
+
+	/*
+	 * LAN8700 may miss the final link-up IRQ when forced to 10 Mbps
+	 * (half/full duplex) and connected to an autonegotiating partner.
+	 *
+	 * To recover, poll at 1 Hz for up to 30 seconds after the last
+	 * interrupt - but only in this specific configuration and while
+	 * the link is still down.
+	 *
+	 * This keeps link-up latency low in common cases while reliably
+	 * detecting rare transitions. Outside of this mode, rely on IRQs.
+	 */
+	if (phydev->autoneg == AUTONEG_DISABLE && phydev->speed == SPEED_10 &&
+	    !phydev->link) {
+		unsigned long last_irq = READ_ONCE(priv->last_irq);
+
+		if (!time_is_before_jiffies(last_irq +
+					    SMSC_IRQ_MAX_POLLING_TIME))
+			return PHY_STATE_TIME;
+	}
+
+	/* switching to IRQ without polling */
+	return PHY_STATE_IRQ;
+}
+
 static struct phy_driver smsc_phy_driver[] = {
 {
 	.phy_id		= 0x0007c0a0, /* OUI=0x00800f, Model#=0x0a */
@@ -749,6 +788,7 @@ static struct phy_driver smsc_phy_driver[] = {
 	/* IRQ related */
 	.config_intr	= smsc_phy_config_intr,
 	.handle_interrupt = smsc_phy_handle_interrupt,
+	.get_next_update_time = smsc_phy_get_next_update,
 
 	/* Statistics */
 	.get_sset_count = smsc_get_sset_count,
-- 
2.39.5

Re: [PATCH net v4 0/3] net: phy: smsc: use IRQ + relaxed polling to fix missed link-up

[Jakub Kicinski]

On Mon, 14 Jul 2025 11:52:37 +0200 Oleksij Rempel wrote:
> This series makes the SMSC LAN8700 (as used in LAN9512 and similar USB
> adapters) reliable again in configurations where it is forced to 10 Mb/s
> and the link partner still advertises autonegotiation.
> 
> In this scenario, the PHY may miss the final link-up interrupt, causing
> the network interface to remain down even though a valid link is
> present.

Could we get a PHY maintainer ack on these (especially patch 2)?

Re: [PATCH net v4 0/3] net: phy: smsc: use IRQ + relaxed polling to fix missed link-up

[Andrew Lunn]

On Mon, Jul 14, 2025 at 11:52:37AM +0200, Oleksij Rempel wrote:
> This series makes the SMSC LAN8700 (as used in LAN9512 and similar USB
> adapters) reliable again in configurations where it is forced to 10 Mb/s
> and the link partner still advertises autonegotiation.

I've seen a comment from another Maintainer that thinks this is rather
hackish. I tend to agree, you are adding complexity to the core to
handle one broken PHY, and a corner case in that PHY. It would be
better to hide as much of this in the PHY driver.

I'm wondering if there is a much simpler solution, which does not need
the core changing. Have the driver dynamically flip between interrupts
and polling, depending on the link mode.

Start up in the usual way. If the platform supports interrupts, let
the core get the interrupt, install the handler and use
interrupts. Otherwise do polling.

If .config_aneg() puts the PHY into the broken state, forced to 10
Mb/s, and interrupts are used, set phydev->irq = PHY_POLL, and call
phy_trigger_machine() to kick off polling.

If .config_aneg() is called to take it out of the broken state,
restore phydev->irq. An additional poll up to one second later should
not cause any issues.

I don't think this needs any core code changes.

Maybe there is an issue with phy_free_interrupt() being called while
irq has been set to polling? You might be able to use the
phy_driver.remove() to handle that?

	Andrew