Re: RTL8402 stops working after hibernate/resume

From: Petr Tesarik <ptesarik@suse.cz>
To: Heiner Kallweit <hkallweit1@gmail.com>,
	Hans de Goede <hdegoede@redhat.com>
Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>,
	netdev@vger.kernel.org, linux-clk@vger.kernel.org
Subject: Re: RTL8402 stops working after hibernate/resume
Date: Tue, 29 Sep 2020 21:07:37 +0200	[thread overview]
Message-ID: <20200929210737.7f4a6da7@ezekiel.suse.cz> (raw)
In-Reply-To: <30969885-9611-06d8-d50a-577897fcab29@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

Hi Heiner (and now also Hans)!

@Hans: I'm adding you to this conversation, because you're the author
of commit b1e3454d39f99, which seems to break the r8169 driver on a
laptop of mine.

On Fri, 25 Sep 2020 16:47:54 +0200
Heiner Kallweit <hkallweit1@gmail.com> wrote:

> On 25.09.2020 14:56, Petr Tesarik wrote:
> > On Fri, 25 Sep 2020 11:52:41 +0200
> > Petr Tesarik <ptesarik@suse.cz> wrote:
> >   
> >> On Fri, 25 Sep 2020 11:44:09 +0200
> >> Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>  
> >>> On 25.09.2020 10:54, Petr Tesarik wrote:    
> >> [...]  
> >>>> Does it make sense to bisect the change that broke the driver for me, or should I rather dispose of this waste^Wlaptop in an environmentally friendly manner? I mean, would you eventually accept a workaround for a few machines with a broken BIOS?
> >>>>       
> >>> If the workaround is small and there's little chance to break other stuff: then usually yes.
> >>> If you can spend the effort to bisect the issue, this would be appreciated.    
> >>
> >> OK, then I'm going to give it a try.  
> > 
> > Done. The system freezes when this commit is applied:
> > 
> > commit 9f0b54cd167219266bd3864570ae8f4987b57520
> > Author: Heiner Kallweit <hkallweit1@gmail.com>
> > Date:   Wed Jun 17 22:55:40 2020 +0200
> > 
> >     r8169: move switching optional clock on/off to pll power functions
> >   
> This sounds weird. On your system tp->clk should be NULL, making
> clk_prepare_enable() et al no-ops. Please check whether tp->clk
> is NULL after the call to rtl_get_ether_clk().

This might be part of the issue. On my system tp->clk is definitely not
NULL:

crash> *rtl8169_private.clk 0xffff9277aca58940
  clk = 0xffff9277ac2c82a0

crash> *clk 0xffff9277ac2c82a0
struct clk {
  core = 0xffff9277aef65d00, 
  dev = 0xffff9277aed000b0, 
  dev_id = 0xffff9277aec60c00 "0000:03:00.2", 
  con_id = 0xffff9277ad04b080 "ether_clk", 
  min_rate = 0, 
  max_rate = 18446744073709551615, 
  exclusive_count = 0, 
  clks_node = {
    next = 0xffff9277ad2428d8, 
    pprev = 0xffff9277aef65dc8
  }
}

The dev_id corresponds to the Ethernet controller:

03:00.2 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL810xE PCI Express Fast Ethernet controller (rev 06)

Looking at clk_find(), it matches this entry in clocks:

struct clk_lookup {
  node = {
    next = 0xffffffffbc702f40, 
    prev = 0xffff9277bf7190c0
  }, 
  dev_id = 0x0, 
  con_id = 0xffff9277bf719524 "ether_clk", 
  clk = 0x0, 
  clk_hw = 0xffff9277ad2427f8
}

That's because this kernel is built with CONFIG_PMC_ATOM=y, and looking
at the platform initialization code, the "ether_clk" alias is created
unconditionally. Hans already added.

Petr T

[-- Attachment #2: Digitální podpis OpenPGP --]
[-- Type: application/pgp-signature, Size: 488 bytes --]