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

[Russell King (Oracle)]

On Tue, Jul 22, 2025 at 03:40:16PM +0200, Andrew Lunn wrote:
> I know Russell has also replied about issues with stmmac. Please
> consider that when reading what i say... It might be not applicable.
> 
> > Seems like a fair and logical approach. It seems reasonable that the
> > MAC driver relies on the get_wol() API to know what's supported.
> > 
> > The tricky thing for the PHY used in this patchset is to get this
> > information:
> > 
> > Extract from the documentation of the LAN8742A PHY:
> > "The WoL detection can be configured to assert the nINT interrupt pin
> > or nPME pin"
> 
> https://www.kernel.org/doc/Documentation/devicetree/bindings/power/wakeup-source.txt
> 
> It is a bit messy, but in the device tree, you could have:
> 
>     interrupts = <&sirq 0 IRQ_TYPE_LEVEL_LOW>
>                  <&pmic 42 IRQ_TYPE_LEVEL_LOW>;
>     interrupt-names = "nINT", "wake";
>     wakeup-source
> 
> You could also have:
> 
>     interrupts = <&sirq 0 IRQ_TYPE_LEVEL_LOW>;
>     interrupt-names = "wake";
>     wakeup-source
> 
> In the first example, since there are two interrupts listed, it must
> be using the nPME. For the second, since there is only one, it must be
> using nINT.
> 
> Where this does not work so well is when you have a board which does
> not have nINT wired, but does have nPME. The phylib core will see
> there is an interrupt and request it, and disable polling. And then
> nothing will work. We might be able to delay solving that until such a
> board actually exists?

(Officially, I'm still on vacation...)

At this point, I'd like to kick off a discussion about PHY-based
wakeup that is relevant to this thread.

The kernel has device-based wakeup support. We have:

- device_set_wakeup_capable(dev, flag) - indicates that the is
  capable of waking the system depending on the flag.

- device_set_wakeup_enable(dev, flag) - indicates whether "dev"
  has had wake-up enabled or disabled depending on the flag.

- dev*_pm_set_wake_irq(dev, irq) - indicates to the wake core that
  the indicated IRQ is capable of waking the system, and the core
  will handle enabling/disabling irq wake capabilities on the IRQ
  as appropriate (dependent on device_set_wakeup_enable()). Other
  functions are available for wakeup IRQs that are dedicated to
  only waking up the system (e.g. the WOL_INT pin on AR8031).

Issue 1. In stmmac_init_phy(), we have this code:

        if (!priv->plat->pmt) {
                struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };

                phylink_ethtool_get_wol(priv->phylink, &wol);
                device_set_wakeup_capable(priv->device, !!wol.supported);
                device_set_wakeup_enable(priv->device, !!wol.wolopts);
        }

This reads the WoL state from the PHY (a different struct device)
and sets the wakeup capability and enable state for the _stmmac_
device accordingly, but in the case of PHY based WoL, it's the PHY
doing the wakeup, not the MAC. So this seems wrong on the face of
it.

Issue 2. no driver in phylib, nor the core, ever uses any of the
device_set_wakeup_*() functions. As PHYs on their own are capable
of WoL, isn't this an oversight? Shouldn't phylib be supporting
this rather than leaving it to MAC drivers to figure something out?

Issue 3. should pins like WOL_INT or nPME be represented as an
interrupt, and dev_pm_set_dedicated_wake_irq() used to manage that
interrupt signal if listed as an IRQ in the PHY's DT description?

(Side note: I have tried WoL on the Jetson Xavier NX board I have
which uses stmmac-based WoL, but it seems non-functional. I've
dropped a private email to Jon and Thierry to see whether this is
expected or something that needs fixing. I'm intending to convert
stmmac to use core wakeirq support, rather than managing
the enable_irq_wake()/disable_irq_wake() by itself.)

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