[PATCH net-next v2 0/4] net: add WoL from PHY support for stm32mp135f-dk

[PATCH net-next v2 0/4] net: add WoL from PHY support for stm32mp135f-dk

[Gatien Chevallier]

A previous patchset in drivers/net/phy/smsc.c introduced the WoL
from PHY capability of some PHYs. The Microchip LAN8742 PHY is
present on the stm32mp135f-dk board and posesses this capability.

Therefore, add the possibility to specify in the device tree that,
for a MAC instance, we want to use the WoL from PHY capability
instead of the MAC one.

However, when testing multiple power sequences with magic packets,
the first one succeeded but the following ones failed. This was
caused by uncleared flags in a PHY register. Therefore, I added
a patch to add suspend()/resume() callbacks that handle these.
These callbacks are only implemented for the Microchip LAN8742 as I
have no way of testing it for other WoL capable PHYs.

Note: This patchset does not solve the issue regarding the PHY pin
alternate fuctions (nPME/nINT). The driver still statically configures
the LED2/nINT/nPME in nPME mode. The interrupt management discussed in
V1 still needs rework, and probably at framework level.

The current state after this patchset is:
- Successive system wakeup with WoL event now possible
- WoL event received while the system is up do not prevent system wakeup
  from a WoL event. I took inspiration from the broadcom driver
  regarding this issue

Question: Given that the PHYs have pins with alternate functions that
are difficult to handle (some drivers like smsc.c are configuring them
statically), should we consider working on a pinctrl-like solution for
them?

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
Changes in v2:
- Rework commit message of the bindings patch
- Handle WoL flags in Microchip driver suspend() callback and always
  call the suspend callback.
- Link to v1: https://lore.kernel.org/r/20250721-wol-smsc-phy-v1-0-89d262812dba@foss.st.com

---
Gatien Chevallier (4):
      dt-bindings: net: document st,phy-wol property
      net: stmmac: stm32: add WoL from PHY support
      net: phy: smsc: fix and improve WoL support
      arm: dts: st: activate ETH1 WoL from PHY on stm32mp135f-dk

 .../devicetree/bindings/net/stm32-dwmac.yaml       |  6 ++
 arch/arm/boot/dts/st/stm32mp135f-dk.dts            |  1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  |  5 ++
 drivers/net/phy/smsc.c                             | 66 ++++++++++++++++++++--
 include/linux/smscphy.h                            |  2 +
 5 files changed, 75 insertions(+), 5 deletions(-)
---
base-commit: 5e87fdc37f8dc619549d49ba5c951b369ce7c136
change-id: 20250721-wol-smsc-phy-ff3b40848852

Best regards,
-- 
Gatien Chevallier <gatien.chevallier@foss.st.com>

[PATCH net-next v2 1/4] dt-bindings: net: document st,phy-wol property

[Gatien Chevallier]

Add the  "st,phy-wol" to indicate the MAC to use the wakeup capability
of the PHY instead of the MAC.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
index 987254900d0da7aab81237f20b1540ad8a17bd21..985bd4c320b3e07fd1cd0aa398d6cce0b55a4e4d 100644
--- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
@@ -121,6 +121,12 @@ properties:
     minItems: 1
     maxItems: 2
 
+  st,phy-wol:
+    description:
+      set this property to use the wakeup capability from the PHY, if supported, instead of the
+      MAC.
+    type: boolean
+
 required:
   - compatible
   - clocks

-- 
2.35.3

Re: [PATCH net-next v2 1/4] dt-bindings: net: document st,phy-wol property

[Rob Herring]

On Wed, Sep 17, 2025 at 05:36:36PM +0200, Gatien Chevallier wrote:
> Add the  "st,phy-wol" to indicate the MAC to use the wakeup capability
> of the PHY instead of the MAC.

Why is this ST specific? PHYs being wakeup capable or not is independent 
of ST. If you want to or can use wakeup from the PHY, shouldn't that be 
a property in the PHY?

Seems to me you would want to define what all components are wakeup 
capable and then let the kernel decide which component to use. I'd think 
the kernel would prefer the PHY as that's closest to the wire and 
probably lowest power.

That's my 2 cents spending all of 5 minutes thinking about it. I'll 
defer to Russell and Andrew...

Rob

Re: [PATCH net-next v2 1/4] dt-bindings: net: document st,phy-wol property

[Gatien CHEVALLIER]

On 9/22/25 19:05, Rob Herring wrote:
> On Wed, Sep 17, 2025 at 05:36:36PM +0200, Gatien Chevallier wrote:
>> Add the  "st,phy-wol" to indicate the MAC to use the wakeup capability
>> of the PHY instead of the MAC.
> 
> Why is this ST specific? PHYs being wakeup capable or not is independent
> of ST. If you want to or can use wakeup from the PHY, shouldn't that be
> a property in the PHY?
> 
> Seems to me you would want to define what all components are wakeup
> capable and then let the kernel decide which component to use. I'd think
> the kernel would prefer the PHY as that's closest to the wire and
> probably lowest power.
> 
> That's my 2 cents spending all of 5 minutes thinking about it. I'll
> defer to Russell and Andrew...
> 
> Rob

Hi Rob,

I think we're all aligned on this now. The thing about this property
being added is really related to how the STMMAC driver was interacting
with the PHY drivers and how the PHY drivers communicate their WoL
capability. It has been done previously for the "mediatek,mac-wol"
property.

