On 29.09.2020 21:07, Petr Tesarik wrote:
> 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 <hkallwe...@gmail.com> wrote:
>
>> On 25.09.2020 14:56, Petr Tesarik wrote:
>>> On Fri, 25 Sep 2020 11:52:41 +0200
>>> Petr Tesarik <ptesa...@suse.cz> wrote:
>>>
>>>> On Fri, 25 Sep 2020 11:44:09 +0200
>>>> Heiner Kallweit <hkallwe...@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 <hkallwe...@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:
>
OK, interesting. The referenced patch changes driver behavior in a way
that ether_clk is disabled again in probe(), and only re-enabled
in ndo_open(). This should be helpful from a power-saving perspective
because before that we enabled the clock even if the network device
wasn't used.
It seems however that disabling ether_clk has (at least on your system)
side effects causing a system freeze. That's a best guess from my side
however, and not really something I can comment on.
> 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
>
