[PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Russell King (Oracle)]

Implement Wake-on-Lan correctly. The existing implementation has
multiple issues:

1. It assumes that Wake-on-Lan can always be used, whether or not the
   interrupt is wired, and whether or not the interrupt is capable of
   waking the system. This breaks the ability for MAC drivers to detect
   whether the PHY WoL is functional.
2. switching the interrupt pin in the .set_wol() method to PMEB mode
   immediately silences link-state interrupts, which breaks phylib
   when interrupts are being used rather than polling mode.
3. the code claiming to "reset WOL status" was doing nothing of the
   sort. Bit 15 in page 0xd8a register 17 controls WoL reset, and
   needs to be pulsed low to reset the WoL state. This bit was always
   written as '1', resulting in no reset.
4. not resetting WoL state results in the PMEB pin remaining asserted,
   which in turn leads to an interrupt storm. Only resetting the WoL
   state in .set_wol() is not sufficient.
5. PMEB mode does not allow software detection of the wake-up event as
   there is no status bit to indicate we received the WoL packet.
6. across reboots of at least the Jetson Xavier NX system, the WoL
   configuration is preserved.

Fix all of these issues by essentially rewriting the support. We:
1. clear the WoL event enable register at probe time.
2. detect whether we can support wake-up by having a valid interrupt,
   and the "wakeup-source" property in DT. If we can, then we mark
   the MDIO device as wakeup capable, and associate the interrupt
   with the wakeup source.
3. arrange for the get_wol() and set_wol() implementations to handle
   the case where the MDIO device has not been marked as wakeup
   capable (thereby returning no WoL support, and refusing to enable
   WoL support.)
4. avoid switching to PMEB mode, instead using INTB mode with the
   interrupt enable, reconfiguring the interrupt enables at suspend
   time, and restoring their original state at resume time (we track
   the state of the interrupt enable register in .config_intr()
   register.)
5. move WoL reset from .set_wol() to the suspend function to ensure
   that WoL state is cleared prior to suspend. This is necessary
   after the PME interrupt has been enabled as a second WoL packet
   will not re-raise a previously cleared PME interrupt.
6. when a PME interrupt (for wakeup) is asserted, pass this to the
   PM wakeup so it knows which device woke the system.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
I've sort-of tested this on the Jetson Xavier NX platform, but it's
been difficult because it appears that the whole interrupt/wakeup
stuff for the SoC is foobar in mainline. One gets the choice of
specifying the GPIO interrupt in DT and have working normal interrupt
or the power management controller interrupt for the same line and
having wakeup functional. You can't have both together.

I'm not sure whether this change should target the net or net-next
tree; what we have currently in 6.16 is totally and utterly broken,
so arguably this is a fix - but it's not a regression because 6.16
is the first kernel that WoL "support" for RTL8211F is in. This is
also a large change.

However, I can't see that it was tested, given all the problems
identified above. As a result, I've taken the decision in this patch
to not worry about breaking anyone's existing setup.

So, I have no problem with requiring "wakeup-source" to be added to DT
for rtl8211f PHYs that are to support wake-up, meaning that they are
properly wired to support WoL. We can't tell just from having an
interrupt - not all interrupts on all devices may be wake-up capable.

 drivers/net/phy/realtek/realtek_main.c | 174 ++++++++++++++++++++-----
 1 file changed, 142 insertions(+), 32 deletions(-)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index dd0d675149ad..520fdeb88c32 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -10,6 +10,7 @@
 #include <linux/bitops.h>
 #include <linux/of.h>
 #include <linux/phy.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/netdevice.h>
 #include <linux/module.h>
 #include <linux/delay.h>
@@ -31,6 +32,7 @@
 #define RTL821x_INER				0x12
 #define RTL8211B_INER_INIT			0x6400
 #define RTL8211E_INER_LINK_STATUS		BIT(10)
+#define RTL8211F_INER_PME			BIT(7)
 #define RTL8211F_INER_LINK_STATUS		BIT(4)
 
 #define RTL821x_INSR				0x13
