netdev - Re: [PATCH net-next] r8169: deprecate support for RTL_GIGA_MAC_VER

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 for Android: free password hash cracker in your pocket

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

Message-ID: <facbe27f-3b5e-6dc2-8837-e3989b178847@gmail.com>
Date:   Sun, 10 Jan 2021 12:58:40 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     David Miller <davem@...emloft.net>,
        Realtek linux nic maintainers <nic_swsd@...ltek.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] r8169: deprecate support for RTL_GIGA_MAC_VER_27

On 10.01.2021 03:17, Jakub Kicinski wrote:
> On Fri, 8 Jan 2021 13:24:16 +0100 Heiner Kallweit wrote:
>> RTL8168dp is ancient anyway, and I haven't seen any trace of its early
>> version 27 yet. This chip versions needs quite some special handling,
>> therefore it would facilitate driver maintenance if support for it
>> could be dropped. For now just disable detection of this chip version.
>> If nobody complains we can remove support for it in the near future.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
>> ---
>>  drivers/net/ethernet/realtek/r8169_main.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>> index e72d8f3ae..f94965615 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -1962,7 +1962,11 @@ static enum mac_version rtl8169_get_mac_version(u16 xid, bool gmii)
>>  		{ 0x7c8, 0x280,	RTL_GIGA_MAC_VER_26 },
>>  
>>  		/* 8168DP family. */
>> -		{ 0x7cf, 0x288,	RTL_GIGA_MAC_VER_27 },
>> +		/* It seems this early RTL8168dp version never made it to
>> +		 * the wild. Let's see whether somebody complains, if not
>> +		 * we'll remove support for this chip version completely.
>> +		 * { 0x7cf, 0x288,      RTL_GIGA_MAC_VER_27 },
>> +		 */
>>  		{ 0x7cf, 0x28a,	RTL_GIGA_MAC_VER_28 },
>>  		{ 0x7cf, 0x28b,	RTL_GIGA_MAC_VER_31 },
>>  
> 
> No objection to deprecating the support (although quick grep does not
> reveal the special handling you speak of), but would it make sense to
> also print some message to save the potential user out there debug time?

Version 27 has it's own way of handling MDIO access, we could get rid of
r8168dp_1_mdio_access(), r8168dp_1_mdio_write(), r8168dp_1_mdio_read(),
and also of the chip-specific PHY config rtl8168d_3_hw_phy_config().

> Something like:
> 
> 	dev_err(dev, "support for device has been removed please contact <email>");
> 
So far we'd emit the following error in dmesg: "unknown chip XID 288".
I agree that we should add a hint to contact the maintainers, this may help
also in case somebody has a system with a new, not yet supported chip version.

> ?
>