Message-ID: <6ae02156-8903-70fd-675f-a5aee9c452b4@gmail.com>
Date:   Fri, 25 Jan 2019 07:49:56 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Marc Haber <mh+netdev@...schlus.de>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: WoL broken in r8169.c since kernel 4.19

Hi Marc,

thanks a lot for the detailed analysis. That this ethtool sequence

ethtool -s <if> wol d
ethtool -s <if> wol g

helps makes me think that the following patch should help too.
Could you please test?

There's an old story why this call is missing. Certain notebooks immediately
woke up again if WoL was enabled and they were suspended. I think they
should disable WoL instead of having this hack in the driver.

Heiner

---
 drivers/net/ethernet/realtek/r8169.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 04c403539..1d101659f 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7423,6 +7423,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 
 	tp->saved_wolopts = __rtl8169_get_wol(tp);
+	device_set_wakeup_enable(&pdev->dev, tp->saved_wolopts);
 
 	mutex_init(&tp->wk.mutex);
 	INIT_WORK(&tp->wk.work, rtl_task);
-- 
2.20.1




On 25.01.2019 06:59, Marc Haber wrote:
> Hi Heiner,
> 
> On Tue, Jan 22, 2019 at 07:47:45PM +0100, Heiner Kallweit wrote:
>> Which version of 4.18 are you running that is ok? To check the code ..
> 
> I pull over drivers/net/ethernet/realtek/r8169.c from an unpacked
> 4.18.16 source tree. It sets RTL8169_VERSION "2.3LK-NAPI". The last
> commit in this file's history is
> 
> commit 241e5a13a5ad75fd0b5a47b583e098fa08917d5c
> Author: Maciej S. Szmigiero <mail@...iej.szmigiero.name>
> Date:   Thu Oct 11 16:02:10 2018 +0200
> 
>     r8169: set RX_MULTI_EN bit in RxConfig for 8168F-family chips
> 
> Unfortunately, very different r8169.c in 4.20.3 also sets RTL8169_VERSION "2.3LK-NAPI", so we cannot use this as identification.
> 
>> Based on what I wrote above, could you try the following sequence and
>> check whether wake from S3 works then?
>>
>> ethtool -s <if> wol d
>> ethtool -s <if> wol g
> 
> This test was done with the Debian kernel package 4.20.0-trunk-amd64,
> which contains a kernel 4.20.1 compiled on 2018-12-24, from Debian's
> experimental repository.
> 
> This are my steps:
> (1) ethtool -s lanc0 wol d
> (2) ethtool -s lanc0 wol g
> (3) systemctl suspend
> (4) system gows down into Suspend to RAM, an audible *click* from a
>     relay, power LED blinking, Switch reporting that the ethernet link state
>     changed from 1000Full to down and then to 10Full.
> (5) sent a magic packet
> (6) *click* system up again. This is good.
> (7) switch reported link state change from 10Full to down and then to
>     1000Full
> (8) system and network operational
> (9) shut down the system completely, remove power. all LEDs off, link
>     down
> (10) re-plug power, all LEDs off, link 10Full
> (11) push power button, bring system up again with Debian's 4.20 kernel
> (12) link up at 1000Full
> (12a) take register dump, exhibit 12a
> (13) systemctl suspend
> (14) system gows down into Suspend to RAM, an audible *click* from a
>      relay, power LED blinking, Switch reporting that the ethernet link state
>      changed from 1000Full to down and then to 10Full.
> (15) send magic packet identical to step (5)
> (16) system remains suspended, blinking LED; link 10Full
> (17) push any key on keyboard
> (18) *click* system up again. This is good.
> (19) switch reported link state change from 10Full to down and then to
>      1000Full
> (19a) take register dump, exhibit 19a
> (20) system and network operational
> (21) repeat steps (1) through (5)
> (22) system reacts to magic packet and wakes up
> (23) ethtool -s lanc0 wol d
> (24) ethtool -s lanc0 wol g
> (25) take register dump, exhibit 25
> (26) systemctl suspend
> (27) system gows down into Suspend to RAM, an audible *click* from a
>     relay, power LED blinking, Switch reporting that the ethernet link state
>     changed from 1000Full to down and then to 10Full.
> (28) sent a magic packet
> (29) *click* system up again. This is good.
> (30) switch reported link state change from 10Full to down and then to
>    1000Full
> (31) take register dump, exhibit 31
> (32) system and network operational
> 
> So, our hypothesis is now that the older driver does "something" that
> the new driver doesn't, but what is needed for my machine to wake up on
> magic packet from suspend to RAM. the two ethtool calls do seem to fix
> that.
> 
> Unfortunately, exhibits 19a, 25 and 31 are identical, and exhibit 12a
> only differs in a single line, PHY access holding 0x8001796d instead of
> 0x80040de1. So, whatever the two ethtools do, it is not visible in the
> register dump.
> 
> I repaeted the experiment with stock 4.20.4, with the same results, and
> 4.20.4 with a r8169.c transplant from 4.18.16, with the result of
> ethtool not needed.
> 
> Exhibits from the stock 4.20.4 experiments:
> exhibit-4.20.4-preeth-presleep - direct after boot
> exhibit-4.20.4-preeth-postsleep - after suspend with manual wakeup needed
> exhibit-4.20.4-posteth-presleep - after the two ethtool calls
> exhibit-4.20.4-posteth-postsleep - after successful wake on lan
> 
> Exhibits from the 4.20.4+transplant experiments:
> exhibit-4.18-presleep - direct after boot
> exhibit-4.18-postsleep - after successful wake on LAN.
> 
> I hope this helps. Let me know if additional data is helpful.
> 
> Greetings
> Marc
> 

Powered by blists - more mailing lists