@@ -96,17 +98,13 @@
 #define RTL8211F_RXCR				0x15
 #define RTL8211F_RX_DELAY			BIT(3)
 
-/* RTL8211F WOL interrupt configuration */
-#define RTL8211F_INTBCR_PAGE			0xd40
-#define RTL8211F_INTBCR				0x16
-#define RTL8211F_INTBCR_INTB_PMEB		BIT(5)
-
 /* RTL8211F WOL settings */
-#define RTL8211F_WOL_SETTINGS_PAGE		0xd8a
+#define RTL8211F_WOL_PAGE		0xd8a
 #define RTL8211F_WOL_SETTINGS_EVENTS		16
 #define RTL8211F_WOL_EVENT_MAGIC		BIT(12)
-#define RTL8211F_WOL_SETTINGS_STATUS		17
-#define RTL8211F_WOL_STATUS_RESET		(BIT(15) | 0x1fff)
+#define RTL8211F_WOL_RST_RMSQ		17
+#define RTL8211F_WOL_RG_RSTB			BIT(15)
+#define RTL8211F_WOL_RMSQ			0x1fff
 
 /* RTL8211F Unique phyiscal and multicast address (WOL) */
 #define RTL8211F_PHYSICAL_ADDR_PAGE		0xd8c
@@ -172,7 +170,8 @@ struct rtl821x_priv {
 	u16 phycr2;
 	bool has_phycr2;
 	struct clk *clk;
-	u32 saved_wolopts;
+	/* rtl8211f */
+	u16 iner;
 };
 
 static int rtl821x_read_page(struct phy_device *phydev)
@@ -255,6 +254,35 @@ static int rtl821x_probe(struct phy_device *phydev)
 	return 0;
 }
 
