Message-Id: <89595AAE-8A92-4D8B-A40C-E5437B750B42@exactco.de>
Date: Tue, 2 Dec 2025 14:46:32 +0100
From: René Rebe <rene@...ctco.de>
To: Heiner Kallweit <hkallweit1@...il.com>
Cc: netdev@...r.kernel.org,
 nic_swsd@...ltek.com
Subject: Re: [PATCH] r8169: fix RTL8117 Wake-on-Lan in DASH mode

Hi,

> On 2. Dec 2025, at 08:07, Heiner Kallweit <hkallweit1@...il.com> wrote:
> 
> On 12/2/2025 12:26 AM, René Rebe wrote:
>> Hi,
>> 
>>> On 1. Dec 2025, at 22:12, Heiner Kallweit <hkallweit1@...il.com> wrote:
>>> 
>>> On 12/1/2025 9:31 PM, René Rebe wrote:
>>>> Hi
>>>> 
>>>>> On 1. Dec 2025, at 21:15, Heiner Kallweit <hkallweit1@...il.com> wrote:
>>>>> 
>>>>> On 12/1/2025 8:17 PM, René Rebe wrote:
>>>>>> Wake-on-Lan does currently not work in DASH mode, e.g. the ASUS Pro WS
>>>>>> X570-ACE with RTL8168fp/RTL8117.
>>>>>> 
>>>>>> Fix by not returning early in rtl_prepare_power_down when dash_enabled.
>>>>> Good
>>>>> 
>>>>>> While this fixes WOL, it still kills the OOB RTL8117 remote management
>>>>>> BMC connection. Fix by not calling rtl8168_driver_stop if wol is enabled.
>>>>>> 
>>>>> You mean remote management whilst system is powered down and waiting
>>>>> for a WoL packet? Note that link speed is reduced to a minimum then,
>>>>> and DMA is disabled. Who would drive the MAC?
>>>>> Realtek doesn't provide any chip documentation, therefore it's hard to
>>>>> say what is expected from the MAC driver in DASH case.
>>>> 
>>>> This RTL8117 has a 250 or 400 MHz MIPS cpu inside that runs
>>>> a out-of-band linux kernel. Pretty sketchy low-quality setup unfortunately:
>>>> 
>>>> https://www.youtube.com/watch?v=YqEa8Gd1c2I&t=1695s
>>>>> 
>>>>>> While at it, enable wake on magic packet by default, like most other
>>>>>> Linux drivers do.
>>>>>> 
>>>>> It's by intent that WoL is disabled per default. Most users don't use WoL
>>>>> and would suffer from higher power consumption if system is suspended
>>>>> or powered down.
>>>> 
>>>> It was just a suggestion, I can use ethtool, it is the only driver that does
>>>> not have it on by default in all the systems I have.
>>>> 
>>>>> Which benefit would you see if WoL would be enabled by default
>>>>> (in DASH and non-DASH case)?
>>>> 
>>>> So it just works when pro-sumers want to wake it up, not the most
>>>> important detail of the patch.
>>>> 
>>>>>> Signed-off-by: René Rebe <rene@...ctco.de>
>>>>> 
>>>>> Your patch apparently is meant to be a fix. Therefore please add Fixes
>>>>> tag and address to net tree.
>>>>> https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.rst
>>>>> And please add all netdev maintainers when re-submitting.
>>>>> scripts/get_maintainer.pl provides all needed info.
>>>> 
>>>> Yes, I realized after sending. The only Fixes: would be the original
>>>> change adding the DASH support I assume?
>>>> 
>>>> Any opinion re not stopping DASH on if down? IMHO taking a
>>>> link down should not break the remote management connection.
>>>> 
>>> I have no clue how the OOB BMC interacts with MAC/PHY, and I have no
>>> hw supporting DASH to test. So not really a basis for an opinion.
>>> However: DASH has been existing on Realtek hw for at least 15 yrs,
>>> and I'm not aware of any complaint related to what you mention.
>>> So it doesn't seem to be a common use case.
>> 
>> Well the Asus Control Center Express is so bad and barely working
>> it does not surprise me nobody is using it. We reversed the protocol
>> and wrote some script and hacked VNC client to make it useful for us.
>> 
>> The Asus GPL compliance code dump for the MIPS Linux BMC system
>> has some rtl8168_oob or so BMC side driver for it to learn more details.
>> 
> Do you have a link to this code?

https://www.asus.com/motherboards-components/motherboards/workstation/pro-ws-x570-ace/helpdesk_download?model2Name=Pro-WS-X570-ACE

Select OS: Others:  Source Code: 8117 firmware source code

	rtl8117_src_0114.tar.gz.zip

They updated it since my last download. The last time I checked they
where IMHO not GPL compliant: I mailed them this twice in 2022
but never heard back:

> and on an initial review already found it to be incomplete and at least lacking the apro-ctrl
> driver statically linked into the Linux kernel. The GPL v2 clearly says among
> other details in section 3:
> 
> complete machine-readable copy of the corresponding source code
> 
> I would therefore kindly ask to make available the whole modified Linux
> kernel source code.


I should check if it is more complete now. The BMC systems is so barely
usable and buggy old outdated stuff, I would actually update it with my
own, ...

>> Maybe I should backup a fork of it on my GitHub to archive it.
>> 
>> Given the BMC should be reachable, would be acceptable if I work
>> out a patch to not take the phy down and always keep it up even
>> for if down and module unload when in “dash” mode?
>> 
> Yes
> 
>>> There are different generations of DASH in RTL8168DP, RTL8168EP,
>>> RTL8117, variants of RTL8125, RTL8127 etc. Having said that,
>>> there's a certain chance of a regression, even if the patch works
>>> correctly on your system. Therefore I'd prefer to handle any additional
>>> changes in separate patches, to facilitate bisecting in case of a
>>> regression.
>> 
>> Of course, will sent separate patches for each topic.
>> 
>> What about defaulting to wol by magic like most other linux
>> drivers? IMHO the drivers should all be have similar and not
>> all have some other defaults. In theory it could be made
>> a tree-wide kconfig if users and distros care enough what the
>> global default should be.
>> 
> I'm not in favor of enabling WoL per default, as it prevents
> powering down the PHY if system is suspended / shut down.
> This impacts especially users of mobile devices.
> "Others do it too" for me is a weak argument, if no one can
> explain why it's a good thing what they're doing.

Well, the argument is for wakeup to “just work”. There also
should be some consistency in Linux. Either all drivers should
enable it or disable it by default. That is why I have thrown in
the idea of a new kconfig options for downstream distros to
make a conscious global choice. E.g. we would ship it it
enabled.

	René

>> René
>> 
>>>> I probably would need to single step thru the driver init to find out
>>>> what reset stops the out of band traffic there, too.
>>>> 
>>>> René
>>>> 
>>>>>> ---
>>>>>> 
>>>>>> There is still another issue that should be fixed: the dirver init
>>>>>> kills the OOB BMC connection until if up, too. We also should probaly
>>>>>> not even conditionalize rtl8168_driver_stop on wol_enabled as the BMC
>>>>>> should always be accessible. IMHO even on module unload.
>>>>>> 
>>>>>> ---
>>>>>> drivers/net/ethernet/realtek/r8169_main.c | 9 +++++----
>>>>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>>>> 
>>>>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>>>>> index 853aabedb128..e2f9b9027fe2 100644
>>>>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>>>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>>>>> @@ -2669,9 +2669,6 @@ static void rtl_wol_enable_rx(struct rtl8169_private *tp)
>>>>>> 
>>>>>> static void rtl_prepare_power_down(struct rtl8169_private *tp)
>>>>>> {
>>>>>> - if (tp->dash_enabled)
>>>>>> - return;
>>>>>> -
>>>>>> if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
>>>>>>  tp->mac_version == RTL_GIGA_MAC_VER_33)
>>>>>> rtl_ephy_write(tp, 0x19, 0xff64);
>>>>>> @@ -4807,7 +4804,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
>>>>>> rtl_disable_exit_l1(tp);
>>>>>> rtl_prepare_power_down(tp);
>>>>>> 
>>>>>> - if (tp->dash_type != RTL_DASH_NONE)
>>>>>> + if (tp->dash_type != RTL_DASH_NONE && !tp->saved_wolopts)
>>>>>> rtl8168_driver_stop(tp);
>>>>>> }
>>>>>> 
>>>>>> @@ -5406,6 +5403,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>>>> tp->pci_dev = pdev;
>>>>>> tp->supports_gmii = ent->driver_data == RTL_CFG_NO_GBIT ? 0 : 1;
>>>>>> tp->ocp_base = OCP_STD_PHY_BASE;
>>>>>> + tp->saved_wolopts = WAKE_MAGIC;
>>>>>> 
>>>>>> raw_spin_lock_init(&tp->mac_ocp_lock);
>>>>>> mutex_init(&tp->led_lock);
>>>>>> @@ -5565,6 +5563,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>>>> if (rc)
>>>>>> return rc;
>>>>>> 
>>>>>> + if (tp->saved_wolopts)
>>>>>> + __rtl8169_set_wol(tp, tp->saved_wolopts);
>>>>>> +
>>>>>> rc = register_netdev(dev);
>>>>>> if (rc)
>>>>>> return rc;
>>>>> 
>>>> 
>>> 
>> 
> 

-- 
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Powered by blists - more mailing lists