The complicated thing to handle is: Is the PHY capable of doing WoL
AND is the wiring actually present on the platform on the correct
pin.

I'll send a V3 taking into account the different discussions we had
with Andrew and Russell.

Best regards,
Gatien

[PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Gatien Chevallier]

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@foss.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;
 
 	ret = stm32_dwmac_init(plat_dat);
 	if (ret)

-- 
2.35.3

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Russell King (Oracle)]

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@foss.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!

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Gatien CHEVALLIER]

On 9/17/25 18:31, Russell King (Oracle) wrote:
> 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@foss.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.

So how can we distinguish whether a PHY that implements WoL features
is actually able (wired) to wake up the system? By adding the
"wakeup-source" property to the PHY node?

Therefore, only set the "can wakeup" capability when both the PHY
supports WoL and the property is present in the PHY node?

However, this does not solve the actual static pin function
configuration for pins that can, if correct alternate function is
selected, generate interrupts, in PHY drivers.

It would be nice to be able to apply some kind of pinctrl to configure
the PHY pins over the MDIO bus thanks to some kind of pinctrl hogging.
This suggests modifying relevant PHY drivers and documentation to be
able to handle an optional pinctrl.

Disregarding syntax issues, could be something like:

phy0_eth1: ethernet-phy@0 {
	compatible = "ethernet-phy-id0007.c131";
	reg = <0>;
	reset-gpios = <&mcp23017 9 GPIO_ACTIVE_LOW>;
	wakeup-source;
	pinctrl-0 = <&phy_pin_npme_hog &phy_pin_nint_hog>;

	phy_pin_npme_hog: nPME {
		pins = "LED2/nINT/nPME/nINTSEL";
		function = "nPME";
	};

	phy_pin_nint_hog: nINT {
		pins = "LED1/nINT/nPME/nINTSEL";
		function = "nINT";
	};
};

What do you think of that?

Otherwise, the idea below looks promising to me.

> 
> 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.
> 

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Russell King (Oracle)]

On Thu, Sep 18, 2025 at 02:46:54PM +0200, Gatien CHEVALLIER wrote:
> On 9/17/25 18:31, Russell King (Oracle) wrote:
> > 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@foss.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.
> 
> So how can we distinguish whether a PHY that implements WoL features
> is actually able (wired) to wake up the system? By adding the
> "wakeup-source" property to the PHY node?

Andrew's original idea was essentially that if the PHY reports that it
supports WoL, then it's functional.

Sadly, that's not the case with many PHY drivers - the driver
implementers just considered "does this PHY have the ability to detect
WoL packets" and not "can this PHY actually wake the system."

Thankfully, all but one PHY driver does not use
device_set_wakeup_capable() - my recent patches for realtek look like
the first PHY driver to use this.

Thus, if we insist that PHY drivers use device_set_wakeup_capable()
to indicate that (a) they have WoL capability _and_ are really
capable of waking the system, we have a knob we can test for.

Sadly, there is no way to really know whether the interrupt that the
PHY is attached to can wake the system. Things get worse with PHYs
that don't use interrupts to wake the system. So, I would suggest
that, as we already have this "wakeup-source" property available for
_any_ device in DT, we start using this to say "on this system, this
PHY is connected to something that can wake the system up."

See the past discussion when Realtek was being added - some of the
context there covers what I mention above.

> Therefore, only set the "can wakeup" capability when both the PHY
> supports WoL and the property is present in the PHY node?

Given that telling the device model that a device is wakeup
capable causes this to be advertised to userspace, we really do
not want devices saying that they are wakeup capable when they
aren't capable of waking the system. So I would say that a call
to device_set_wakeup_capable(dev, true) should _only_ be made if
the driver is 100% certain that this device really can, without
question, wake the platform.

If we don't have that guarantee, then we're on a hiding to nothing
and chaos will reign, MAC drivers won't work properly... but I would
suggest that's the price to be paid for shoddy implementation and
not adhering to a sensible approach such as what I outline above.

> However, this does not solve the actual static pin function
> configuration for pins that can, if correct alternate function is
> selected, generate interrupts, in PHY drivers.
> 
> It would be nice to be able to apply some kind of pinctrl to configure
> the PHY pins over the MDIO bus thanks to some kind of pinctrl hogging.
> This suggests modifying relevant PHY drivers and documentation to be
> able to handle an optional pinctrl.

How would that work with something like the Realtek 8821F which has
a single pin which can either signal interrupts (including a wake-up)
or be in PME mode, where it only ever signals a wake-up event.
Dynamically switching between the two modes is what got us into the
crazy situation where, when WoL was enabled on this PHY, phylib
stopped working because the pin was switched to PME mode, and we no
longer got link status interrupts. So one could enable WoL, plug in
an ethernet cable, and the kernel has no idea that the link has come
up.

So no. In a situation like this, either we want to be in interrupt
mode (in which case we have an interrupt), or the pin is wired to
a power management controller and needs to be in PME mode, or it isn't
wired.

Which it is can be easily identified.

$1. Is there an interrupt specified (Y/N) ?
$2. Is there a wakeup-source property (Y/N) ?

States:
$1  $2
*   N   we have no idea if an interrupt (if specified) can wake the
        system, or if there is other wiring from the PHY which might.
	Legacy driver, or has no wake-up support. We have to fall back
	on existing approaches to maintain compatibility and void
	breaking userspace, which may suggest WoL is supported when it
	isn't. For example, with stmmac, if STMMAC_FLAG_USE_PHY_WOL is
	set, we must assume the PHY can wake the system in this case.