+static int rtl8211f_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	int ret;
+
+	ret = rtl821x_probe(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* Disable all PME events */
+	ret = phy_write_paged(phydev, RTL8211F_WOL_PAGE,
+			      RTL8211F_WOL_SETTINGS_EVENTS, 0);
+	if (ret < 0)
+		return ret;
+
+	/* Mark this PHY as wakeup capable and register the interrupt as a
+	 * wakeup IRQ if the PHY is marked as a wakeup source in firmware,
+	 * and the interrupt is valid. Save the INTB/PMEB pin coniguration,
+	 * so it can be restored when resuming.
+	 */
+	if (device_property_read_bool(dev, "wakeup-source") &&
+	    phy_interrupt_is_valid(phydev)) {
+		device_set_wakeup_capable(dev, true);
+		devm_pm_set_wake_irq(dev, phydev->irq);
+	}
+
+	return ret;
+}
+
 static int rtl8201_ack_interrupt(struct phy_device *phydev)
 {
 	int err;
@@ -352,6 +380,7 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
 
 static int rtl8211f_config_intr(struct phy_device *phydev)
 {
+	struct rtl821x_priv *priv = phydev->priv;
 	u16 val;
 	int err;
 
@@ -362,8 +391,10 @@ static int rtl8211f_config_intr(struct phy_device *phydev)
 
 		val = RTL8211F_INER_LINK_STATUS;
 		err = phy_write_paged(phydev, 0xa42, RTL821x_INER, val);
+		if (err == 0)
+			priv->iner = val;
 	} else {
-		val = 0;
+		priv->iner = val = 0;
 		err = phy_write_paged(phydev, 0xa42, RTL821x_INER, val);
 		if (err)
 			return err;
@@ -426,21 +457,34 @@ static irqreturn_t rtl8211f_handle_interrupt(struct phy_device *phydev)
 		return IRQ_NONE;
 	}
 
-	if (!(irq_status & RTL8211F_INER_LINK_STATUS))
-		return IRQ_NONE;
+	if (irq_status & RTL8211F_INER_LINK_STATUS) {
+		phy_trigger_machine(phydev);
+		return IRQ_HANDLED;
+	}
 
-	phy_trigger_machine(phydev);
+	if (irq_status & RTL8211F_INER_PME) {
+		pm_wakeup_event(&phydev->mdio.dev, 0);
+		return IRQ_HANDLED;
+	}
 
-	return IRQ_HANDLED;
+	return IRQ_NONE;
 }
 
 static void rtl8211f_get_wol(struct phy_device *dev, struct ethtool_wolinfo *wol)
 {
 	int wol_events;
 
+	/* If the PHY is not capable of waking the system, then WoL can not
+	 * be supported.
+	 */
+	if (!device_can_wakeup(&dev->mdio.dev)) {
+		wol->supported = 0;
+		return;
+	}
+
 	wol->supported = WAKE_MAGIC;
 
-	wol_events = phy_read_paged(dev, RTL8211F_WOL_SETTINGS_PAGE, RTL8211F_WOL_SETTINGS_EVENTS);
+	wol_events = phy_read_paged(dev, RTL8211F_WOL_PAGE, RTL8211F_WOL_SETTINGS_EVENTS);
 	if (wol_events < 0)
 		return;
 
@@ -453,6 +497,9 @@ static int rtl8211f_set_wol(struct phy_device *dev, struct ethtool_wolinfo *wol)
 	const u8 *mac_addr = dev->attached_dev->dev_addr;
 	int oldpage;
 
+	if (!device_can_wakeup(&dev->mdio.dev))
+		return -EOPNOTSUPP;
+
 	oldpage = phy_save_page(dev);
 	if (oldpage < 0)
 		goto err;
@@ -464,25 +511,23 @@ static int rtl8211f_set_wol(struct phy_device *dev, struct ethtool_wolinfo *wol)
 		__phy_write(dev, RTL8211F_PHYSICAL_ADDR_WORD1, mac_addr[3] << 8 | (mac_addr[2]));
 		__phy_write(dev, RTL8211F_PHYSICAL_ADDR_WORD2, mac_addr[5] << 8 | (mac_addr[4]));
 
-		/* Enable magic packet matching and reset WOL status */
-		rtl821x_write_page(dev, RTL8211F_WOL_SETTINGS_PAGE);
+		/* Enable magic packet matching */
+		rtl821x_write_page(dev, RTL8211F_WOL_PAGE);
 		__phy_write(dev, RTL8211F_WOL_SETTINGS_EVENTS, RTL8211F_WOL_EVENT_MAGIC);
-		__phy_write(dev, RTL8211F_WOL_SETTINGS_STATUS, RTL8211F_WOL_STATUS_RESET);
-
-		/* Enable the WOL interrupt */
-		rtl821x_write_page(dev, RTL8211F_INTBCR_PAGE);
-		__phy_set_bits(dev, RTL8211F_INTBCR, RTL8211F_INTBCR_INTB_PMEB);
+		/* Set the maximum packet size, and assert WoL reset */
+		__phy_write(dev, RTL8211F_WOL_RST_RMSQ, RTL8211F_WOL_RMSQ);
 	} else {
-		/* Disable the WOL interrupt */
-		rtl821x_write_page(dev, RTL8211F_INTBCR_PAGE);
-		__phy_clear_bits(dev, RTL8211F_INTBCR, RTL8211F_INTBCR_INTB_PMEB);
-
-		/* Disable magic packet matching and reset WOL status */
-		rtl821x_write_page(dev, RTL8211F_WOL_SETTINGS_PAGE);
+		/* Disable magic packet matching */
+		rtl821x_write_page(dev, RTL8211F_WOL_PAGE);
 		__phy_write(dev, RTL8211F_WOL_SETTINGS_EVENTS, 0);
-		__phy_write(dev, RTL8211F_WOL_SETTINGS_STATUS, RTL8211F_WOL_STATUS_RESET);
+
+		/* Place WoL in reset */
+		__phy_clear_bits(dev, RTL8211F_WOL_RST_RMSQ,
+				 RTL8211F_WOL_RG_RSTB);
 	}
 
