Message-ID: <6bff48d0-dd19-48d4-91e6-0d991365b8f9@bootlin.com>
Date: Wed, 22 Oct 2025 16:06:00 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
 Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, linux-arm-kernel@...ts.infradead.org,
 linux-stm32@...md-mailman.stormreply.com,
 Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org,
 Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 3/6] net: phylink: add phylink managed MAC
 Wake-on-Lan support



On 17/10/2025 14:04, Russell King (Oracle) wrote:
> 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.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> ---
>  drivers/net/phy/phylink.c | 80 +++++++++++++++++++++++++++++++++++++--
>  include/linux/phylink.h   | 26 +++++++++++++
>  2 files changed, 102 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 9d7799ea1c17..939438a6d6f5 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -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;

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.

> +		}
> +
> +		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;
>  }
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index 9af0411761d7..59cb58b29d1d 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -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;

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

Reviewed-by: Maxime Chevallier <maxime.chevallier@...tlin.com>

Maxime

Powered by blists - more mailing lists