Hi Heiner again,

On Wed, 30 Sep 2020 19:32:59 +0200
Petr Tesarik <ptesarik@suse.cz> wrote:

> Hi Heiner,
> 
> On Wed, 30 Sep 2020 18:41:24 +0200
> Petr Tesarik <ptesarik@suse.cz> wrote:
> 
> > HI Heiner,
> > 
> > On Wed, 30 Sep 2020 17:47:15 +0200
> > Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >[...]  
> > > Petr,
> > > in the following I send two patches. First one is supposed to fix the freeze.
> > > It also fixes another issue that existed before my ether_clk change:
> > > ether_clk was disabled on suspend even if WoL is enabled. And the network
> > > chip most likely needs the clock to check for WoL packets.    
> > 
> > Should I also check whether WoL works? Plus, it would be probably good
> > if I could check whether it indeed didn't work before the change. ;-)
> >   
> > > Please let me know whether it fixes the freeze, then I'll add your
> > >   Tested-by.    
> > 
> > Will do.  
> 
> Here are the results:
> 
> - WoL does not work without your patch; this was tested with 5.8.4,
>   because master freezes hard on load.
> 
> - I got a kernel crash when I woke up the laptop through keyboard; I am
>   not sure if it is related, although I could spend some time looking
>   at the resulting crash dump if you think it's worth the time.
> 
> - After applying your first patch on top of v5.9-rc6, the driver can be
>   loaded, but stops working properly on suspend/resume.
> 
> - WoL still does not work, but I'm no longer getting a kernel crash at
>   least. ;-)
> 
> Tested-by: Petr Tesarik <ptesarik@suse.com>
> 
> > > Second patch is a re-send of the one I sent before, it should fix
> > > the rx issues after resume from suspend for you.    
> > 
> > All right, going to rebuild the kernel and see how it goes.  
> 
> This second patch does not fix suspend/resume for me, unfortunately. The
> receive is still erratic, but the behaviour is now different: some
> packets are truncated, other packets are delayed by approx. 1024 ms.
> Yes, 1024 may ring a bell, but I've no idea which.

I'm sorry, I added some debugging message, and as I was wondering why
the resume path is not run, I found out I was not properly reloading the
modified module. So, after fixing my build, your patch also fixes the
Rx queue on resume! Both with and without NetworkManager.

So, you can also add to the second patch:

Tested-by: Petr Tesarik <ptesarik@suse.com>

WoL still does not work on my laptop, but this might be an unrelated
issue, and I can even imagine the BIOS is buggy in this regard.

Petr T

> HTH,
> Petr T
> 
> > Stay tuned,
> > Petr T
> > 
> >   
> > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c
> > >   b/drivers/net/ethernet/realtek/r8169_main.c index
> > >   6c7c004c2..72351c5b0 100644 ---
> > >   a/drivers/net/ethernet/realtek/r8169_main.c +++
> > >   b/drivers/net/ethernet/realtek/r8169_main.c @@ -2236,14 +2236,10
> > >   @@ static void rtl_pll_power_down(struct rtl8169_private *tp)
> > >   default: break;
> > >  	}
> > > -
> > > -	clk_disable_unprepare(tp->clk);
> > >  }
> > >  
> > >  static void rtl_pll_power_up(struct rtl8169_private *tp)
> > >  {
> > > -	clk_prepare_enable(tp->clk);
> > > -
> > >  	switch (tp->mac_version) {
> > >  	case RTL_GIGA_MAC_VER_25 ... RTL_GIGA_MAC_VER_33:
> > >  	case RTL_GIGA_MAC_VER_37:
> > > @@ -4820,29 +4816,39 @@ static void rtl8169_net_suspend(struct
> > >   rtl8169_private *tp) 
> > >  #ifdef CONFIG_PM
> > >  
> > > +static int rtl8169_net_resume(struct rtl8169_private *tp)
> > > +{
> > > +	rtl_rar_set(tp, tp->dev->dev_addr);
> > > +
> > > +	if (tp->TxDescArray)
> > > +		rtl8169_up(tp);
> > > +
> > > +	netif_device_attach(tp->dev);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int __maybe_unused rtl8169_suspend(struct device *device)
> > >  {
> > >  	struct rtl8169_private *tp = dev_get_drvdata(device);
> > >  
> > >  	rtnl_lock();
> > >  	rtl8169_net_suspend(tp);
> > > +	if (!device_may_wakeup(tp_to_dev(tp)))
> > > +		clk_disable_unprepare(tp->clk);
> > >  	rtnl_unlock();
> > >  
> > >  	return 0;
> > >  }
> > >  
> > > -static int rtl8169_resume(struct device *device)
> > > +static int __maybe_unused rtl8169_resume(struct device *device)
> > >  {
> > >  	struct rtl8169_private *tp = dev_get_drvdata(device);
> > >  
> > > -	rtl_rar_set(tp, tp->dev->dev_addr);
> > > -
> > > -	if (tp->TxDescArray)
> > > -		rtl8169_up(tp);
> > > +	if (!device_may_wakeup(tp_to_dev(tp)))
> > > +		clk_prepare_enable(tp->clk);
> > >  
> > > -	netif_device_attach(tp->dev);
> > > -
> > > -	return 0;
> > > +	return rtl8169_net_resume(tp);
> > >  }
> > >  
> > >  static int rtl8169_runtime_suspend(struct device *device)
> > > @@ -4868,7 +4874,7 @@ static int rtl8169_runtime_resume(struct
> > >   device *device) 
> > >  	__rtl8169_set_wol(tp, tp->saved_wolopts);
> > >  
> > > -	return rtl8169_resume(device);
> > > +	return rtl8169_net_resume(tp);
> > >  }
> > >  
> > >  static int rtl8169_runtime_idle(struct device *device)    
> >   
>