+	device_set_wakeup_enable(&dev->mdio.dev, !!(wol->wolopts & WAKE_MAGIC));
+
 err:
 	return phy_restore_page(dev, oldpage, 0);
 }
@@ -628,6 +673,52 @@ static int rtl821x_suspend(struct phy_device *phydev)
 	return ret;
 }
 
+static int rtl8211f_suspend(struct phy_device *phydev)
+{
+	u16 wol_rst;
+	int ret;
+
+	ret = rtl821x_suspend(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* 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);
+		if (ret < 0)
+			goto err;
+
+		/* Read the INSR to clear any pending interrupt */
+		phy_read_paged(phydev, RTL8211F_INSR_PAGE, RTL8211F_INSR);
+
+		/* Reset the WoL to ensure that an event is picked up.
+		 * Unless we do this, even if we receive another packet,
+		 * we may not have a PME interrupt raised.
+		 */
+		ret = phy_read_paged(phydev, RTL8211F_WOL_PAGE,
+				     RTL8211F_WOL_RST_RMSQ);
+		if (ret < 0)
+			goto err;
+
+		wol_rst = ret & ~RTL8211F_WOL_RG_RSTB;
+		ret = phy_write_paged(phydev, RTL8211F_WOL_PAGE,
+				      RTL8211F_WOL_RST_RMSQ, wol_rst);
+		if (ret < 0)
+			goto err;
+
+		wol_rst |= RTL8211F_WOL_RG_RSTB;
+		ret = phy_write_paged(phydev, RTL8211F_WOL_PAGE,
+				      RTL8211F_WOL_RST_RMSQ, wol_rst);
+	}
+
+err:
+	return ret;
+}
+
 static int rtl821x_resume(struct phy_device *phydev)
 {
 	struct rtl821x_priv *priv = phydev->priv;
@@ -645,6 +736,25 @@ static int rtl821x_resume(struct phy_device *phydev)
 	return 0;
 }
 
+static int rtl8211f_resume(struct phy_device *phydev)
+{
+	struct rtl821x_priv *priv = phydev->priv;
+	int ret;
+
+	ret = rtl821x_resume(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* If the device was programmed for a PME event, restore the interrupt
+	 * enable and interrupt pin configuration state so phylib can receive
+	 * link state interrupts.
+	 */
+	if (device_may_wakeup(&phydev->mdio.dev))
+		ret = phy_write_paged(phydev, 0xa42, RTL821x_INER, priv->iner);
+
+	return ret;
+}
+
 static int rtl8211x_led_hw_is_supported(struct phy_device *phydev, u8 index,
 					unsigned long rules)
 {
@@ -1612,15 +1722,15 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc916),
 		.name		= "RTL8211F Gigabit Ethernet",
-		.probe		= rtl821x_probe,
+		.probe		= rtl8211f_probe,
 		.config_init	= &rtl8211f_config_init,
 		.read_status	= rtlgen_read_status,
 		.config_intr	= &rtl8211f_config_intr,
 		.handle_interrupt = rtl8211f_handle_interrupt,
 		.set_wol	= rtl8211f_set_wol,
 		.get_wol	= rtl8211f_get_wol,
-		.suspend	= rtl821x_suspend,
-		.resume		= rtl821x_resume,
+		.suspend	= rtl8211f_suspend,
+		.resume		= rtl8211f_resume,
 		.read_page	= rtl821x_read_page,
 		.write_page	= rtl821x_write_page,
 		.flags		= PHY_ALWAYS_CALL_SUSPEND,
-- 
2.30.2

Re: [PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Andrew Lunn]

> 2. detect whether we can support wake-up by having a valid interrupt,
>    and the "wakeup-source" property in DT. If we can, then we mark
>    the MDIO device as wakeup capable, and associate the interrupt
>    with the wakeup source.

We should document "wakeup-source" in ethernet-phy.yaml.

What are the different hardware architectures?

1) A single interrupt line from the PHY to the SoC, which does both
link status and WoL.

2) The PHY has a dedicated WoL output pin, which is connected to an
interrupt.

3) The PHY has a dedicated WoL output pin, which is connected directly
to a PMIC. No software involved, the pin toggling turns the power back
on.

For 1), i don't think 'wakeup-source' tells us anything useful. The
driver just needs to check that interrupts are in use.

For 2) we should recommend that the wakeup interrupt is called
"wakeup", following wakeup-source.txt, and the "wakeup-source"
property is present.

For 3) its more magical, there is no interrupt properties involved, so
we do need the "wakeup-source" to know that the pin is actually
connected to something.

We need to differentiate between drivers newly getting WoL support,
and existing drivers. We can be much more strict with new support.

	Andrew

Re: [PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Russell King (Oracle)]

On Wed, Jul 30, 2025 at 03:59:32PM +0200, Andrew Lunn wrote:
> > 2. detect whether we can support wake-up by having a valid interrupt,
> >    and the "wakeup-source" property in DT. If we can, then we mark
> >    the MDIO device as wakeup capable, and associate the interrupt
> >    with the wakeup source.
> 
> We should document "wakeup-source" in ethernet-phy.yaml.
> 
> What are the different hardware architectures?
> 
> 1) A single interrupt line from the PHY to the SoC, which does both
> link status and WoL.
> 
> 2) The PHY has a dedicated WoL output pin, which is connected to an
> interrupt.
> 
> 3) The PHY has a dedicated WoL output pin, which is connected directly
> to a PMIC. No software involved, the pin toggling turns the power back
> on.
> 
> For 1), i don't think 'wakeup-source' tells us anything useful. The
> driver just needs to check that interrupts are in use.

Not all interrupts are capable of waking the system up, and there is
no way for a PHY to know whether it's connected to an interrupt that
has that ability.

As things currently stand with how Jetson Xavier NX DT describes the
PHY's connection, it falls into this "it has an interrupt which can't
wake the system" - so these cases really do exist in the real world.

So, we _need_ to have some way to differentiate these two cases, and
I put the question to you - if not by "wakeup-source" then how do we
determine whether a PHY, which itself is capable of signalling wakeup
through its interrupt pin, can actually wake the system, and thus
should expose WoL functionality?

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

Re: [PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Gatien CHEVALLIER]

On 7/30/25 15:59, Andrew Lunn wrote:
>> 2. detect whether we can support wake-up by having a valid interrupt,
>>     and the "wakeup-source" property in DT. If we can, then we mark
>>     the MDIO device as wakeup capable, and associate the interrupt
>>     with the wakeup source.
> 
> We should document "wakeup-source" in ethernet-phy.yaml.
> 

+1

> What are the different hardware architectures?
> 
> 1) A single interrupt line from the PHY to the SoC, which does both
> link status and WoL.
> 
> 2) The PHY has a dedicated WoL output pin, which is connected to an
> interrupt.
> 
> 3) The PHY has a dedicated WoL output pin, which is connected directly
> to a PMIC. No software involved, the pin toggling turns the power back
> on.
> 

Just my 2 cents:

In some cases like the LAN8742, there are some flags that need to be
cleared when waking up in order to be able to handle another WoL event.
It can be done either in the suspend()/resume() or in an interrupt
handler of the PHY. In 3) This suggests that the interrupt is somehow
forwarded to the Linux kernel.

This is what I was ultimately trying to do in two steps with the TEE
notifying the kernel of that interrupt.

Moreover, if a WoL event occurs when the system is not in a low-power
mode, then the flags will never be cleared and another WoL event cannot
be detected while the system is in a low-power mode.

Maybe we can argue that these flags can be cleared in suspend() and
and resume(). But then, if there's no interrupt to be handled by the
kernel, how do we know that we have woken up from a WoL event?

IMHO, I think 3) may optionally declare another interrupt as well
for WoL events.

Eventually, 2) and 3) would have 1 interrupt(WoL) if PHY is in polling
mode and 2 if not?

Please tell me if that makes any sense to you or if I missed something.

