netdev - Re: [PATCH net] net: phy: replace preliminary fix for PHY driver sometimes not binding to the device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 29 Dec 2018 16:44:38 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Norbert Jurkeit <norbert.jurkeit@....de>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Frank Crawford <frank@...wford.emu.id.au>
Subject: Re: [PATCH net] net: phy: replace preliminary fix for PHY driver
 sometimes not binding to the device

On 29.12.2018 16:31, Norbert Jurkeit wrote:
> Am 29.12.18 um 14:55 schrieb Heiner Kallweit:
>> Great, then let's go for one more test. Could you apply the following to 4.19 and start in a fail scenario?
>> I would be interested in the additional dmesg line, just grep for "hk:".
>>
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
>> index 99bc3de90..20457c4e6 100644
>> --- a/drivers/net/ethernet/realtek/r8169.c
>> +++ b/drivers/net/ethernet/realtek/r8169.c
>> @@ -7048,6 +7048,7 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>>       new_bus->name = "r8169";
>>       new_bus->priv = tp;
>>       new_bus->parent = &pdev->dev;
>> +    new_bus->phy_mask = GENMASK(31, 1);
>>       new_bus->irq[0] = PHY_IGNORE_INTERRUPT;
>>       snprintf(new_bus->id, MII_BUS_ID_SIZE, "r8169-%x",
>>            PCI_DEVID(pdev->bus->number, pdev->devfn));
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 51990002d..b92699397 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -793,6 +793,8 @@ struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
>>       if (r)
>>           return ERR_PTR(r);
>>   +    pr_info("hk: addr = %d, phyid = 0x%08x, bmcr = 0x%x\n", addr, phy_id, mdiobus_read(bus, addr, 0));
>> +
>>       /* If the phy_id is mostly Fs, there is no device there */
>>       if ((phy_id & 0x1fffffff) == 0x1fffffff)
>>           return ERR_PTR(-ENODEV);
> 
> Damn. I applied the patch to mainline kernel 4.19.10 because most object files already exist and this kernel is known to fail without patches. Although not intended the above patch makes it work, i.e. the link comes up at the end of the boot process. FWIW, the dmesg lines are:
> 
I don't think this patch can have any impact on the issue. Maybe WoL is still active from previous test?
Manual WoL settings may survive a reboot, you can disable WoL by "ethtool -s <if> wol d".

> [    4.674485] libphy: hk: addr = 0, phyid = 0x001cc912, bmcr = 0x800

That's interesting. bmcr value states that PHY is powered down, but still the correct phyid is read.
Having said that it should not be the cause of the issue.

> [    4.682743] libphy: hk: addr = 0, phyid = 0x001cc913, bmcr = 0x1000
> 
> Please remember that my PC has also a 2. NIC of type RTL8169 which works fine.
> 
> I will try again with newly released kernel 4.19.13, but this will take longer and I can't promise better results.
> 
> 
It should make no difference whether you go with 4.10 or 4.13.
What could be helpful in addition: I provided a patch with some debug output in comment 106
in the bug ticket (https://bugzilla.redhat.com/show_bug.cgi?id=1650984).
If you could apply this, trigger a fail scenario, and attach the full dmesg to the bug ticket.

Thanks a lot!