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

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

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 |