> For 1), i don't think 'wakeup-source' tells us anything useful. The
> driver just needs to check that interrupts are in use.
> 
> For 2) we should recommend that the wakeup interrupt is called
> "wakeup", following wakeup-source.txt, and the "wakeup-source"
> property is present.
> 
> For 3) its more magical, there is no interrupt properties involved, so
> we do need the "wakeup-source" to know that the pin is actually
> connected to something.
> 
> We need to differentiate between drivers newly getting WoL support,
> and existing drivers. We can be much more strict with new support.
> 
> 	Andrew

Re: [PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Andrew Lunn]

> Not all interrupts are capable of waking the system up, and there is
> no way for a PHY to know whether it's connected to an interrupt that
> has that ability.

I was wondering about that. And maybe that enable_irq_wake() returns
-EOPNOTSUPP if it cannot actually wake the system? But there is no
documentation about that.

So we cannot trust it, and "wakeup-source" is needed.

   Andrew

Re: [PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Andrew Lunn]

> > 3) The PHY has a dedicated WoL output pin, which is connected directly
> > to a PMIC. No software involved, the pin toggling turns the power back
> > on.
> > 
> 
> Just my 2 cents:
> 
> In some cases like the LAN8742, there are some flags that need to be
> cleared when waking up in order to be able to handle another WoL event.
> It can be done either in the suspend()/resume() or in an interrupt
> handler of the PHY. In 3) This suggests that the interrupt is somehow
> forwarded to the Linux kernel.
> 
> This is what I was ultimately trying to do in two steps with the TEE
> notifying the kernel of that interrupt.
> 
> Moreover, if a WoL event occurs when the system is not in a low-power
> mode, then the flags will never be cleared and another WoL event cannot
> be detected while the system is in a low-power mode.
> 
> Maybe we can argue that these flags can be cleared in suspend() and
> and resume(). But then, if there's no interrupt to be handled by the
> kernel, how do we know that we have woken up from a WoL event?
> 
> IMHO, I think 3) may optionally declare another interrupt as well
> for WoL events.

The 3) i've seen is a Marvell based NAS box. It is a long time
ago. But as far as i remember, the SoC had no idea why it woke up, it
could not ask the PMIC why the power was turned on. So there was no
ability to invoke an interrupt handler.

I've also seen X86/BIOS platforms which are similar. The BIOS swallows
the interrupt, it never gets delivered to Linux once it is
running. And the BIOS itself might poke PHY registers.

> Eventually, 2) and 3) would have 1 interrupt(WoL) if PHY is in polling
> mode and 2 if not?

Nope. These NAS boxed often had 0 PHY interrupts, and polling was
used. If you have a single ethernet port, you don't care it takes a
while to know the link is dead. There is nothing you can do, you don't
have a second interface to fall back to.

     Andrew

Re: [PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Daniel Braunwarth]

>> IMHO, I think 3) may optionally declare another interrupt as well
>> for WoL events.
>
> The 3) i've seen is a Marvell based NAS box. It is a long time
> ago. But as far as i remember, the SoC had no idea why it woke up, it
> could not ask the PMIC why the power was turned on. So there was no
> ability to invoke an interrupt handler.
>
> I've also seen X86/BIOS platforms which are similar. The BIOS swallows
> the interrupt, it never gets delivered to Linux once it is
> running. And the BIOS itself might poke PHY registers.

That's like our hardware looks like.

We have a TI J784S4 SoC which has an integrated 9 port switch. One internal, and 8 external ports.

It's similar to the device tree in arch/arm64/boot/dts/ti/k3-j784s4-evm-quad-port-eth-exp1.dtso.

The PHYs are directly connected to the PMIC of our board.
We don't have any clue **why** the PMIC wakes the board.
Could be a WoL event or somebody pressed the power button of our robot controller.


Regards
Daniel

Internal

Re: [PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Russell King (Oracle)]

