Message-ID: <aMriVDAgZkL8DAdH@shell.armlinux.org.uk>
Date: Wed, 17 Sep 2025 17:31:16 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Gatien Chevallier <gatien.chevallier@...s.st.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Christophe Roullier <christophe.roullier@...s.st.com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Simon Horman <horms@...nel.org>,
	Tristram Ha <Tristram.Ha@...rochip.com>,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	netdev@...r.kernel.org, devicetree@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY
 support

On Wed, Sep 17, 2025 at 05:36:37PM +0200, Gatien Chevallier wrote:
> If the "st,phy-wol" property is present in the device tree node,
> set the STMMAC_FLAG_USE_PHY_WOL flag to use the WoL capability of
> the PHY.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@...s.st.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> index 77a04c4579c9dbae886a0b387f69610a932b7b9e..6f197789cc2e8018d6959158b795e4bca46869c5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -106,6 +106,7 @@ struct stm32_dwmac {
>  	u32 speed;
>  	const struct stm32_ops *ops;
>  	struct device *dev;
> +	bool phy_wol;
>  };
>  
>  struct stm32_ops {
> @@ -433,6 +434,8 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
>  		}
>  	}
>  
> +	dwmac->phy_wol = of_property_read_bool(np, "st,phy-wol");
> +
>  	return err;
>  }
>  
> @@ -557,6 +560,8 @@ static int stm32_dwmac_probe(struct platform_device *pdev)
>  	plat_dat->bsp_priv = dwmac;
>  	plat_dat->suspend = stm32_dwmac_suspend;
>  	plat_dat->resume = stm32_dwmac_resume;
> +	if (dwmac->phy_wol)
> +		plat_dat->flags |= STMMAC_FLAG_USE_PHY_WOL;

I would much rather we found a different approach, rather than adding
custom per-driver DT properties to figure this out.

Andrew has previously suggested that MAC drivers should ask the PHY
whether WoL is supported, but this pre-supposes that PHY drivers are
coded correctly to only report WoL capabilities if they are really
capable of waking the system. As shown in your smsc PHY driver patch,
this may not be the case.

Given that we have historically had PHY drivers reporting WoL
capabilities without being able to wake the system, we can't
implement Andrew's suggestion easily.

The only approach I can think that would allow us to transition is
to add:

static inline bool phy_can_wakeup(struct phy_device *phy_dev)
{
	return device_can_wakeup(&phy_dev->mdio.dev);
}

to include/linux/phy.h, and a corresponding wrapper for phylink.
This can then be used to determine whether to attempt to use PHY-based
Wol in stmmac_get_wol() and rtl8211f_set_wol(), falling back to
PMT-based WoL if supported at the MAC.

So, maybe something like:

static u32 stmmac_wol_support(struct stmmac_priv *priv)
{
	u32 support = 0;

	if (priv->plat->pmt && device_can_wakeup(priv->device)) {
		support = WAKE_UCAST;
		if (priv->hw_cap_support && priv->dma_cap.pmt_magic_frame)
			support |= WAKE_MAGIC;
	}

	return support;
}

static void stmmac_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
{
	struct stmmac_priv *priv = netdev_priv(dev);
	int err;

	/* Check STMMAC_FLAG_USE_PHY_WOL for legacy */
	if (phylink_can_wakeup(priv->phylink) ||
	    priv->plat->flags & STMMAC_FLAG_USE_PHY_WOL) {
		err = phylink_ethtool_get_wol(priv->phylink, wol);
		if (err != 0 && err != -EOPNOTSUPP)
			return;
	}

	wol->supported |= stmmac_wol_support(priv);

	/* A read of priv->wolopts is single-copy atomic. Locking
	 * doesn't add any benefit.
	 */
	wol->wolopts |= priv->wolopts;
}

static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
{
	struct stmmac_priv *priv = netdev_priv(dev);
	u32 support, wolopts;
	int err;

	wolopts = wol->wolopts;

	/* Check STMMAC_FLAG_USE_PHY_WOL for legacy */
	if (phylink_can_wakeup(priv->phylink) ||
	    priv->plat->flags & STMMAC_FLAG_USE_PHY_WOL) {
		struct ethtool_wolinfo w;

		err = phylink_ethtool_set_wol(priv->phylink, wol);
		if (err != -EOPNOTSUPP)
			return err;

		/* Remove the WoL modes that the PHY is handling */
		if (!phylink_ethtool_get_wol(priv->phylink, &w))
			wolopts &= ~w.wolopts;
	}

	support = stmmac_wol_support(priv);

	mutex_lock(&priv->lock);
	priv->wolopts = wolopts & support;
	device_set_wakeup_enable(priv->device, !!priv->wolopts);
	mutex_unlock(&priv->lock);

	return 0;
}

... and now I'm wondering whether this complexity is something that
phylink should handle internally, presenting a mac_set_wol() method
to configure the MAC-side WoL settings. What makes it difficult to
just move into phylink is the STMMAC_FLAG_USE_PHY_WOL flag, but
that could be a "force_phy_wol" flag in struct phylink_config as
a transitionary measure... so long as PHY drivers get fixed.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists