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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aMwnCWT5JFY4jstm@shell.armlinux.org.uk>
Date: Thu, 18 Sep 2025 16:36:41 +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 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!