Skip to contentSkip to conversation

net: add phylink managed WoL and convert stmmac

PATCH Superseded Russell King (Oracle) 6 participants 22 messages Oct 17, 2025 – Oct 23, 2025

Subsystems (6)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Russell King (Oracle)

author maintainer
#0 lore raw

cover net: add phylink managed WoL and convert stmmac

Hi,

This series is implementing the thoughts of Andrew, Florian and myself to improve the quality of Wake-on-Lan (WoL) implementations.

This changes nothing for MAC drivers that do not wish to participate in this, but if they do, then they gain the benefit of phylink configuring WoL at the point closest to the media as possible.

We first need to solve the problem that the multitude of PHY drivers report their device supports WoL, but are not capable of waking the system. Correcting this is fundamental to choosing where WoL should be enabled - a mis-reported WoL support can render WoL completely ineffective.

The only PHY drivers which uses the driver model's wakeup support is drivers/net/phy/broadcom.c, and until recently, realtek. This means we have the opportunity for PHY drivers to be _correctly_ converted to use this method of signalling wake-up capability only when they can actually wake the system, and thus providing a way for phylink to know whether to use PHY-based WoL at all.

However, a PHY driver not implementing that logic doesn't become a blocker to MACs wanting to convert. In full, the logic is:

- phylink supports a flag, wol_phy_legacy, which forces phylink to use the PHY-based WoL even if the MDIO device is not marked as wake-up capable.

- when wol_phy_legacy is not set, we check whether the PHY MDIO device is wake-up capable. If it is, we offer the WoL request to the PHY.

- if neither wol_phy_legacy is set, or the PHY is not wake-up capable, we do not offer the WoL request to the PHY.

In both cases, after setting any PHY based WoL, we remove the options that the PHY now reports are enabled from the options mask, and offer these (if any) to the MAC. The mac will get a "mac_set_wol()" method call when any settings change.

Phylink mainatains the WoL state for the MAC, so there's no need for a "mac_get_wol()" method. There may be the need to set the initial state but this is not supported at present.

I've also added support for doing the PHY speed-up/speed-down at suspend/resume time depending on the WoL state, which takes another issue from the MAC authors.

Lastly, with phylink now having the full picture for WoL, the "mac_wol" argument for phylink_suspend() becomes redundant, and for MAC drivers that implement mac_set_wol(), the value passed becomes irrelevant.

Changes since RFC:
- patch 3: fix location of phylink_xxx_supports_wol() helpers
- patch 3: handle sopass for WAKE_MAGICSECURE only when the MAC is handling magic secure.

 drivers/net/ethernet/stmicro/stmmac/stmmac.h       | 11 +--
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 31 +-------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 45 +++++++----
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  4 +-
 drivers/net/phy/phy_device.c                       | 14 +++-
 drivers/net/phy/phylink.c                          | 92 +++++++++++++++++++++-
 include/linux/phy.h                                | 21 +++++
 include/linux/phylink.h                            | 28 +++++++
 8 files changed, 182 insertions(+), 64 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#1 lore raw

PATCH 1/6 net: phy: add phy_can_wakeup()

Add phy_can_wakeup() to report whether the PHY driver has marked the PHY device as being wake-up capable as far as the driver model is concerned.

---
 include/linux/phy.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
include/linux/phy.h +12 -0
index 3c7634482356..3eeeaec52832 100644
@@ -1379,6 +1379,18 @@ static inline void phy_disable_eee_mode(struct phy_device *phydev, u32 link_mode
 	linkmode_clear_bit(link_mode, phydev->advertising_eee);
 }
 
+/**
+ * phy_can_wakeup() - indicate whether PHY has driver model wakeup capabilities
+ * @phydev: The phy_device struct
+ *
+ * Returns: true/false depending on the PHY driver's device_set_wakeup_capable()
+ * setting.
+ */
+static inline bool phy_can_wakeup(struct phy_device *phydev)
+{
+	return device_can_wakeup(&phydev->mdio.dev);
+}
+
 void phy_resolve_aneg_pause(struct phy_device *phydev);
 void phy_resolve_aneg_linkmode(struct phy_device *phydev);
 
Signature · 1 line
2.47.3
#4 lore raw

PATCH 2/6 net: phy: add phy_may_wakeup()

In reply to Russell King (Oracle)

Add phy_may_wakeup() which uses the driver model's device_may_wakeup() when the PHY driver has marked the device as wakeup capable in the driver model, otherwise use phy_drv_wol_enabled().

Replace the sites that used to call phy_drv_wol_enabled() with this as checking the driver model will be more efficient than checking the WoL state.

Export phy_may_wakeup() so that phylink can use it.

---
 drivers/net/phy/phy_device.c | 14 ++++++++++++--
 include/linux/phy.h          |  9 +++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)
drivers/net/phy/phy_device.c +12 -2
index 7a67c900e79a..b7feaf0cb1df 100644
@@ -251,6 +251,16 @@ static bool phy_drv_wol_enabled(struct phy_device *phydev)
 	return wol.wolopts != 0;
 }
 
+bool phy_may_wakeup(struct phy_device *phydev)
+{
+	/* If the PHY is using driver-model based wakeup, use that state. */
+	if (phy_can_wakeup(phydev))
+		return device_may_wakeup(&phydev->mdio.dev);
+
+	return phy_drv_wol_enabled(phydev);
+}
+EXPORT_SYMBOL_GPL(phy_may_wakeup);
+
 static void phy_link_change(struct phy_device *phydev, bool up)
 {
 	struct net_device *netdev = phydev->attached_dev;
@@ -302,7 +312,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 	/* If the PHY on the mido bus is not attached but has WOL enabled
 	 * we cannot suspend the PHY.
 	 */
-	if (!netdev && phy_drv_wol_enabled(phydev))
+	if (!netdev && phy_may_wakeup(phydev))
 		return false;
 
 	/* PHY not attached? May suspend if the PHY has not already been
@@ -1909,7 +1919,7 @@ int phy_suspend(struct phy_device *phydev)
 	if (phydev->suspended || !phydrv)
 		return 0;
 
-	phydev->wol_enabled = phy_drv_wol_enabled(phydev) ||
+	phydev->wol_enabled = phy_may_wakeup(phydev) ||
 			      (netdev && netdev->ethtool->wol_enabled);
 	/* If the device has WOL enabled, we cannot suspend the PHY */
 	if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
include/linux/phy.h +9 -0
index 3eeeaec52832..801356da1fb2 100644
@@ -1391,6 +1391,15 @@ static inline bool phy_can_wakeup(struct phy_device *phydev)
 	return device_can_wakeup(&phydev->mdio.dev);
 }
 
+/**
+ * phy_may_wakeup() - indicate whether PHY has driver model wakeup is enabled
+ * @phydev: The phy_device struct
+ *
+ * Returns: true/false depending on the PHY driver's device_set_wakeup_enabled()
+ * setting.
+ */
+bool phy_may_wakeup(struct phy_device *phydev);
+
 void phy_resolve_aneg_pause(struct phy_device *phydev);
 void phy_resolve_aneg_linkmode(struct phy_device *phydev);
 
Signature · 1 line
2.47.3
#11 lore raw

PATCH 3/6 net: phylink: add phylink managed MAC Wake-on-Lan support

In reply to Russell King (Oracle)

Add core phylink managed Wake-on-Lan support, which is enabled when the MAC driver fills in the new .mac_wol_set() method that this commit creates.

When this feature is disabled, phylink acts as it has in the past, merely passing the ethtool WoL calls to phylib whenever a PHY exists. No other new functionality provided by this commit is enabled.

When this feature is enabled, a more inteligent approach is used. Phylink will first pass WoL options to the PHY, read them back, and attempt to set any options that were not set at the PHY at the MAC.