Y   Y   interrupt wakes the system, we're good for WoL
N   Y   non-interrupt method of waking the system, e.g. PME

I'd prefer not to go for a complicated solution to this, e.g. involving
pinctrl, when we don't know how many PHYs need this, because forcing
extra complexity on driver authors means we have yet more to review, and
I think it's fair to say that we're already missing stuff. Getting the
pinctrl stuff right also requires knowledge of the hardware, and is
likely something that reviewers can't know if it's correct or not -
because datasheets giving this information aren't publicly available.

So, I'm all in favour of "keep it damn simple, don't give people more
work, even if it looks nice in DT" here.

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

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Gatien CHEVALLIER]

On 9/18/25 15:59, Russell King (Oracle) wrote:
> On Thu, Sep 18, 2025 at 02:46:54PM +0200, Gatien CHEVALLIER wrote:
>> On 9/17/25 18:31, Russell King (Oracle) wrote:
>>> 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@foss.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.
>>
>> So how can we distinguish whether a PHY that implements WoL features
>> is actually able (wired) to wake up the system? By adding the
>> "wakeup-source" property to the PHY node?
> 
> Andrew's original idea was essentially that if the PHY reports that it
> supports WoL, then it's functional.
> 
> Sadly, that's not the case with many PHY drivers - the driver
> implementers just considered "does this PHY have the ability to detect
> WoL packets" and not "can this PHY actually wake the system."
> 
> Thankfully, all but one PHY driver does not use
> device_set_wakeup_capable() - my recent patches for realtek look like
> the first PHY driver to use this.
> 
> Thus, if we insist that PHY drivers use device_set_wakeup_capable()
> to indicate that (a) they have WoL capability _and_ are really
> capable of waking the system, we have a knob we can test for.
> 
> Sadly, there is no way to really know whether the interrupt that the
> PHY is attached to can wake the system. Things get worse with PHYs
> that don't use interrupts to wake the system. So, I would suggest
> that, as we already have this "wakeup-source" property available for
> _any_ device in DT, we start using this to say "on this system, this
> PHY is connected to something that can wake the system up."
> 

Sure, seems fair.

> See the past discussion when Realtek was being added - some of the
> context there covers what I mention above.
> 
>> Therefore, only set the "can wakeup" capability when both the PHY
>> supports WoL and the property is present in the PHY node?
> 
> Given that telling the device model that a device is wakeup
> capable causes this to be advertised to userspace, we really do
> not want devices saying that they are wakeup capable when they
> aren't capable of waking the system. So I would say that a call
> to device_set_wakeup_capable(dev, true) should _only_ be made if
> the driver is 100% certain that this device really can, without
> question, wake the platform.
> 
> If we don't have that guarantee, then we're on a hiding to nothing
> and chaos will reign, MAC drivers won't work properly... but I would
> suggest that's the price to be paid for shoddy implementation and
> not adhering to a sensible approach such as what I outline above.
> 
>> However, this does not solve the actual static pin function
>> configuration for pins that can, if correct alternate function is
>> selected, generate interrupts, in PHY drivers.
>>
>> It would be nice to be able to apply some kind of pinctrl to configure
>> the PHY pins over the MDIO bus thanks to some kind of pinctrl hogging.
>> This suggests modifying relevant PHY drivers and documentation to be
>> able to handle an optional pinctrl.
> 
> How would that work with something like the Realtek 8821F which has
> a single pin which can either signal interrupts (including a wake-up)
> or be in PME mode, where it only ever signals a wake-up event.
> Dynamically switching between the two modes is what got us into the
> crazy situation where, when WoL was enabled on this PHY, phylib
> stopped working because the pin was switched to PME mode, and we no
> longer got link status interrupts. So one could enable WoL, plug in
> an ethernet cable, and the kernel has no idea that the link has come
> up.
>  > So no. In a situation like this, either we want to be in interrupt
> mode (in which case we have an interrupt), or the pin is wired to
> a power management controller and needs to be in PME mode, or it isn't
> wired.
> 

If you are in interrupt mode, plugging a cable would trigger a
system wakeup in low-power mode if the INTB/PMEB line is wired to a
power management controller and the WoL is enabled because we're no
longer in polling mode, wouldn't it?
I tested on the stm32mp135f-dk and it seems that's the case.
Or in this case, maybe I should never describe an interrupt in the DT?

You can argue that as per the Realtek 8211F datasheet:
"The interrupts can be individually enabled or disabled by setting or
clearing bits in the interrupt enable register INER". That requires
PHY registers handling when going to low-power mode.

There are PHYs like the LAN8742 on which 3 pins can be configured
as nINT(equivalent to INTB), and 2 as nPME(equivalent to PMEB). The
smsc driver, as is, contains hardcoded nPME mode on the
LED2/nINT/nPME/nINTSEL pin. What if a manufacturer wired the power
management controller to the LED1/nINT/nPME/nINTSEL?
This is where the pinctrl would help even if I do agree it might be a
bit tedious at first. The pinctrl would be optional though.

On the other hand, if the pin is wired to a power management controller
and configured in PMEB mode, then an interrupt cannot be described in
the DT because the polling mode will be disabled and the PHY won't
generate an interrupt on, let's say, a cable being plugged => no
link as well (I had this issue on the stm32mp135f-dk board where
the LED2/nINT/nPME/nINTSEL is wired to the power management
controller and there is no other nINT-capable line wired).

