From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Gatien CHEVALLIER <gatien.chevallier@foss.st.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
Andrew Lunn <andrew+netdev@lunn.ch>,
Eric Dumazet <edumazet@google.com>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
linux-arm-kernel@lists.infradead.org,
Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [Linux-stm32] [PATCH RFC net-next 6/7] net: stmmac: add helpers to indicate WoL enable status
Date: Tue, 29 Jul 2025 16:31:20 +0100 [thread overview]
Message-ID: <aIjpSEz4jZz12c2Q@shell.armlinux.org.uk> (raw)
In-Reply-To: <aIiQ6qnj1M2qTudc@shell.armlinux.org.uk>
On Tue, Jul 29, 2025 at 10:14:18AM +0100, Russell King (Oracle) wrote:
> On Tue, Jul 29, 2025 at 10:03:22AM +0100, Russell King (Oracle) wrote:
> > Without Thierry's .dts patch, as I predicted, enabling WoL at the
> > PHY results in Bad Stuff happening - the code in the realtek driver
> > for WoL is quite simply broken and wrong.
> >
> > Switching the pin from INTB mode to PMEB mode results in:
> > - No link change interrupts once WoL is enabled
> > - The interrupt output being stuck at active level, causing an
> > interrupt storm and the interrupt is eventually disabled.
> > The PHY can be configured to pulse the PMEB or hold at an active
> > level until the WoL is cleared - and by default it's the latter.
> >
> > So, switching the interrupt pin to PMEB mode is simply wrong and
> > breaks phylib. I guess the original WoL support was only tested on
> > a system which didn't use the PHY interrupt, only using the interrupt
> > pin for wake-up purposes.
>
> I will also state that the only way the WoL support for Realtek that
> was merged can possibly work is if there is some other agent in the
> system which configures the PHY such that PMEB only pulses on WoL
> packet reception. Unless this is the case, the PMEB pin will be
> pulled active on the first matching WoL packet, and remain there.
> That means WoL will work for the first attempt and not after.
>
> This makes the WoL support that was merged completely broken for the
> general case where there isn't some other agent configuring the PHY
> e.g. at boot time.
>
> I am in two minds whether it should be reverted until it can be
> correctly implemented. It's a relatively recent addition to the
> kernel - the patch is dated 29th April 2025. See
> https://patch.msgid.link/20250429-realtek_wol-v2-1-8f84def1ef2c@kuka.com
Oh, and it gets better...
/* Disable magic packet matching and reset WOL status */
rtl821x_write_page(dev, RTL8211F_WOL_SETTINGS_PAGE);
__phy_write(dev, RTL8211F_WOL_SETTINGS_EVENTS, 0);
__phy_write(dev, RTL8211F_WOL_SETTINGS_STATUS, RTL8211F_WOL_STATUS_RESET);
where RTL8211F_WOL_STATUS_RESET is defined as:
#define RTL8211F_WOL_STATUS_RESET (BIT(15) | 0x1fff)
Now, this does nothing of the sort. Yes, bit 15 is the WoL reset bit,
but if one bothers to read the application note, one discovers that
bit 15 is _active_ _low_ !
This bit is required to be written as zero to reset the PMEB output
to inactive state. No where in the driver is this done.
Ergo, the addition of WoL support for RTL8211F, basically, is totally
and utterly broken, even if pin 31 is used solely for PMEB
functionality.
Therefore, the only conclusion at this point is that the patch adding
WoL support was likely hardly functionally tested, if at all. Given
everything I've stated about the current code, I think it's safe to
ignore the "functionality" provided by existing code, and not worry
about breaking anyone's setup: it's already completely broken.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2025-07-29 15:31 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-28 15:45 [PATCH RFC net-next 0/7] net: stmmac: EEE and WoL cleanups Russell King (Oracle)
2025-07-28 15:45 ` [PATCH RFC net-next 1/7] net: stmmac: remove unnecessary checks in ethtool eee ops Russell King (Oracle)
2025-07-28 17:03 ` Andrew Lunn
2025-07-28 15:45 ` [PATCH RFC net-next 2/7] net: stmmac: remove write-only mac->pmt Russell King (Oracle)
2025-07-28 17:04 ` Andrew Lunn
2025-07-28 15:45 ` [PATCH RFC net-next 3/7] net: stmmac: remove redundant WoL option validation Russell King (Oracle)
2025-07-28 17:06 ` Andrew Lunn
2025-07-28 15:45 ` [PATCH RFC net-next 4/7] net: stmmac: remove unnecessary "stmmac: wakeup enable" print Russell King (Oracle)
2025-07-28 17:06 ` Andrew Lunn
2025-07-28 15:45 ` [PATCH RFC net-next 5/7] net: stmmac: use core wake IRQ support Russell King (Oracle)
2025-07-28 17:12 ` Andrew Lunn
2025-07-28 15:45 ` [PATCH RFC net-next 6/7] net: stmmac: add helpers to indicate WoL enable status Russell King (Oracle)
2025-07-28 17:28 ` Andrew Lunn
2025-07-28 17:54 ` Russell King (Oracle)
2025-07-29 8:43 ` [Linux-stm32] " Gatien CHEVALLIER
2025-07-29 9:03 ` Russell King (Oracle)
2025-07-29 9:14 ` Russell King (Oracle)
2025-07-29 15:31 ` Russell King (Oracle) [this message]
2025-07-29 12:45 ` Russell King (Oracle)
2025-07-29 13:10 ` Gatien CHEVALLIER
2025-07-29 14:44 ` Russell King (Oracle)
2025-07-29 15:34 ` Gatien CHEVALLIER
2025-07-29 16:35 ` Russell King (Oracle)
2025-07-29 17:27 ` Andrew Lunn
2025-07-29 18:19 ` Russell King (Oracle)
2025-07-29 22:01 ` Florian Fainelli
2025-07-28 15:46 ` [PATCH RFC net-next 7/7] net: stmmac: explain the phylink_speed_down() call in stmmac_release() Russell King (Oracle)
2025-07-28 17:19 ` Andrew Lunn
2025-07-28 17:29 ` Andrew Lunn
2025-07-29 8:47 ` Russell King (Oracle)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aIjpSEz4jZz12c2Q@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew+netdev@lunn.ch \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gatien.chevallier@foss.st.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).