* [PATCH net v3 1/3] net: phy: enable polling when driver implements get_next_update_time
2025-07-11 9:49 [PATCH net v3 0/3] net: phy: smsc: use IRQ + relaxed polling to fix missed link-up Oleksij Rempel
@ 2025-07-11 9:49 ` Oleksij Rempel
2025-07-11 9:49 ` [PATCH net v3 2/3] net: phy: allow drivers to disable polling via get_next_update_time() Oleksij Rempel
2025-07-11 9:49 ` [PATCH net v3 3/3] net: phy: smsc: recover missed link-up IRQs on LAN8700 with adaptive polling Oleksij Rempel
2 siblings, 0 replies; 6+ messages in thread
From: Oleksij Rempel @ 2025-07-11 9:49 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Oleksij Rempel, kernel, linux-kernel, Russell King, netdev,
Andre Edich, Lukas Wunner
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net v3 2/3] net: phy: allow drivers to disable polling via get_next_update_time()
2025-07-11 9:49 [PATCH net v3 0/3] net: phy: smsc: use IRQ + relaxed polling to fix missed link-up Oleksij Rempel
2025-07-11 9:49 ` [PATCH net v3 1/3] net: phy: enable polling when driver implements get_next_update_time Oleksij Rempel
@ 2025-07-11 9:49 ` Oleksij Rempel
2025-07-12 13:18 ` Simon Horman
2025-07-11 9:49 ` [PATCH net v3 3/3] net: phy: smsc: recover missed link-up IRQs on LAN8700 with adaptive polling Oleksij Rempel
2 siblings, 1 reply; 6+ messages in thread
From: Oleksij Rempel @ 2025-07-11 9:49 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Oleksij Rempel, kernel, linux-kernel, Russell King, netdev,
Andre Edich, Lukas Wunner
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 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..3314accedcfd 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("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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net v3 2/3] net: phy: allow drivers to disable polling via get_next_update_time()
2025-07-11 9:49 ` [PATCH net v3 2/3] net: phy: allow drivers to disable polling via get_next_update_time() Oleksij Rempel
@ 2025-07-12 13:18 ` Simon Horman
2025-07-14 9:53 ` Oleksij Rempel
0 siblings, 1 reply; 6+ messages in thread
From: Simon Horman @ 2025-07-12 13:18 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, kernel, linux-kernel, Russell King,
netdev, Andre Edich, Lukas Wunner
On Fri, Jul 11, 2025 at 11:49:08AM +0200, Oleksij Rempel wrote:
...
> @@ -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("phy: %s: driver requested IRQ mode but needs polling for stats\n",
> + phydev_name(phydev));
The first argument to WARN_ONCE() should be a condition, not a format string.
Flagged by GCC and Clang builds with KCFLAGS=-Wformat-security
> + phy_queue_state_machine(phydev, PHY_STATE_TIME);
> + }
> + } else {
> + phy_queue_state_machine(phydev, next_time);
> + }
> + }
>
> return state_work;
> }
...
--
pw-bot: changes-requested
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net v3 2/3] net: phy: allow drivers to disable polling via get_next_update_time()
2025-07-12 13:18 ` Simon Horman
@ 2025-07-14 9:53 ` Oleksij Rempel
0 siblings, 0 replies; 6+ messages in thread
From: Oleksij Rempel @ 2025-07-14 9:53 UTC (permalink / raw)
To: Simon Horman
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, kernel, linux-kernel, Russell King,
netdev, Andre Edich, Lukas Wunner
On Sat, Jul 12, 2025 at 02:18:48PM +0100, Simon Horman wrote:
> > + if (phydev->drv->update_stats) {
> > + WARN_ONCE("phy: %s: driver requested IRQ mode but needs polling for stats\n",
> > + phydev_name(phydev));
>
> The first argument to WARN_ONCE() should be a condition, not a format string.
>
> Flagged by GCC and Clang builds with KCFLAGS=-Wformat-security
Thank you! I added it to my build test setup.
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net v3 3/3] net: phy: smsc: recover missed link-up IRQs on LAN8700 with adaptive polling
2025-07-11 9:49 [PATCH net v3 0/3] net: phy: smsc: use IRQ + relaxed polling to fix missed link-up Oleksij Rempel
2025-07-11 9:49 ` [PATCH net v3 1/3] net: phy: enable polling when driver implements get_next_update_time Oleksij Rempel
2025-07-11 9:49 ` [PATCH net v3 2/3] net: phy: allow drivers to disable polling via get_next_update_time() Oleksij Rempel
@ 2025-07-11 9:49 ` Oleksij Rempel
2 siblings, 0 replies; 6+ messages in thread
From: Oleksij Rempel @ 2025-07-11 9:49 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Oleksij Rempel, Lukas Wunner, kernel, linux-kernel, Russell King,
netdev, Andre Edich
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
^ permalink raw reply related [flat|nested] 6+ messages in thread