So it seems there are still some issues but I may be missing
some elements here.

> Which it is can be easily identified.
> 
> $1. Is there an interrupt specified (Y/N) ?
> $2. Is there a wakeup-source property (Y/N) ?
> 
> States:
> $1  $2
> *   N   we have no idea if an interrupt (if specified) can wake the
>          system, or if there is other wiring from the PHY which might.
> 	Legacy driver, or has no wake-up support. We have to fall back
> 	on existing approaches to maintain compatibility and void
> 	breaking userspace, which may suggest WoL is supported when it
> 	isn't. For example, with stmmac, if STMMAC_FLAG_USE_PHY_WOL is
> 	set, we must assume the PHY can wake the system in this case.
> Y   Y   interrupt wakes the system, we're good for WoL
> N   Y   non-interrupt method of waking the system, e.g. PME
> 
> I'd prefer not to go for a complicated solution to this, e.g. involving
> pinctrl, when we don't know how many PHYs need this, because forcing
> extra complexity on driver authors means we have yet more to review, and
> I think it's fair to say that we're already missing stuff. Getting the
> pinctrl stuff right also requires knowledge of the hardware, and is
> likely something that reviewers can't know if it's correct or not -
> because datasheets giving this information aren't publicly available.
> 
> So, I'm all in favour of "keep it damn simple, don't give people more
> work, even if it looks nice in DT" here.
> 

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Russell King (Oracle)]

On Thu, Sep 18, 2025 at 05:07:00PM +0200, Gatien CHEVALLIER wrote:
> On 9/18/25 15:59, Russell King (Oracle) wrote:
> >  > So no. In a situation like this, either we want to be in interrupt
> > mode (in which case we have an interrupt), or the pin is wired to
> > a power management controller and needs to be in PME mode, or it isn't
> > wired.
> > 
> 
> If you are in interrupt mode, plugging a cable would trigger a
> system wakeup in low-power mode if the INTB/PMEB line is wired to a
> power management controller and the WoL is enabled because we're no
> longer in polling mode, wouldn't it?

What Andrew suggested, which is what I implemented for Realtek, other
interrupts get disabled when we enter suspend:

static int rtl8211f_suspend(struct phy_device *phydev)
{
...
        /* If a PME event is enabled, then configure the interrupt for
         * PME events only, disabling link interrupt. We avoid switching
         * to PMEB mode as we don't have a status bit for that.
         */
        if (device_may_wakeup(&phydev->mdio.dev)) {
                ret = phy_write_paged(phydev, 0xa42, RTL821x_INER,
                                      RTL8211F_INER_PME);

This disables all other interrupts when entering suspend _if_ WoL
is enabled and only if WoL is enabled.

If you're getting woken up when you unplug/replug the ethernet cable
when WoL is disabled, that suggests you have something wrong in your
interrupt controller - the wake-up state of the interrupt is managed
by core driver-model code. I tested this on nVidia Jetson Xavier NX
and if WoL wasn't enabled at the PHY, no wakeup occurred.

> You can argue that as per the Realtek 8211F datasheet:
> "The interrupts can be individually enabled or disabled by setting or
> clearing bits in the interrupt enable register INER". That requires
> PHY registers handling when going to low-power mode.

... which is what my patch does.

> There are PHYs like the LAN8742 on which 3 pins can be configured
> as nINT(equivalent to INTB), and 2 as nPME(equivalent to PMEB). The
> smsc driver, as is, contains hardcoded nPME mode on the
> LED2/nINT/nPME/nINTSEL pin. What if a manufacturer wired the power
> management controller to the LED1/nINT/nPME/nINTSEL?
> This is where the pinctrl would help even if I do agree it might be a
> bit tedious at first. The pinctrl would be optional though.

I'm not opposing the idea of pinctrl on PHYs. I'm opposing the idea
of tying it into the WoL code in a way that makes it mandatory.
Of course, if it makes sense for a PHY driver to do pinctrl stuff
then go ahead - and if from that, the driver can work out that
the PHY is wake-up capable, even better.

What I was trying to say is that in such a case as the Realtek
driver, I don't want to see pinctrl forced upon it unless there is
a real reason and benefit, especially when there are simpler ways
to do this.

I also think that it would be helpful to add the wakeup-source
property where PHYs are so capable even if the PHY driver doesn't
need it for two reasons. 1. OS independence. 2. it's useful docs.
3. just because our driver as it stands at whatever moment in time
doesn't make use of it doesn't mean that will always be the case.
(e.g., we may want to have e.g. phylib looking at that property.)

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

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Gatien CHEVALLIER]

On 9/18/25 17:36, Russell King (Oracle) wrote:
> On Thu, Sep 18, 2025 at 05:07:00PM +0200, Gatien CHEVALLIER wrote:
>> On 9/18/25 15:59, Russell King (Oracle) wrote:
>>>   > So no. In a situation like this, either we want to be in interrupt
>>> mode (in which case we have an interrupt), or the pin is wired to
>>> a power management controller and needs to be in PME mode, or it isn't
>>> wired.
>>>
>>
>> If you are in interrupt mode, plugging a cable would trigger a
>> system wakeup in low-power mode if the INTB/PMEB line is wired to a
>> power management controller and the WoL is enabled because we're no
>> longer in polling mode, wouldn't it?
> 
> What Andrew suggested, which is what I implemented for Realtek, other
> interrupts get disabled when we enter suspend:
> 
> static int rtl8211f_suspend(struct phy_device *phydev)
> {
> ...
>          /* If a PME event is enabled, then configure the interrupt for
>           * PME events only, disabling link interrupt. We avoid switching
>           * to PMEB mode as we don't have a status bit for that.
>           */
>          if (device_may_wakeup(&phydev->mdio.dev)) {
>                  ret = phy_write_paged(phydev, 0xa42, RTL821x_INER,
>                                        RTL8211F_INER_PME);
> 
> This disables all other interrupts when entering suspend _if_ WoL
> is enabled and only if WoL is enabled.
> 
> If you're getting woken up when you unplug/replug the ethernet cable
> when WoL is disabled, that suggests you have something wrong in your
> interrupt controller - the wake-up state of the interrupt is managed
> by core driver-model code. I tested this on nVidia Jetson Xavier NX
> and if WoL wasn't enabled at the PHY, no wakeup occurred.
> 

I'll replicate what you've done for the masking of interrupt when
going to low-power modes to the Microchip driver and see where it takes
me. The wakeup occurred because I didn't mask the other interrupt
sources in nINT mode... :)

>> You can argue that as per the Realtek 8211F datasheet:
>> "The interrupts can be individually enabled or disabled by setting or
>> clearing bits in the interrupt enable register INER". That requires
>> PHY registers handling when going to low-power mode.
> 
> ... which is what my patch does.
> 
>> There are PHYs like the LAN8742 on which 3 pins can be configured
>> as nINT(equivalent to INTB), and 2 as nPME(equivalent to PMEB). The
>> smsc driver, as is, contains hardcoded nPME mode on the
>> LED2/nINT/nPME/nINTSEL pin. What if a manufacturer wired the power
>> management controller to the LED1/nINT/nPME/nINTSEL?
>> This is where the pinctrl would help even if I do agree it might be a
>> bit tedious at first. The pinctrl would be optional though.
> 
> I'm not opposing the idea of pinctrl on PHYs. I'm opposing the idea
> of tying it into the WoL code in a way that makes it mandatory.
> Of course, if it makes sense for a PHY driver to do pinctrl stuff
> then go ahead - and if from that, the driver can work out that
> the PHY is wake-up capable, even better.
> 
> What I was trying to say is that in such a case as the Realtek
> driver, I don't want to see pinctrl forced upon it unless there is
> a real reason and benefit, especially when there are simpler ways
> to do this.
> 

Yes, sure, I think there's a proper use-case for it. If it's going
to happen, I think it would need a dedicated P-R. I'll send a V3 in
the near future addressing what we discussed here. Thank you for
the feedback.

> I also think that it would be helpful to add the wakeup-source
> property where PHYs are so capable even if the PHY driver doesn't
> need it for two reasons. 1. OS independence. 2. it's useful docs.
> 3. just because our driver as it stands at whatever moment in time
> doesn't make use of it doesn't mean that will always be the case.
> (e.g., we may want to have e.g. phylib looking at that property.)
> 

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Andrew Lunn]

> > 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.
> 
> So how can we distinguish whether a PHY that implements WoL features
> is actually able (wired) to wake up the system? By adding the
> "wakeup-source" property to the PHY node?
> 
> Therefore, only set the "can wakeup" capability when both the PHY
> supports WoL and the property is present in the PHY node?

There are layering issue to solve, and backwards compatibility
problems, but basically yes.

I would prefer to keep the phylib API simple. Call get_wol() and it
returns an empty set if the PHY is definitely not capable of waking
the system. Calling set_wol() returns -EOPNOTSUPP, or maybe -EINVAL,
if it definitely cannot wake the system. 

However, 'wakeup-source' on its own is not sufficient. It indicates
the PHY definitely can wake the system. However, it being missing does
not tell us it cannot wake the system, because old DT blobs never had
it, but i assume some work, and some are broken.

We need the PHY driver involved as well. If the driver only supports
WoL via interrupts, and phy_interrupt_is_valid() returns False, it
cannot wake the system.

There other tests we can make, like device_can_wakeup(). In the end,
we probably have some cases where we know it should work, some cases
we know it will not work, and a middle ground, shrug our shoulders, it
might work, try it and see.

> However, this does not solve the actual static pin function
> configuration for pins that can, if correct alternate function is
> selected, generate interrupts, in PHY drivers.
> 
> It would be nice to be able to apply some kind of pinctrl to configure
> the PHY pins over the MDIO bus thanks to some kind of pinctrl hogging.

I don't think it needs to be hogging. From what i remember of pinctrl,
when a driver is probed, pinctrl-0 is activated. It is not limited to
pins which the driver directly uses. So if LED2 is connected to a pin,
pinctrl can at least select the needed function for that pin.

	Andrew

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Gatien CHEVALLIER]

On 9/18/25 17:34, Andrew Lunn wrote:
>>> 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.
>>
>> So how can we distinguish whether a PHY that implements WoL features
>> is actually able (wired) to wake up the system? By adding the
>> "wakeup-source" property to the PHY node?
>>
>> Therefore, only set the "can wakeup" capability when both the PHY
>> supports WoL and the property is present in the PHY node?
> 
> There are layering issue to solve, and backwards compatibility
> problems, but basically yes.
> 

Ack