On Wed, Jul 30, 2025 at 04:35:45PM +0200, Andrew Lunn wrote:
> > Not all interrupts are capable of waking the system up, and there is
> > no way for a PHY to know whether it's connected to an interrupt that
> > has that ability.
> 
> I was wondering about that. And maybe that enable_irq_wake() returns
> -EOPNOTSUPP if it cannot actually wake the system? But there is no
> documentation about that.

,,, and that means we can't use the wakeirq helpers.

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

Re: [PATCH RFC ???net???] net: phy: realtek: fix wake-on-lan support

[Florian Fainelli]

On 7/30/25 01:35, Russell King (Oracle) wrote:
> Implement Wake-on-Lan correctly. The existing implementation has
> multiple issues:
> 
> 1. It assumes that Wake-on-Lan can always be used, whether or not the
>     interrupt is wired, and whether or not the interrupt is capable of
>     waking the system. This breaks the ability for MAC drivers to detect
>     whether the PHY WoL is functional.
> 2. switching the interrupt pin in the .set_wol() method to PMEB mode
>     immediately silences link-state interrupts, which breaks phylib
>     when interrupts are being used rather than polling mode.
> 3. the code claiming to "reset WOL status" was doing nothing of the
>     sort. Bit 15 in page 0xd8a register 17 controls WoL reset, and
>     needs to be pulsed low to reset the WoL state. This bit was always
>     written as '1', resulting in no reset.
> 4. not resetting WoL state results in the PMEB pin remaining asserted,
>     which in turn leads to an interrupt storm. Only resetting the WoL
>     state in .set_wol() is not sufficient.
> 5. PMEB mode does not allow software detection of the wake-up event as
>     there is no status bit to indicate we received the WoL packet.
> 6. across reboots of at least the Jetson Xavier NX system, the WoL
>     configuration is preserved.
> 
> Fix all of these issues by essentially rewriting the support. We:
> 1. clear the WoL event enable register at probe time.
> 2. detect whether we can support wake-up by having a valid interrupt,
>     and the "wakeup-source" property in DT. If we can, then we mark
>     the MDIO device as wakeup capable, and associate the interrupt
>     with the wakeup source.
> 3. arrange for the get_wol() and set_wol() implementations to handle
>     the case where the MDIO device has not been marked as wakeup
>     capable (thereby returning no WoL support, and refusing to enable
>     WoL support.)
> 4. avoid switching to PMEB mode, instead using INTB mode with the
>     interrupt enable, reconfiguring the interrupt enables at suspend
>     time, and restoring their original state at resume time (we track
>     the state of the interrupt enable register in .config_intr()
>     register.)
> 5. move WoL reset from .set_wol() to the suspend function to ensure
>     that WoL state is cleared prior to suspend. This is necessary
>     after the PME interrupt has been enabled as a second WoL packet
>     will not re-raise a previously cleared PME interrupt.
> 6. when a PME interrupt (for wakeup) is asserted, pass this to the
>     PM wakeup so it knows which device woke the system.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

This looks much better and straightforward to me, thanks!

> ---
> I've sort-of tested this on the Jetson Xavier NX platform, but it's
> been difficult because it appears that the whole interrupt/wakeup
> stuff for the SoC is foobar in mainline. One gets the choice of
> specifying the GPIO interrupt in DT and have working normal interrupt
> or the power management controller interrupt for the same line and
> having wakeup functional. You can't have both together.
> 
> I'm not sure whether this change should target the net or net-next
> tree; what we have currently in 6.16 is totally and utterly broken,
> so arguably this is a fix - but it's not a regression because 6.16
> is the first kernel that WoL "support" for RTL8211F is in. This is
> also a large change.
> 
> However, I can't see that it was tested, given all the problems
> identified above. As a result, I've taken the decision in this patch
> to not worry about breaking anyone's existing setup.
> 
> So, I have no problem with requiring "wakeup-source" to be added to DT
> for rtl8211f PHYs that are to support wake-up, meaning that they are
> properly wired to support WoL. We can't tell just from having an
> interrupt - not all interrupts on all devices may be wake-up capable.

That seems entirely reasonable to me to expect.
-- 
Florian