Since we have PHY drivers that report they support WoL, and accept WoL configuration even though they aren't wired up to be capable of waking the system, we need a way to differentiate between PHYs that think they support WoL and those which actually do. As PHY drivers do not make use of the driver model's wake-up infrastructure, but could, we use this to determine whether PHY drivers can participate. This gives a path forward where, as MAC drivers are converted to this, it encourages PHY drivers to also be converted.

Phylink will also ignore the mac_wol argument to phylink_suspend() as it now knows the WoL state at the MAC.

MAC drivers are expected to record/configure the Wake-on-Lan state in their .mac_set_wol() method, and deal appropriately with it in their suspend/resume methods. The driver model provides assistance to set the IRQ wake support which may assist driver authors in achieving the necessary configuration.

---
 drivers/net/phy/phylink.c | 80 +++++++++++++++++++++++++++++++++++++--
 include/linux/phylink.h   | 26 +++++++++++++
 2 files changed, 102 insertions(+), 4 deletions(-)
drivers/net/phy/phylink.c +76 -4
index 9d7799ea1c17..939438a6d6f5 100644
@@ -93,6 +93,9 @@ struct phylink {
 	u8 sfp_port;
 
 	struct eee_config eee_cfg;
+
+	u32 wolopts_mac;
+	u8 wol_sopass[SOPASS_MAX];
 };
 
 #define phylink_printk(level, pl, fmt, ...) \
@@ -2562,6 +2565,17 @@ void phylink_rx_clk_stop_unblock(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_rx_clk_stop_unblock);
 