> I would prefer to keep the phylib API simple. Call get_wol() and it
> returns an empty set if the PHY is definitely not capable of waking
> the system. Calling set_wol() returns -EOPNOTSUPP, or maybe -EINVAL,
> if it definitely cannot wake the system.
> 
> However, 'wakeup-source' on its own is not sufficient. It indicates
> the PHY definitely can wake the system. However, it being missing does
> not tell us it cannot wake the system, because old DT blobs never had
> it, but i assume some work, and some are broken.
> 
> We need the PHY driver involved as well. If the driver only supports
> WoL via interrupts, and phy_interrupt_is_valid() returns False, it
> cannot wake the system.
> 
> There other tests we can make, like device_can_wakeup(). In the end,
> we probably have some cases where we know it should work, some cases
> we know it will not work, and a middle ground, shrug our shoulders, it
> might work, try it and see.
> 
>> However, this does not solve the actual static pin function
>> configuration for pins that can, if correct alternate function is
>> selected, generate interrupts, in PHY drivers.
>>
>> It would be nice to be able to apply some kind of pinctrl to configure
>> the PHY pins over the MDIO bus thanks to some kind of pinctrl hogging.
> 
> I don't think it needs to be hogging. From what i remember of pinctrl,
> when a driver is probed, pinctrl-0 is activated. It is not limited to
> pins which the driver directly uses. So if LED2 is connected to a pin,
> pinctrl can at least select the needed function for that pin.
> 

Ok, I'll see where it takes me in a separate patch-set.
Thank you for the feedback and clarifications.

Gatien

> 	Andrew

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Russell King (Oracle)]

[-- Attachment #1: Type: text/plain, Size: 5087 bytes --]

On Wed, Sep 17, 2025 at 05:31:16PM +0100, Russell King (Oracle) wrote:
> 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@foss.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.

I came up with this as an experiment - I haven't tested it beyond
running it through the compiler (didn't let it get to the link stage
yet.) Haven't even done anything with it for stmmac yet.

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

[-- Attachment #2: 0001-net-phy-add-phy_can_wakeup.patch --]
[-- Type: text/x-diff, Size: 1199 bytes --]

From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
Subject: [PATCH net-next 1/2] 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.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 include/linux/phy.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index b377dfaa6801..7f6758198948 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -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);
 
-- 
2.47.3


[-- Attachment #3: 0002-net-phylink-add-Wake-on-Lan-MAC-support.patch --]
[-- Type: text/x-diff, Size: 5411 bytes --]

From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
Subject: [PATCH net-next 2/2] net: phylink: add Wake-on-Lan MAC support

Add core phylink Wake-on-Lan support.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 66 ++++++++++++++++++++++++++++++++++++---
 include/linux/phylink.h   | 20 ++++++++++++
 2 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 9d7799ea1c17..e857a147f76b 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, ...) \
@@ -2673,6 +2676,17 @@ void phylink_resume(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_resume);
 
+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_ethtool_get_wol() - get the wake on lan parameters for the PHY
  * @pl: a pointer to a &struct phylink returned from phylink_create()
@@ -2689,8 +2703,21 @@ 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.
+		 */
+		wol->supported |= pl->config->wol_mac_support;
+		wol->wolopts |= pl->wolopts_mac;
+		memcpy(wol->sopass, pl->wol_sopass, sizeof(wol->sopass));
+	} else {
+		/* Legacy */
+		if (pl->phydev)
+			phy_ethtool_get_wol(pl->phydev, wol);
+	}
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_wol);
 
@@ -2707,12 +2734,43 @@ 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_MAGIC)
+			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);
+	} 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..26ff099c32cb 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,
+			   u8 *sopass);
 };
 
 #if 0 /* For kernel-doc purposes only. */
@@ -440,6 +450,16 @@ 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
+ *
+ * Returns: 0 on success.
+ */
+int (*mac_wol_set)(struct phylink_config *config, u32 wolopts, u8 *sopass);
 #endif
 
 struct phylink_pcs_ops;
-- 
2.47.3

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Florian Fainelli]

On 9/26/2025 10:59 AM, Russell King (Oracle) wrote:
> On Wed, Sep 17, 2025 at 05:31:16PM +0100, Russell King (Oracle) wrote:
>> 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@foss.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.
> 
> I came up with this as an experiment - I haven't tested it beyond
> running it through the compiler (didn't let it get to the link stage
> yet.) Haven't even done anything with it for stmmac yet.
> 

I like the direction this is going, we could probably take one step 
further and extract the logic present in bcmgenet_wol.c and make those 
helper functions for other drivers to get the overlay of PHY+MAC WoL 
options/password consistent across all drivers. What do you think?
-- 
Florian

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Florian Fainelli]

On 9/26/2025 12:05 PM, Florian Fainelli wrote:
> 
> 
> On 9/26/2025 10:59 AM, Russell King (Oracle) wrote:
>> On Wed, Sep 17, 2025 at 05:31:16PM +0100, Russell King (Oracle) wrote:
>>> 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@foss.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.
>>
>> I came up with this as an experiment - I haven't tested it beyond
>> running it through the compiler (didn't let it get to the link stage
>> yet.) Haven't even done anything with it for stmmac yet.
>>
> 
> I like the direction this is going, we could probably take one step 
> further and extract the logic present in bcmgenet_wol.c and make those 
> helper functions for other drivers to get the overlay of PHY+MAC WoL 
> options/password consistent across all drivers. What do you think?

+		if (wolopts & WAKE_MAGIC)
+			changed |= !!memcmp(wol->sopass, pl->wol_sopass,
+					    sizeof(wol->sopass));


Should not the hunk above be wolopts & WAKE_MAGICSECURE?

-- 
Florian

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Russell King (Oracle)]

On Sat, Sep 27, 2025 at 02:04:15PM -0700, Florian Fainelli wrote:
> 
> 
> On 9/26/2025 12:05 PM, Florian Fainelli wrote:
> > 
> > 
> > On 9/26/2025 10:59 AM, Russell King (Oracle) wrote:
> > > On Wed, Sep 17, 2025 at 05:31:16PM +0100, Russell King (Oracle) wrote:
> > > > 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@foss.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.
> > > 
> > > I came up with this as an experiment - I haven't tested it beyond
> > > running it through the compiler (didn't let it get to the link stage
> > > yet.) Haven't even done anything with it for stmmac yet.
> > > 
> > 
> > I like the direction this is going, we could probably take one step
> > further and extract the logic present in bcmgenet_wol.c and make those
> > helper functions for other drivers to get the overlay of PHY+MAC WoL
> > options/password consistent across all drivers. What do you think?
> 
> +		if (wolopts & WAKE_MAGIC)
> +			changed |= !!memcmp(wol->sopass, pl->wol_sopass,
> +					    sizeof(wol->sopass));
> 
> 
> Should not the hunk above be wolopts & WAKE_MAGICSECURE?

Yes, and there's a few other bits missing as well. The series has
progressed, and stmmac is converted and tested on my Jetson platform.

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

Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Russell King (Oracle)]

On Fri, Sep 26, 2025 at 12:05:19PM -0700, Florian Fainelli wrote:
> I like the direction this is going, we could probably take one step further
> and extract the logic present in bcmgenet_wol.c and make those helper
> functions for other drivers to get the overlay of PHY+MAC WoL
> options/password consistent across all drivers. What do you think?

The logic I've implemented is fairly similar, but with one difference:
I'm always storing the sopass, which means in the wol_get method I
don't have to be concerned with the sopass returned by the PHY.
This should be fine, unless the PHY was already configured for WoL
magicsecure, and in that case we'll return a zero SOPASS but indicating
WAKE_MAGICSECURE which probably isn't great.

So, my new get_wol logic is:

        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;

with:

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));
}

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

This is to cope with PHYs that respond to phy_ethtool_get_wol()
reporting that they support WoL but have no capability to actually wake
the system up. MACs can choose whether to override that by setting
phylink_config->wol_phy_legacy.

Much like taking this logic away from MAC driver authors, I think we
need to take the logic around "can this PHY actually wake-up the
system" away from the PHY driver author. I believe every driver that
supports WoL with the exception of realtek and broadcom.c reports that
WoL is supported and accepts set_wol() even when they're not capable
of waking the system. e.g. bcm_phy_get_wol():

        wol->supported = BCM54XX_WOL_SUPPORTED_MASK;
        wol->wolopts = 0;

with no prior checks. This is why the "phylink_phy_supports_wol()"
logic above is necessary, otherwise implementing this "use either
the PHY or MAC" logic will break stuff.

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

[PATCH net-next v2 3/4] net: phy: smsc: fix and improve WoL support

[Gatien Chevallier]

Add suspend()/resume() callbacks that do not shut down the PHY if the
WoL is supported and handle the WoL status flags.

If the WoL is supported by the PHY, indicate that the PHY device can
be a source of wake up for the platform. When setting the WoL
configuration, enable this capability.

The suspend() callback now handle WoL event flags that would prevent
a system to wake up from a WoL event when in low-power mode. Because
the WoL prevents a call to the suspend() callback, add the
PHY_ALWAYS_CALL_SUSPEND flag to the LAN8742 PHYs.

Fixes: 8b305ee2a91c ("net: phy: smsc: add WoL support to LAN8740/LAN8742 PHYs")
Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 drivers/net/phy/smsc.c  | 66 +++++++++++++++++++++++++++++++++++++++++++++----
 include/linux/smscphy.h |  2 ++
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 48487149c22528631be5aca98ff4980f55b495d9..818cb21b833a530c7fa2384f605bbb2e93c5eb5f 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -537,14 +537,67 @@ static int lan874x_set_wol(struct phy_device *phydev,
 		}
 	}
 
-	rc = phy_write_mmd(phydev, MDIO_MMD_PCS, MII_LAN874X_PHY_MMD_WOL_WUCSR,
-			   val_wucsr);
+	/* Enable wakeup on PHY device if at least one WoL feature is configured */
+	device_set_wakeup_enable(&phydev->mdio.dev, !!(val_wucsr & MII_LAN874X_PHY_WOL_MASK));
+
+	rc = phy_write_mmd(phydev, MDIO_MMD_PCS, MII_LAN874X_PHY_MMD_WOL_WUCSR, val_wucsr);
 	if (rc < 0)
 		return rc;
 
 	return 0;
 }
 