+static bool phylink_mac_supports_wol(struct phylink *pl)
+{
+	return !!pl->mac_ops->mac_wol_set;
+}
+
+static bool phylink_phy_supports_wol(struct phylink *pl,
+				     struct phy_device *phydev)
+{
+	return phydev && (pl->config->wol_phy_legacy || phy_can_wakeup(phydev));
+}
+
 /**
  * phylink_suspend() - handle a network device suspend event
  * @pl: a pointer to a &struct phylink returned from phylink_create()
@@ -2575,11 +2589,17 @@ EXPORT_SYMBOL_GPL(phylink_rx_clk_stop_unblock);
  *   can also bring down the link between the MAC and PHY.
  * - If Wake-on-Lan is active, but being handled by the MAC, the MAC
  *   still needs to receive packets, so we can not bring the link down.
+ *
+ * Note: when phylink managed Wake-on-Lan is in use, @mac_wol is ignored.
+ * (struct phylink_mac_ops.mac_set_wol populated.)
  */
 void phylink_suspend(struct phylink *pl, bool mac_wol)
 {
 	ASSERT_RTNL();
 
+	if (phylink_mac_supports_wol(pl))
+		mac_wol = !!pl->wolopts_mac;
+
 	if (mac_wol && (!pl->netdev || pl->netdev->ethtool->wol_enabled)) {
 		/* Wake-on-Lan enabled, MAC handling */
 		mutex_lock(&pl->state_mutex);
@@ -2689,8 +2709,24 @@ void phylink_ethtool_get_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 	wol->supported = 0;
 	wol->wolopts = 0;
 
-	if (pl->phydev)
-		phy_ethtool_get_wol(pl->phydev, wol);
+	if (phylink_mac_supports_wol(pl)) {
+		if (phylink_phy_supports_wol(pl, pl->phydev))
+			phy_ethtool_get_wol(pl->phydev, wol);
+
+		/* Where the MAC augments the WoL support, merge its support and
+		 * current configuration.
+		 */
+		if (~wol->wolopts & pl->wolopts_mac & WAKE_MAGICSECURE)
+			memcpy(wol->sopass, pl->wol_sopass,
+			       sizeof(wol->sopass));
+
+		wol->supported |= pl->config->wol_mac_support;
+		wol->wolopts |= pl->wolopts_mac;
+	} else {
+		/* Legacy */
+		if (pl->phydev)
+			phy_ethtool_get_wol(pl->phydev, wol);
+	}
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_wol);
 
@@ -2707,12 +2743,48 @@ EXPORT_SYMBOL_GPL(phylink_ethtool_get_wol);
  */
 int phylink_ethtool_set_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 {
+	struct ethtool_wolinfo w;
 	int ret = -EOPNOTSUPP;
+	bool changed;
+	u32 wolopts;
 
 	ASSERT_RTNL();
 
-	if (pl->phydev)
-		ret = phy_ethtool_set_wol(pl->phydev, wol);
+	if (phylink_mac_supports_wol(pl)) {
+		wolopts = wol->wolopts;
+
+		if (phylink_phy_supports_wol(pl, pl->phydev)) {
+			ret = phy_ethtool_set_wol(pl->phydev, wol);
+			if (ret != 0 && ret != -EOPNOTSUPP)
+				return ret;
+
+			phy_ethtool_get_wol(pl->phydev, &w);
+
+			/* Any Wake-on-Lan modes which the PHY is handling
+			 * should not be passed on to the MAC.
+			 */
+			wolopts &= ~w.wolopts;
+		}
+
+		wolopts &= pl->config->wol_mac_support;
+		changed = pl->wolopts_mac != wolopts;
+		if (wolopts & WAKE_MAGICSECURE)
+			changed |= !!memcmp(wol->sopass, pl->wol_sopass,
+					    sizeof(wol->sopass));
+		memcpy(pl->wol_sopass, wol->sopass, sizeof(pl->wol_sopass));
+
+		if (changed) {
+			ret = pl->mac_ops->mac_wol_set(pl->config, wolopts,
+						       wol->sopass);
+			if (!ret)
+				pl->wolopts_mac = wolopts;
+		} else {
+			ret = 0;
+		}
+	} else {
+		if (pl->phydev)
+			ret = phy_ethtool_set_wol(pl->phydev, wol);
+	}
 
 	return ret;
 }
include/linux/phylink.h +26 -0
index 9af0411761d7..59cb58b29d1d 100644
@@ -156,6 +156,8 @@ enum phylink_op_type {
  * @lpi_capabilities: MAC speeds which can support LPI signalling
  * @lpi_timer_default: Default EEE LPI timer setting.
  * @eee_enabled_default: If set, EEE will be enabled by phylink at creation time
+ * @wol_phy_legacy: Use Wake-on-Lan with PHY even if phy_can_wakeup() is false
+ * @wol_mac_support: Bitmask of MAC supported %WAKE_* options
  */
 struct phylink_config {
 	struct device *dev;
@@ -173,6 +175,10 @@ struct phylink_config {
 	unsigned long lpi_capabilities;
 	u32 lpi_timer_default;
 	bool eee_enabled_default;
+
+	/* Wake-on-Lan support */
+	bool wol_phy_legacy;
+	u32 wol_mac_support;
 };
 
 void phylink_limit_mac_speed(struct phylink_config *config, u32 max_speed);
@@ -188,6 +194,7 @@ void phylink_limit_mac_speed(struct phylink_config *config, u32 max_speed);
  * @mac_link_up: allow the link to come up.
  * @mac_disable_tx_lpi: disable LPI.
  * @mac_enable_tx_lpi: enable and configure LPI.
+ * @mac_wol_set: configure Wake-on-Lan settings at the MAC.
  *
  * The individual methods are described more fully below.
  */
@@ -211,6 +218,9 @@ struct phylink_mac_ops {
 	void (*mac_disable_tx_lpi)(struct phylink_config *config);
 	int (*mac_enable_tx_lpi)(struct phylink_config *config, u32 timer,
 				 bool tx_clk_stop);
+
+	int (*mac_wol_set)(struct phylink_config *config, u32 wolopts,
+			   const u8 *sopass);
 };
 
 #if 0 /* For kernel-doc purposes only. */
@@ -440,6 +450,22 @@ void mac_disable_tx_lpi(struct phylink_config *config);
  */
 int mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
 		      bool tx_clk_stop);
+
+/**
+ * mac_wol_set() - configure the Wake-on-Lan parameters
+ * @config: a pointer to a &struct phylink_config.
+ * @wolopts: Bitmask of %WAKE_* flags for enabled Wake-On-Lan modes.
+ * @sopass: SecureOn(tm) password; meaningful only for %WAKE_MAGICSECURE
+ *
+ * Enable the specified Wake-on-Lan options at the MAC. Options that the
+ * PHY can handle will have been removed from @wolopts.
+ *
+ * The presence of this method enables phylink-managed WoL support.
+ *
+ * Returns: 0 on success.
+ */
+int (*mac_wol_set)(struct phylink_config *config, u32 wolopts,
+		   const u8 *sopass);
 #endif
 
 struct phylink_pcs_ops;
Signature · 1 line
2.47.3
#15 lore raw

PATCH 4/6 net: phylink: add phylink managed wake-on-lan PHY speed control

In reply to Russell King (Oracle)

Some drivers, e.g. stmmac, use the speed_up()/speed_down() APIs to gain additional power saving during Wake-on-LAN where the PHY is managing the state.

Add support to phylink for this, which can be enabled by the MAC driver. Only change the PHY speed if the PHY is configured for wake-up, but without any wake-up on the MAC side, as MAC side means changing the configuration once the negotiation has completed.

---
 drivers/net/phy/phylink.c | 12 ++++++++++++
 include/linux/phylink.h   |  2 ++
 2 files changed, 14 insertions(+)
drivers/net/phy/phylink.c +12 -0
index 939438a6d6f5..26bd4b7619dd 100644
@@ -2576,6 +2576,12 @@ static bool phylink_phy_supports_wol(struct phylink *pl,
 	return phydev && (pl->config->wol_phy_legacy || phy_can_wakeup(phydev));
 }
 
+static bool phylink_phy_pm_speed_ctrl(struct phylink *pl)
+{
+	return pl->config->wol_phy_speed_ctrl && !pl->wolopts_mac &&
+	       pl->phydev && phy_may_wakeup(pl->phydev);
+}
+
 /**
  * phylink_suspend() - handle a network device suspend event
  * @pl: a pointer to a &struct phylink returned from phylink_create()
@@ -2625,6 +2631,9 @@ void phylink_suspend(struct phylink *pl, bool mac_wol)
 	} else {
 		phylink_stop(pl);
 	}
+
+	if (phylink_phy_pm_speed_ctrl(pl))
+		phy_speed_down(pl->phydev, false);
 }
 EXPORT_SYMBOL_GPL(phylink_suspend);
 
@@ -2664,6 +2673,9 @@ void phylink_resume(struct phylink *pl)
 {
 	ASSERT_RTNL();
 
+	if (phylink_phy_pm_speed_ctrl(pl))
+		phy_speed_up(pl->phydev);
+
 	if (test_bit(PHYLINK_DISABLE_MAC_WOL, &pl->phylink_disable_state)) {
 		/* Wake-on-Lan enabled, MAC handling */
 
include/linux/phylink.h +2 -0
index 59cb58b29d1d..38363e566ac3 100644
@@ -157,6 +157,7 @@ enum phylink_op_type {
  * @lpi_timer_default: Default EEE LPI timer setting.
  * @eee_enabled_default: If set, EEE will be enabled by phylink at creation time
  * @wol_phy_legacy: Use Wake-on-Lan with PHY even if phy_can_wakeup() is false
+ * @wol_phy_speed_ctrl: Use phy speed control on suspend/resume
  * @wol_mac_support: Bitmask of MAC supported %WAKE_* options
  */
 struct phylink_config {
@@ -178,6 +179,7 @@ struct phylink_config {
 
 	/* Wake-on-Lan support */
 	bool wol_phy_legacy;
+	bool wol_phy_speed_ctrl;
 	u32 wol_mac_support;
 };
 
Signature · 1 line
2.47.3
#18 lore raw

PATCH 5/6 net: stmmac: convert to phylink-managed Wake-on-Lan

In reply to Russell King (Oracle)

Convert stmmac to use phylink-managed Wake-on-Lan support. To achieve this, we implement the .mac_wol_set() method, which simply configures the driver model's struct device wakeup for stmmac, and sets the priv->wolopts appropriately.

When STMMAC_FLAG_USE_PHY_WOL is set, in the stmmac world this means to only use the PHY's WoL support and ignore the MAC's WoL capabilities. To preserve this behaviour, we enable phylink's legacy mode, and avoid telling phylink that the MAC has any WoL support. This achieves the same functionality for this case.

When STMMAC_FLAG_USE_PHY_WOL is not set, we provide the MAC's WoL capabilities to phylink, which then allows phylink to choose between the PHY and MAC for WoL depending on their individual capabilities as described in the phylink commit. This only augments the WoL functionality with PHYs that declare to the driver model that they are wake-up capable. Currently, very few PHY drivers support this.

---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  6 +---
 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  | 34 ++++---------------
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 30 ++++++++++++++--
 .../ethernet/stmicro/stmmac/stmmac_platform.c |  4 +--
 4 files changed, 36 insertions(+), 38 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac.h +1 -5
index ec8bddc1c37f..f5628a6af7d3 100644
@@ -287,6 +287,7 @@ struct stmmac_priv {
 	int hw_cap_support;
 	int synopsys_id;
 	u32 msg_enable;
+	/* Our MAC Wake-on-Lan options */
 	int wolopts;
 	int wol_irq;
 	u32 gmii_address_bus_config;
@@ -375,11 +376,6 @@ enum stmmac_state {
 
 extern const struct dev_pm_ops stmmac_simple_pm_ops;
 
-static inline bool stmmac_wol_enabled_mac(struct stmmac_priv *priv)
-{
-	return priv->plat->pmt && device_may_wakeup(priv->device);
-}
-
 static inline bool stmmac_wol_enabled_phy(struct stmmac_priv *priv)
 {
 	return !priv->plat->pmt && device_may_wakeup(priv->device);
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +6 -28
index 39fa1ec92f82..fd29b551a082 100644
@@ -787,41 +787,19 @@ static void stmmac_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
 
-	if (!priv->plat->pmt)
-		return phylink_ethtool_get_wol(priv->phylink, wol);
-
-	mutex_lock(&priv->lock);
-	if (device_can_wakeup(priv->device)) {
-		wol->supported = WAKE_MAGIC | WAKE_UCAST;
-		if (priv->hw_cap_support && !priv->dma_cap.pmt_magic_frame)
-			wol->supported &= ~WAKE_MAGIC;
-		wol->wolopts = priv->wolopts;
-	}
-	mutex_unlock(&priv->lock);
+	return phylink_ethtool_get_wol(priv->phylink, wol);
 }
 
 static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
+	int ret;
 
-	if (!device_can_wakeup(priv->device))
-		return -EOPNOTSUPP;
-
-	if (!priv->plat->pmt) {
-		int ret = phylink_ethtool_set_wol(priv->phylink, wol);
-
-		if (!ret)
-			device_set_wakeup_enable(priv->device, !!wol->wolopts);
-		return ret;
-	}
-
-	device_set_wakeup_enable(priv->device, !!wol->wolopts);
+	ret = phylink_ethtool_set_wol(priv->phylink, wol);
+	if (!ret)
+		device_set_wakeup_enable(priv->device, !!wol->wolopts);
 
-	mutex_lock(&priv->lock);
-	priv->wolopts = wol->wolopts;
-	mutex_unlock(&priv->lock);
-
-	return 0;
+	return ret;
 }
 
 static int stmmac_ethtool_op_get_eee(struct net_device *dev,
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +27 -3
index c9fa965c8566..a959a4949282 100644
@@ -1065,6 +1065,20 @@ static int stmmac_mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
 	return 0;
 }
 
+static int stmmac_mac_wol_set(struct phylink_config *config, u32 wolopts,
+			      const u8 *sopass)
+{
+	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
+
+	device_set_wakeup_enable(priv->device, !!wolopts);
+
+	mutex_lock(&priv->lock);
+	priv->wolopts = wolopts;
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
 static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
 	.mac_get_caps = stmmac_mac_get_caps,
 	.mac_select_pcs = stmmac_mac_select_pcs,
@@ -1074,6 +1088,7 @@ static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
 	.mac_link_up = stmmac_mac_link_up,
 	.mac_disable_tx_lpi = stmmac_mac_disable_tx_lpi,
 	.mac_enable_tx_lpi = stmmac_mac_enable_tx_lpi,
+	.mac_wol_set = stmmac_mac_wol_set,
 };
 
 /**
@@ -1250,6 +1265,15 @@ static int stmmac_phylink_setup(struct stmmac_priv *priv)
 		config->eee_enabled_default = true;
 	}
 
+	if (priv->plat->flags & STMMAC_FLAG_USE_PHY_WOL) {
+		config->wol_phy_legacy = true;
+	} else {
+		if (priv->dma_cap.pmt_remote_wake_up)
+			config->wol_mac_support |= WAKE_UCAST;
+		if (priv->dma_cap.pmt_magic_frame)
+			config->wol_mac_support |= WAKE_MAGIC;
+	}
+
 	fwnode = priv->plat->port_node;
 	if (!fwnode)
 		fwnode = dev_fwnode(priv->device);
@@ -7760,7 +7784,7 @@ int stmmac_suspend(struct device *dev)
 		priv->plat->serdes_powerdown(ndev, priv->plat->bsp_priv);
 
 	/* Enable Power down mode by programming the PMT regs */
-	if (stmmac_wol_enabled_mac(priv)) {
+	if (priv->wolopts) {
 		stmmac_pmt(priv, priv->hw, priv->wolopts);
 		priv->irq_wake = 1;
 	} else {
@@ -7774,7 +7798,7 @@ int stmmac_suspend(struct device *dev)
 	if (stmmac_wol_enabled_phy(priv))
 		phylink_speed_down(priv->phylink, false);
 
-	phylink_suspend(priv->phylink, stmmac_wol_enabled_mac(priv));
+	phylink_suspend(priv->phylink, !!priv->wolopts);
 	rtnl_unlock();
 
 	if (stmmac_fpe_supported(priv))
@@ -7850,7 +7874,7 @@ int stmmac_resume(struct device *dev)
 	 * this bit because it can generate problems while resuming
 	 * from another devices (e.g. serial console).
 	 */
-	if (stmmac_wol_enabled_mac(priv)) {
+	if (priv->wolopts) {
 		mutex_lock(&priv->lock);
 		stmmac_pmt(priv, priv->hw, 0);
 		mutex_unlock(&priv->lock);
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +2 -2
index 27bcaae07a7f..ed38925eb6f6 100644
@@ -970,7 +970,7 @@ static int __maybe_unused stmmac_pltfr_noirq_suspend(struct device *dev)
 	if (!netif_running(ndev))
 		return 0;
 
-	if (!stmmac_wol_enabled_mac(priv)) {
+	if (!priv->wolopts) {
 		/* Disable clock in case of PWM is off */
 		clk_disable_unprepare(priv->plat->clk_ptp_ref);
 
@@ -991,7 +991,7 @@ static int __maybe_unused stmmac_pltfr_noirq_resume(struct device *dev)
 	if (!netif_running(ndev))
 		return 0;
 
-	if (!stmmac_wol_enabled_mac(priv)) {
+	if (!priv->wolopts) {
 		/* enable the clk previously disabled */
 		ret = pm_runtime_force_resume(dev);
 		if (ret)
Signature · 1 line
2.47.3
#19 lore raw

PATCH 6/6 net: stmmac: convert to phylink managed WoL PHY speed

In reply to Russell King (Oracle)

Convert stmmac to use phylink's management of the PHY speed when Wake-on-Lan is enabled.

---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |  5 -----
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c  |  7 +------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 15 +--------------
 3 files changed, 2 insertions(+), 25 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac.h +0 -5
index f5628a6af7d3..aa8fa180c237 100644
@@ -376,11 +376,6 @@ enum stmmac_state {
 
 extern const struct dev_pm_ops stmmac_simple_pm_ops;
 
-static inline bool stmmac_wol_enabled_phy(struct stmmac_priv *priv)
-{
-	return !priv->plat->pmt && device_may_wakeup(priv->device);
-}
-
 int stmmac_mdio_unregister(struct net_device *ndev);
 int stmmac_mdio_register(struct net_device *ndev);
 int stmmac_mdio_reset(struct mii_bus *mii);
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +1 -6
index fd29b551a082..7ab736de308f 100644
@@ -793,13 +793,8 @@ static void stmmac_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
-	int ret;
 
-	ret = phylink_ethtool_set_wol(priv->phylink, wol);
-	if (!ret)
-		device_set_wakeup_enable(priv->device, !!wol->wolopts);
-
-	return ret;
+	return phylink_ethtool_set_wol(priv->phylink, wol);
 }
 
 static int stmmac_ethtool_op_get_eee(struct net_device *dev,
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1 -14
index a959a4949282..6d0b52d30ac8 100644
@@ -1189,14 +1189,6 @@ static int stmmac_init_phy(struct net_device *dev)
 		phylink_ethtool_set_eee(priv->phylink, &eee);
 	}
 
-	if (!priv->plat->pmt) {
-		struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
-
-		phylink_ethtool_get_wol(priv->phylink, &wol);
-		device_set_wakeup_capable(priv->device, !!wol.supported);
-		device_set_wakeup_enable(priv->device, !!wol.wolopts);
-	}
-
 	return 0;
 }
 
@@ -1265,6 +1257,7 @@ static int stmmac_phylink_setup(struct stmmac_priv *priv)
 		config->eee_enabled_default = true;
 	}
 
+	config->wol_phy_speed_ctrl = true;
 	if (priv->plat->flags & STMMAC_FLAG_USE_PHY_WOL) {
 		config->wol_phy_legacy = true;
 	} else {
@@ -7795,9 +7788,6 @@ int stmmac_suspend(struct device *dev)
 	mutex_unlock(&priv->lock);
 
 	rtnl_lock();
-	if (stmmac_wol_enabled_phy(priv))
-		phylink_speed_down(priv->phylink, false);
-
 	phylink_suspend(priv->phylink, !!priv->wolopts);
 	rtnl_unlock();
 
@@ -7936,9 +7926,6 @@ int stmmac_resume(struct device *dev)
 	 * workqueue thread, which will race with initialisation.
 	 */
 	phylink_resume(priv->phylink);
-	if (stmmac_wol_enabled_phy(priv))
-		phylink_speed_up(priv->phylink);
-
 	rtnl_unlock();
 
 	netif_device_attach(ndev);
Signature · 1 line
2.47.3
#20 lore raw
In reply to Russell King (Oracle)
Quoted 44 lines · Russell King (Oracle)

Hi,

This series is implementing the thoughts of Andrew, Florian and myself to improve the quality of Wake-on-Lan (WoL) implementations.

This changes nothing for MAC drivers that do not wish to participate in this, but if they do, then they gain the benefit of phylink configuring WoL at the point closest to the media as possible.

We first need to solve the problem that the multitude of PHY drivers report their device supports WoL, but are not capable of waking the system. Correcting this is fundamental to choosing where WoL should be enabled - a mis-reported WoL support can render WoL completely ineffective.

The only PHY drivers which uses the driver model's wakeup support is drivers/net/phy/broadcom.c, and until recently, realtek. This means we have the opportunity for PHY drivers to be _correctly_ converted to use this method of signalling wake-up capability only when they can actually wake the system, and thus providing a way for phylink to know whether to use PHY-based WoL at all.

However, a PHY driver not implementing that logic doesn't become a blocker to MACs wanting to convert. In full, the logic is:

- phylink supports a flag, wol_phy_legacy, which forces phylink to use the PHY-based WoL even if the MDIO device is not marked as wake-up capable.

- when wol_phy_legacy is not set, we check whether the PHY MDIO device is wake-up capable. If it is, we offer the WoL request to the PHY.

- if neither wol_phy_legacy is set, or the PHY is not wake-up capable, we do not offer the WoL request to the PHY.

In both cases, after setting any PHY based WoL, we remove the options that the PHY now reports are enabled from the options mask, and offer these (if any) to the MAC. The mac will get a "mac_set_wol()" method call when any settings change.

Phylink mainatains the WoL state for the MAC, so there's no need for a "mac_get_wol()" method. There may be the need to set the initial state but this is not supported at present.

Quoted 9 lines

I've also added support for doing the PHY speed-up/speed-down at suspend/resume time depending on the WoL state, which takes another issue from the MAC authors.

Lastly, with phylink now having the full picture for WoL, the "mac_wol" argument for phylink_suspend() becomes redundant, and for MAC drivers that implement mac_set_wol(), the value passed becomes irrelevant.

Hello Russell,

Currently, I don't have the bandwidth to work on that subject. It should be better in November.

I manipulated a bit this patchset when it was in the RFC state and I remember having issues when suspending the platform with the PHY in interrupt mode.
There was a PHY state change that generated an interrupt while the suspend sequence was started leading to an immediate wakeup. This state change occurs before the PHY driver ops are called. Therefore, I had no chance of masking the non-WOL interrupts. I'm sorry for being quite vague but I don't have the setup anymore.

Gatien

Quoted 15 lines

Changes since RFC:
- patch 3: fix location of phylink_xxx_supports_wol() helpers
- patch 3: handle sopass for WAKE_MAGICSECURE only when the MAC is handling magic secure.

  drivers/net/ethernet/stmicro/stmmac/stmmac.h       | 11 +--
  .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 31 +-------
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 45 +++++++----
  .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  4 +-
  drivers/net/phy/phy_device.c                       | 14 +++-
  drivers/net/phy/phylink.c                          | 92 +++++++++++++++++++++-
  include/linux/phy.h                                | 21 +++++
  include/linux/phylink.h                            | 28 +++++++
  8 files changed, 182 insertions(+), 64 deletions(-)
author maintainer
#21 lore raw
In reply to Russell King (Oracle)

Hi,

This series is now marked in PW as "Needs ACK"... please can someone review this.

Thanks.

Quoted 72 lines · Russell King (Oracle)

Hi,

This series is implementing the thoughts of Andrew, Florian and myself to improve the quality of Wake-on-Lan (WoL) implementations.

This changes nothing for MAC drivers that do not wish to participate in this, but if they do, then they gain the benefit of phylink configuring WoL at the point closest to the media as possible.

We first need to solve the problem that the multitude of PHY drivers report their device supports WoL, but are not capable of waking the system. Correcting this is fundamental to choosing where WoL should be enabled - a mis-reported WoL support can render WoL completely ineffective.

The only PHY drivers which uses the driver model's wakeup support is drivers/net/phy/broadcom.c, and until recently, realtek. This means we have the opportunity for PHY drivers to be _correctly_ converted to use this method of signalling wake-up capability only when they can actually wake the system, and thus providing a way for phylink to know whether to use PHY-based WoL at all.

However, a PHY driver not implementing that logic doesn't become a blocker to MACs wanting to convert. In full, the logic is:

- phylink supports a flag, wol_phy_legacy, which forces phylink to use the PHY-based WoL even if the MDIO device is not marked as wake-up capable.

- when wol_phy_legacy is not set, we check whether the PHY MDIO device is wake-up capable. If it is, we offer the WoL request to the PHY.

- if neither wol_phy_legacy is set, or the PHY is not wake-up capable, we do not offer the WoL request to the PHY.

In both cases, after setting any PHY based WoL, we remove the options that the PHY now reports are enabled from the options mask, and offer these (if any) to the MAC. The mac will get a "mac_set_wol()" method call when any settings change.

Phylink mainatains the WoL state for the MAC, so there's no need for a "mac_get_wol()" method. There may be the need to set the initial state but this is not supported at present.

I've also added support for doing the PHY speed-up/speed-down at suspend/resume time depending on the WoL state, which takes another issue from the MAC authors.

Lastly, with phylink now having the full picture for WoL, the "mac_wol" argument for phylink_suspend() becomes redundant, and for MAC drivers that implement mac_set_wol(), the value passed becomes irrelevant.

Changes since RFC:
- patch 3: fix location of phylink_xxx_supports_wol() helpers
- patch 3: handle sopass for WAKE_MAGICSECURE only when the MAC is handling magic secure.

 drivers/net/ethernet/stmicro/stmmac/stmmac.h       | 11 +--
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 31 +-------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 45 +++++++----
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  4 +-
 drivers/net/phy/phy_device.c                       | 14 +++-
 drivers/net/phy/phylink.c                          | 92 +++++++++++++++++++++-
 include/linux/phy.h                                | 21 +++++
 include/linux/phylink.h                            | 28 +++++++
 8 files changed, 182 insertions(+), 64 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
maintainer Reviewed-by on v1 1/6
#2 lore raw

Re: [PATCH net-next 1/6] net: phy: add phy_can_wakeup()

In reply to Russell King (Oracle)
Quoted 5 lines · Russell King (Oracle)

Add phy_can_wakeup() to report whether the PHY driver has marked the PHY device as being wake-up capable as far as the driver model is concerned.

Maxime

Quoted 27 lines
---
 include/linux/phy.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
include/linux/phy.h +12 -0
index 3c7634482356..3eeeaec52832 100644
@@ -1379,6 +1379,18 @@ static inline void phy_disable_eee_mode(struct phy_device *phydev, u32 link_mode
 	linkmode_clear_bit(link_mode, phydev->advertising_eee);
 }
 
+/**
+ * phy_can_wakeup() - indicate whether PHY has driver model wakeup capabilities
+ * @phydev: The phy_device struct
+ *
+ * Returns: true/false depending on the PHY driver's device_set_wakeup_capable()
+ * setting.
+ */
+static inline bool phy_can_wakeup(struct phy_device *phydev)
+{
+	return device_can_wakeup(&phydev->mdio.dev);
+}
+
 void phy_resolve_aneg_pause(struct phy_device *phydev);
 void phy_resolve_aneg_linkmode(struct phy_device *phydev);
 
maintainer Reviewed-by on v1 2/6
#5 lore raw

Re: [PATCH net-next 2/6] net: phy: add phy_may_wakeup()

In reply to Russell King (Oracle)

Hi Russell,

Quoted 60 lines · Russell King (Oracle)

Add phy_may_wakeup() which uses the driver model's device_may_wakeup() when the PHY driver has marked the device as wakeup capable in the driver model, otherwise use phy_drv_wol_enabled().

Replace the sites that used to call phy_drv_wol_enabled() with this as checking the driver model will be more efficient than checking the WoL state.

Export phy_may_wakeup() so that phylink can use it.

---
 drivers/net/phy/phy_device.c | 14 ++++++++++++--
 include/linux/phy.h          |  9 +++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)
drivers/net/phy/phy_device.c +12 -2
index 7a67c900e79a..b7feaf0cb1df 100644
@@ -251,6 +251,16 @@ static bool phy_drv_wol_enabled(struct phy_device *phydev)
 	return wol.wolopts != 0;
 }
 
+bool phy_may_wakeup(struct phy_device *phydev)
+{
+	/* If the PHY is using driver-model based wakeup, use that state. */
+	if (phy_can_wakeup(phydev))
+		return device_may_wakeup(&phydev->mdio.dev);
+
+	return phy_drv_wol_enabled(phydev);
+}
+EXPORT_SYMBOL_GPL(phy_may_wakeup);
+
 static void phy_link_change(struct phy_device *phydev, bool up)
 {
 	struct net_device *netdev = phydev->attached_dev;
@@ -302,7 +312,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 	/* If the PHY on the mido bus is not attached but has WOL enabled
 	 * we cannot suspend the PHY.
 	 */
-	if (!netdev && phy_drv_wol_enabled(phydev))
+	if (!netdev && phy_may_wakeup(phydev))
 		return false;
 
 	/* PHY not attached? May suspend if the PHY has not already been
@@ -1909,7 +1919,7 @@ int phy_suspend(struct phy_device *phydev)
 	if (phydev->suspended || !phydrv)
 		return 0;
 
-	phydev->wol_enabled = phy_drv_wol_enabled(phydev) ||
+	phydev->wol_enabled = phy_may_wakeup(phydev) ||
 			      (netdev && netdev->ethtool->wol_enabled);
 	/* If the device has WOL enabled, we cannot suspend the PHY */
 	if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
include/linux/phy.h +0 -0
index 3eeeaec52832..801356da1fb2 100644

include/linux/phy.h static inline bool phy_can_wakeup(struct phy_device *phydev) · line 1399

Quoted 9 lines
include/linux/phy.h +6 -0
@@ -1391,6 +1391,15 @@ static inline bool phy_can_wakeup(struct phy_device *phydev)
 	return device_can_wakeup(&phydev->mdio.dev);
 }
 
+/**
+ * phy_may_wakeup() - indicate whether PHY has driver model wakeup is enabled
+ * @phydev: The phy_device struct
+ *
+ * Returns: true/false depending on the PHY driver's device_set_wakeup_enabled()
+ * setting.

That's not exactly what's happening, this suggest this is merely a wrapper around device_may_wakeup().

I don't think it's worth blocking the series though, but if you need to respin maybe this could be reworded.

include/linux/phy.h static inline bool phy_can_wakeup(struct phy_device *phydev) · line 1405

Quoted 6 lines
include/linux/phy.h +3 -0
+ */
+bool phy_may_wakeup(struct phy_device *phydev);
+
 void phy_resolve_aneg_pause(struct phy_device *phydev);
 void phy_resolve_aneg_linkmode(struct phy_device *phydev);
 

Maxime

maintainer Reviewed-by on v1 3/6
#12 lore raw

Re: [PATCH net-next 3/6] net: phylink: add phylink managed MAC Wake-on-Lan support

In reply to Russell King (Oracle)
Quoted 130 lines · Russell King (Oracle)

Add core phylink managed Wake-on-Lan support, which is enabled when the MAC driver fills in the new .mac_wol_set() method that this commit creates.

When this feature is disabled, phylink acts as it has in the past, merely passing the ethtool WoL calls to phylib whenever a PHY exists. No other new functionality provided by this commit is enabled.

When this feature is enabled, a more inteligent approach is used. Phylink will first pass WoL options to the PHY, read them back, and attempt to set any options that were not set at the PHY at the MAC.

Since we have PHY drivers that report they support WoL, and accept WoL configuration even though they aren't wired up to be capable of waking the system, we need a way to differentiate between PHYs that think they support WoL and those which actually do. As PHY drivers do not make use of the driver model's wake-up infrastructure, but could, we use this to determine whether PHY drivers can participate. This gives a path forward where, as MAC drivers are converted to this, it encourages PHY drivers to also be converted.

Phylink will also ignore the mac_wol argument to phylink_suspend() as it now knows the WoL state at the MAC.

MAC drivers are expected to record/configure the Wake-on-Lan state in their .mac_set_wol() method, and deal appropriately with it in their suspend/resume methods. The driver model provides assistance to set the IRQ wake support which may assist driver authors in achieving the necessary configuration.

---
 drivers/net/phy/phylink.c | 80 +++++++++++++++++++++++++++++++++++++--
 include/linux/phylink.h   | 26 +++++++++++++
 2 files changed, 102 insertions(+), 4 deletions(-)
drivers/net/phy/phylink.c +45 -4
index 9d7799ea1c17..939438a6d6f5 100644
@@ -93,6 +93,9 @@ struct phylink {
 	u8 sfp_port;
 
 	struct eee_config eee_cfg;
+
+	u32 wolopts_mac;
+	u8 wol_sopass[SOPASS_MAX];
 };
 
 #define phylink_printk(level, pl, fmt, ...) \
@@ -2562,6 +2565,17 @@ void phylink_rx_clk_stop_unblock(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_rx_clk_stop_unblock);
 
+static bool phylink_mac_supports_wol(struct phylink *pl)
+{
+	return !!pl->mac_ops->mac_wol_set;
+}
+
+static bool phylink_phy_supports_wol(struct phylink *pl,
+				     struct phy_device *phydev)
+{
+	return phydev && (pl->config->wol_phy_legacy || phy_can_wakeup(phydev));
+}
+
 /**
  * phylink_suspend() - handle a network device suspend event
  * @pl: a pointer to a &struct phylink returned from phylink_create()
@@ -2575,11 +2589,17 @@ EXPORT_SYMBOL_GPL(phylink_rx_clk_stop_unblock);
  *   can also bring down the link between the MAC and PHY.
  * - If Wake-on-Lan is active, but being handled by the MAC, the MAC
  *   still needs to receive packets, so we can not bring the link down.
+ *
+ * Note: when phylink managed Wake-on-Lan is in use, @mac_wol is ignored.
+ * (struct phylink_mac_ops.mac_set_wol populated.)
  */
 void phylink_suspend(struct phylink *pl, bool mac_wol)
 {
 	ASSERT_RTNL();
 
+	if (phylink_mac_supports_wol(pl))
+		mac_wol = !!pl->wolopts_mac;
+
 	if (mac_wol && (!pl->netdev || pl->netdev->ethtool->wol_enabled)) {
 		/* Wake-on-Lan enabled, MAC handling */
 		mutex_lock(&pl->state_mutex);
@@ -2689,8 +2709,24 @@ void phylink_ethtool_get_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 	wol->supported = 0;
 	wol->wolopts = 0;
 
-	if (pl->phydev)
-		phy_ethtool_get_wol(pl->phydev, wol);
+	if (phylink_mac_supports_wol(pl)) {
+		if (phylink_phy_supports_wol(pl, pl->phydev))
+			phy_ethtool_get_wol(pl->phydev, wol);
+
+		/* Where the MAC augments the WoL support, merge its support and
+		 * current configuration.
+		 */
+		if (~wol->wolopts & pl->wolopts_mac & WAKE_MAGICSECURE)
+			memcpy(wol->sopass, pl->wol_sopass,
+			       sizeof(wol->sopass));
+
+		wol->supported |= pl->config->wol_mac_support;
+		wol->wolopts |= pl->wolopts_mac;
+	} else {
+		/* Legacy */
+		if (pl->phydev)
+			phy_ethtool_get_wol(pl->phydev, wol);
+	}
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_wol);
 
@@ -2707,12 +2743,48 @@ EXPORT_SYMBOL_GPL(phylink_ethtool_get_wol);
  */
 int phylink_ethtool_set_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 {
+	struct ethtool_wolinfo w;
 	int ret = -EOPNOTSUPP;
+	bool changed;
+	u32 wolopts;
 
 	ASSERT_RTNL();
 
-	if (pl->phydev)
-		ret = phy_ethtool_set_wol(pl->phydev, wol);
+	if (phylink_mac_supports_wol(pl)) {
+		wolopts = wol->wolopts;
+
+		if (phylink_phy_supports_wol(pl, pl->phydev)) {

drivers/net/phy/phylink.c EXPORT_SYMBOL_GPL(phylink_ethtool_get_wol); · line 2766

Quoted 10 lines
drivers/net/phy/phylink.c +10 -0
+			ret = phy_ethtool_set_wol(pl->phydev, wol);
+			if (ret != 0 && ret != -EOPNOTSUPP)
+				return ret;
+
+			phy_ethtool_get_wol(pl->phydev, &w);
+
+			/* Any Wake-on-Lan modes which the PHY is handling
+			 * should not be passed on to the MAC.
+			 */
+			wolopts &= ~w.wolopts;

When PHY drivers gets converted to the new model, we'll have to look at how the .get_wol() behave WRT how they fill-in their wolopts.

The Broadcom driver for example may not set w.wolopts to 0 :

https://elixir.bootlin.com/linux/v6.17.4/source/drivers/net/phy/broadcom.c#L1121

You'd probably end-up with garbage here then. But not blocking for your series.

Quoted 79 lines
drivers/net/phy/phylink.c +21 -0
+		}
+
+		wolopts &= pl->config->wol_mac_support;
+		changed = pl->wolopts_mac != wolopts;
+		if (wolopts & WAKE_MAGICSECURE)
+			changed |= !!memcmp(wol->sopass, pl->wol_sopass,
+					    sizeof(wol->sopass));
+		memcpy(pl->wol_sopass, wol->sopass, sizeof(pl->wol_sopass));
+
+		if (changed) {
+			ret = pl->mac_ops->mac_wol_set(pl->config, wolopts,
+						       wol->sopass);
+			if (!ret)
+				pl->wolopts_mac = wolopts;
+		} else {
+			ret = 0;
+		}
+	} else {
+		if (pl->phydev)
+			ret = phy_ethtool_set_wol(pl->phydev, wol);
+	}
 
 	return ret;
 }
include/linux/phylink.h +19 -0
index 9af0411761d7..59cb58b29d1d 100644
@@ -156,6 +156,8 @@ enum phylink_op_type {
  * @lpi_capabilities: MAC speeds which can support LPI signalling
  * @lpi_timer_default: Default EEE LPI timer setting.
  * @eee_enabled_default: If set, EEE will be enabled by phylink at creation time
+ * @wol_phy_legacy: Use Wake-on-Lan with PHY even if phy_can_wakeup() is false
+ * @wol_mac_support: Bitmask of MAC supported %WAKE_* options
  */
 struct phylink_config {
 	struct device *dev;
@@ -173,6 +175,10 @@ struct phylink_config {
 	unsigned long lpi_capabilities;
 	u32 lpi_timer_default;
 	bool eee_enabled_default;
+
+	/* Wake-on-Lan support */
+	bool wol_phy_legacy;
+	u32 wol_mac_support;
 };
 
 void phylink_limit_mac_speed(struct phylink_config *config, u32 max_speed);
@@ -188,6 +194,7 @@ void phylink_limit_mac_speed(struct phylink_config *config, u32 max_speed);
  * @mac_link_up: allow the link to come up.
  * @mac_disable_tx_lpi: disable LPI.
  * @mac_enable_tx_lpi: enable and configure LPI.
+ * @mac_wol_set: configure Wake-on-Lan settings at the MAC.
  *
  * The individual methods are described more fully below.
  */
@@ -211,6 +218,9 @@ struct phylink_mac_ops {
 	void (*mac_disable_tx_lpi)(struct phylink_config *config);
 	int (*mac_enable_tx_lpi)(struct phylink_config *config, u32 timer,
 				 bool tx_clk_stop);
+
+	int (*mac_wol_set)(struct phylink_config *config, u32 wolopts,
+			   const u8 *sopass);
 };
 
 #if 0 /* For kernel-doc purposes only. */
@@ -440,6 +450,22 @@ void mac_disable_tx_lpi(struct phylink_config *config);
  */
 int mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
 		      bool tx_clk_stop);
+
+/**
+ * mac_wol_set() - configure the Wake-on-Lan parameters
+ * @config: a pointer to a &struct phylink_config.
+ * @wolopts: Bitmask of %WAKE_* flags for enabled Wake-On-Lan modes.
+ * @sopass: SecureOn(tm) password; meaningful only for %WAKE_MAGICSECURE
+ *
+ * Enable the specified Wake-on-Lan options at the MAC. Options that the
+ * PHY can handle will have been removed from @wolopts.

include/linux/phylink.h void mac_disable_tx_lpi(struct phylink_config *config); · line 471

Quoted 10 lines
include/linux/phylink.h +7 -0
+ *
+ * The presence of this method enables phylink-managed WoL support.
+ *
+ * Returns: 0 on success.
+ */
+int (*mac_wol_set)(struct phylink_config *config, u32 wolopts,
+		   const u8 *sopass);
 #endif
 
 struct phylink_pcs_ops;

As far as my current WoL knowedge goes, this looks good to me.

Maxime

#16 lore raw

Re: [PATCH net-next 4/6] net: phylink: add phylink managed wake-on-lan PHY speed control

In reply to Russell King (Oracle)

Hi Russell,

Quoted 34 lines · Russell King (Oracle)

Some drivers, e.g. stmmac, use the speed_up()/speed_down() APIs to gain additional power saving during Wake-on-LAN where the PHY is managing the state.

Add support to phylink for this, which can be enabled by the MAC driver. Only change the PHY speed if the PHY is configured for wake-up, but without any wake-up on the MAC side, as MAC side means changing the configuration once the negotiation has completed.

---
 drivers/net/phy/phylink.c | 12 ++++++++++++
 include/linux/phylink.h   |  2 ++
 2 files changed, 14 insertions(+)
drivers/net/phy/phylink.c +6 -0
index 939438a6d6f5..26bd4b7619dd 100644
@@ -2576,6 +2576,12 @@ static bool phylink_phy_supports_wol(struct phylink *pl,
 	return phydev && (pl->config->wol_phy_legacy || phy_can_wakeup(phydev));
 }
 
+static bool phylink_phy_pm_speed_ctrl(struct phylink *pl)
+{
+	return pl->config->wol_phy_speed_ctrl && !pl->wolopts_mac &&
+	       pl->phydev && phy_may_wakeup(pl->phydev);
+}
+
 /**
  * phylink_suspend() - handle a network device suspend event
  * @pl: a pointer to a &struct phylink returned from phylink_create()

drivers/net/phy/phylink.c void phylink_suspend(struct phylink *pl, bool mac_wol) · line 2636

Quoted 6 lines
drivers/net/phy/phylink.c +3 -0
@@ -2625,6 +2631,9 @@ void phylink_suspend(struct phylink *pl, bool mac_wol)
 	} else {
 		phylink_stop(pl);
 	}
+
+	if (phylink_phy_pm_speed_ctrl(pl))
+		phy_speed_down(pl->phydev, false);

Should this rather be phylink_speed_down, to take into account the fact
that the PHY might be on SFP ? either here or directly in
phylink_phy_pm_speed_ctrl() above ?

Maxime

author maintainer
#17 lore raw

Re: [PATCH net-next 4/6] net: phylink: add phylink managed wake-on-lan PHY speed control

Quoted 9 lines · Maxime Chevallier

Hi Russell,

drivers/net/phy/phylink.c void phylink_suspend(struct phylink *pl, bool mac_wol) · line 2636

Quoted 3 lines
drivers/net/phy/phylink.c +3 -0
+
+	if (phylink_phy_pm_speed_ctrl(pl))
+		phy_speed_down(pl->phydev, false);

Should this rather be phylink_speed_down, to take into account the fact
that the PHY might be on SFP ? either here or directly in
phylink_phy_pm_speed_ctrl() above ?

I think using phylink_speed_*() makes more sense than merging the test into phylink_phy_pm_speed_ctrl(). If something changes in what we do with speed_up/down() then we want everyone to be affected (not that I forsee any change there.) Logically though...

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Reviewed-by on v1 2/6
#9 lore raw

Re: [PATCH net-next 2/6] net: phy: add phy_may_wakeup()

In reply to Russell King (Oracle)
Quoted 11 lines · Russell King (Oracle)

Add phy_may_wakeup() which uses the driver model's device_may_wakeup() when the PHY driver has marked the device as wakeup capable in the driver model, otherwise use phy_drv_wol_enabled().

Replace the sites that used to call phy_drv_wol_enabled() with this as checking the driver model will be more efficient than checking the WoL state.

Export phy_may_wakeup() so that phylink can use it.

Signature · 1 line
Florian
Reviewed-by on v1 3/6
#14 lore raw

Re: [PATCH net-next 3/6] net: phylink: add phylink managed MAC Wake-on-Lan support

In reply to Russell King (Oracle)
Quoted 24 lines · Russell King (Oracle)

Add core phylink managed Wake-on-Lan support, which is enabled when the MAC driver fills in the new .mac_wol_set() method that this commit creates.

When this feature is disabled, phylink acts as it has in the past, merely passing the ethtool WoL calls to phylib whenever a PHY exists. No other new functionality provided by this commit is enabled.

When this feature is enabled, a more inteligent approach is used. Phylink will first pass WoL options to the PHY, read them back, and attempt to set any options that were not set at the PHY at the MAC.

Since we have PHY drivers that report they support WoL, and accept WoL configuration even though they aren't wired up to be capable of waking the system, we need a way to differentiate between PHYs that think they support WoL and those which actually do. As PHY drivers do not make use of the driver model's wake-up infrastructure, but could, we use this to determine whether PHY drivers can participate. This gives a path forward where, as MAC drivers are converted to this, it encourages PHY drivers to also be converted.

Phylink will also ignore the mac_wol argument to phylink_suspend() as it now knows the WoL state at the MAC.

Quoted 7 lines

MAC drivers are expected to record/configure the Wake-on-Lan state in their .mac_set_wol() method, and deal appropriately with it in their suspend/resume methods. The driver model provides assistance to set the IRQ wake support which may assist driver authors in achieving the necessary configuration.

Signature · 1 line
Florian
author maintainer
#6 lore raw

Re: [PATCH net-next 2/6] net: phy: add phy_may_wakeup()

In reply to Maxime Chevallier
Quoted 9 lines · Maxime Chevallier

Hi Russell,

That's not exactly what's happening, this suggest this is merely a wrapper around device_may_wakeup().

I don't think it's worth blocking the series though, but if you need to respin maybe this could be reworded.

I've updated the description as I think patch 4 needs a repost:

+/**
+ * phy_may_wakeup() - indicate whether PHY has wakeup enabled
+ * @phydev: The phy_device struct
+ *
+ * Returns: true/false depending on the PHY driver's device_set_wakeup_enabled()
+ * setting if using the driver model, otherwise the legacy determination.
+ */
+bool phy_may_wakeup(struct phy_device *phydev);
+

Do you want me to still add your r-b?

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
author maintainer
#10 lore raw

Re: [PATCH net-next 2/6] net: phy: add phy_may_wakeup()

In reply to Florian Fainelli
Quoted 9 lines · Florian Fainelli
Quoted 8 lines · Russell King (Oracle)

Add phy_may_wakeup() which uses the driver model's device_may_wakeup() when the PHY driver has marked the device as wakeup capable in the driver model, otherwise use phy_drv_wol_enabled().

Replace the sites that used to call phy_drv_wol_enabled() with this as checking the driver model will be more efficient than checking the WoL state.

Quoted 5 lines
Quoted 3 lines

Export phy_may_wakeup() so that phylink can use it.

Note my reply to Maxime, I've changed the description to:

+/**
+ * phy_may_wakeup() - indicate whether PHY has wakeup enabled
+ * @phydev: The phy_device struct
+ *
+ * Returns: true/false depending on the PHY driver's device_set_wakeup_enabled()
+ * setting if using the driver model, otherwise the legacy determination.
+ */
+bool phy_may_wakeup(struct phy_device *phydev);
+

Are you still okay for me to add your r-b?

Thanks.

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
author maintainer
#13 lore raw

Re: [PATCH net-next 3/6] net: phylink: add phylink managed MAC Wake-on-Lan support

In reply to Maxime Chevallier
Quoted 3 lines · Maxime Chevallier
Quoted 3 lines

int phylink_ethtool_set_wol(struct phylink *pl, struct ethtool_wolinfo *wol) {

drivers/net/phy/phylink.c +1 -0
+	struct ethtool_wolinfo w;

...

Quoted 10 lines

drivers/net/phy/phylink.c EXPORT_SYMBOL_GPL(phylink_ethtool_get_wol); · line 2766

Quoted 6 lines
drivers/net/phy/phylink.c +6 -0
+			phy_ethtool_get_wol(pl->phydev, &w);
+
+			/* Any Wake-on-Lan modes which the PHY is handling
+			 * should not be passed on to the MAC.
+			 */
+			wolopts &= ~w.wolopts;

When PHY drivers gets converted to the new model, we'll have to look at how the .get_wol() behave WRT how they fill-in their wolopts.

Quoted 5 lines

The Broadcom driver for example may not set w.wolopts to 0 :

https://elixir.bootlin.com/linux/v6.17.4/source/drivers/net/phy/broadcom.c#L1121

You'd probably end-up with garbage here then. But not blocking for your series.

Good point. We should initialise 'w' in the same way other users do, so:

+ struct ethtool_wolinfo w = { .cmd = ETHTOOL_GWOL };

Does your r-b still stand with this change?

Thanks.

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#7 lore raw

Re: [PATCH net-next 2/6] net: phy: add phy_may_wakeup()

In reply to Russell King (Oracle)
Quoted 23 lines · Russell King (Oracle)
Quoted 9 lines · Maxime Chevallier

Hi Russell,

That's not exactly what's happening, this suggest this is merely a wrapper around device_may_wakeup().

I don't think it's worth blocking the series though, but if you need to respin maybe this could be reworded.

I've updated the description as I think patch 4 needs a repost:

include/linux/phy.h +9 -0
+/**
+ * phy_may_wakeup() - indicate whether PHY has wakeup enabled
+ * @phydev: The phy_device struct
+ *
+ * Returns: true/false depending on the PHY driver's device_set_wakeup_enabled()
+ * setting if using the driver model, otherwise the legacy determination.
+ */
+bool phy_may_wakeup(struct phy_device *phydev);
+
Quoted 1 line

Do you want me to still add your r-b?

Yes, I saw that comment, should have mentioned this applied to your revision. Thanks!

Signature · 1 line
Florian
#8 lore raw

Re: [PATCH net-next 2/6] net: phy: add phy_may_wakeup()

In reply to Russell King (Oracle)
Quoted 23 lines · Russell King (Oracle)
Quoted 9 lines · Maxime Chevallier

Hi Russell,

That's not exactly what's happening, this suggest this is merely a wrapper around device_may_wakeup().

I don't think it's worth blocking the series though, but if you need to respin maybe this could be reworded.

I've updated the description as I think patch 4 needs a repost:

include/linux/phy.h +9 -0
+/**
+ * phy_may_wakeup() - indicate whether PHY has wakeup enabled
+ * @phydev: The phy_device struct
+ *
+ * Returns: true/false depending on the PHY driver's device_set_wakeup_enabled()
+ * setting if using the driver model, otherwise the legacy determination.
+ */
+bool phy_may_wakeup(struct phy_device *phydev);
+
Quoted 1 line

Do you want me to still add your r-b?

Yes with this change, that's good for me you can add the tag :)

Thanks Russell,

Maxime