+static int smsc_phy_suspend(struct phy_device *phydev)
+{
+	int rc;
+
+	if (!device_may_wakeup(&phydev->mdio.dev))
+		return 0;
+
+	if (!phydev->wol_enabled)
+		return genphy_suspend(phydev);
+
+	/* Handle pending WoL events */
+	rc = phy_read_mmd(phydev, MDIO_MMD_PCS, MII_LAN874X_PHY_MMD_WOL_WUCSR);
+	if (rc < 0) {
+		phy_error(phydev);
+		return -EINVAL;
+	}
+
+	if (!(rc & MII_LAN874X_PHY_WOL_STATUS_MASK))
+		return 0;
+
+	rc = phy_write_mmd(phydev, MDIO_MMD_PCS, MII_LAN874X_PHY_MMD_WOL_WUCSR,
+			   rc | MII_LAN874X_PHY_WOL_STATUS_MASK);
+	if (rc < 0) {
+		phy_error(phydev);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int smsc_phy_resume(struct phy_device *phydev)
+{
+	int rc;
+
+	if (!phydev->wol_enabled)
+		return genphy_resume(phydev);
+
+	rc = phy_read_mmd(phydev, MDIO_MMD_PCS, MII_LAN874X_PHY_MMD_WOL_WUCSR);
+	if (rc < 0)
+		return rc;
+
+	if (!(rc & MII_LAN874X_PHY_WOL_STATUS_MASK))
+		return 0;
+
+	phydev_dbg(phydev, "Woke up from LAN event.\n");
+	rc = phy_write_mmd(phydev, MDIO_MMD_PCS, MII_LAN874X_PHY_MMD_WOL_WUCSR,
+			   rc | MII_LAN874X_PHY_WOL_STATUS_MASK);
+
+	return rc;
+}
+
 static int smsc_get_sset_count(struct phy_device *phydev)
 {
 	return ARRAY_SIZE(smsc_hw_stats);
@@ -673,6 +726,9 @@ int smsc_phy_probe(struct phy_device *phydev)
 
 	phydev->priv = priv;
 
+	if (phydev->drv->set_wol)
+		device_set_wakeup_capable(&phydev->mdio.dev, true);
+
 	/* Make clk optional to keep DTB backward compatibility. */
 	refclk = devm_clk_get_optional_enabled_with_rate(dev, NULL,
 							 50 * 1000 * 1000);
@@ -851,7 +907,7 @@ static struct phy_driver smsc_phy_driver[] = {
 	.name		= "Microchip LAN8742",
 
 	/* PHY_BASIC_FEATURES */
-	.flags		= PHY_RST_AFTER_CLK_EN,
+	.flags		= PHY_RST_AFTER_CLK_EN | PHY_ALWAYS_CALL_SUSPEND,
 
 	.probe		= smsc_phy_probe,
 
@@ -876,8 +932,8 @@ static struct phy_driver smsc_phy_driver[] = {
 	.set_wol	= lan874x_set_wol,
 	.get_wol	= lan874x_get_wol,
 
-	.suspend	= genphy_suspend,
-	.resume		= genphy_resume,
+	.suspend	= smsc_phy_suspend,
+	.resume		= smsc_phy_resume,
 } };
 
 module_phy_driver(smsc_phy_driver);
diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
index 1a6a851d2cf80d225bada7adeb79969e625964bd..cdf266960032609241afc8316da23f1c4834bfee 100644
--- a/include/linux/smscphy.h
+++ b/include/linux/smscphy.h
@@ -65,6 +65,8 @@ int smsc_phy_probe(struct phy_device *phydev);
 #define MII_LAN874X_PHY_WOL_WUEN		BIT(2)
 #define MII_LAN874X_PHY_WOL_MPEN		BIT(1)
 #define MII_LAN874X_PHY_WOL_BCSTEN		BIT(0)
+#define MII_LAN874X_PHY_WOL_MASK		GENMASK(4, 0)
+#define MII_LAN874X_PHY_WOL_STATUS_MASK		GENMASK(7, 4)
 
 #define MII_LAN874X_PHY_WOL_FILTER_EN		BIT(15)
 #define MII_LAN874X_PHY_WOL_FILTER_MCASTTEN	BIT(9)

-- 
2.35.3

Re: [PATCH net-next v2 3/4] net: phy: smsc: fix and improve WoL support

[Russell King (Oracle)]

On Wed, Sep 17, 2025 at 05:36:38PM +0200, Gatien Chevallier wrote:
> @@ -673,6 +726,9 @@ int smsc_phy_probe(struct phy_device *phydev)
>  
>  	phydev->priv = priv;
>  
> +	if (phydev->drv->set_wol)
> +		device_set_wakeup_capable(&phydev->mdio.dev, true);

This suggests that you know that this device is _always_ capable of
waking the system up, irrespective of the properties of e.g. the
interrupt controller. If this is not the case, please consider the
approach I took in the realtek driver.

In that case, you also need to add checks in the get_wol() and
set_wol() methods to refuse WoL configuration, and to report that
WoL is not supported.

Thanks.

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

[PATCH net-next v2 4/4] arm: dts: st: activate ETH1 WoL from PHY on stm32mp135f-dk

[Gatien Chevallier]

On this board, the ETH1 supports WoL from PHY. Add the "st,phy-wol"
property to support it.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp135f-dk.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/st/stm32mp135f-dk.dts b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
index 9764a6bfa5b428c8524a5902c10b7807dda46b3d..d746424b039013759bfbcce5193a701ff775e715 100644
--- a/arch/arm/boot/dts/st/stm32mp135f-dk.dts
+++ b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
@@ -193,6 +193,7 @@ &ethernet1 {
 	pinctrl-names = "default", "sleep";
 	phy-mode = "rmii";
 	phy-handle = <&phy0_eth1>;
+	st,phy-wol;
 
 	mdio {
 		#address-cells = <1>;

-- 
2.35.3