linux-kernel - Re: [PATCH net-next v5 4/7] net: phy: aquantia: add essential functions to aqr105 driver

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] [day] [month] [year] [list]

Message-ID: <d944c0bd-a652-4bfe-b6e8-c264f5b36562@gmx.net>
Date: Sun, 23 Feb 2025 23:26:49 +0100
From: Hans-Frieder Vogt <hfdevel@....net>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>,
 Hans-Frieder Vogt via B4 Relay <devnull+hfdevel.gmx.net@...nel.org>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
 Russell King <linux@...linux.org.uk>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 FUJITA Tomonori <fujita.tomonori@...il.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v5 4/7] net: phy: aquantia: add essential
 functions to aqr105 driver

Hi Maxime,

On 23.02.2025 11.32, Maxime Chevallier wrote:
> Hi,
>
> On Sat, 22 Feb 2025 10:49:31 +0100
> Hans-Frieder Vogt via B4 Relay <devnull+hfdevel.gmx.net@...nel.org>
> wrote:
>
>> From: Hans-Frieder Vogt <hfdevel@....net>
>>
>> This patch makes functions that were provided for aqr107 applicable to
>> aqr105, or replaces generic functions with specific ones. Since the aqr105
>> was introduced before NBASE-T was defined (or 802.3bz), there are a number
>> of vendor specific registers involved in the definition of the
>> advertisement, in auto-negotiation and in the setting of the speed. The
>> functions have been written following the downstream driver for TN4010
>> cards with aqr105 PHY, and use code from aqr107 functions wherever it
>> seemed to make sense.
>>
>> Signed-off-by: Hans-Frieder Vogt <hfdevel@....net>
>> ---
>>   drivers/net/phy/aquantia/aquantia_main.c | 242 ++++++++++++++++++++++++++++++-
>>   1 file changed, 240 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
>> index 86b0e63de5d88fa1050919a8826bdbec4bbcf8ba..38c6cf7814da1fb9a4e715f242249eee15a3cc85 100644
>> --- a/drivers/net/phy/aquantia/aquantia_main.c
>> +++ b/drivers/net/phy/aquantia/aquantia_main.c
>> @@ -33,6 +33,9 @@
>>   #define PHY_ID_AQR115C	0x31c31c33
>>   #define PHY_ID_AQR813	0x31c31cb2
>>
>> +#define MDIO_AN_10GBT_CTRL_ADV_LTIM		BIT(0)
> This is a standard C45 definition, from :
> 45.2.7.10.15 10GBASE-T LD loop timing ability (7.32.0)
>
> So if you need this advertising capability, you should add that in the
> generic definitions for C45 registers in include/uapi/linux/mdio.h
Thanks. Wasn't aware this being a standard definition.

Wouldn't the definition
#define ADVERTISE_XNP                           BIT(12)
then need to go to include/uapi/linux/mii.h accordingly?
There, bit 12 is currently named ADVERTISE_RESV and commented as unused
(which it obviously is not, because it is used in
drivers/net/ethernet/sfc/falcon/mdio_10g.c
I think, for now, I will just do the same as in the falcon driver and
use ADVERTISE_RESV instead. Then it may be renamed later in all places.
>
> That being said, as it looks this is the first driver using this
> feature, do you actually need to advertise Loop Timing ability here ?
> I guess it comes from the vendor driver ?
you are right. The code just tries to replicate the vendor code.
However, I have now tested the driver without this flag and haven't
noticed any unusual behavior. So, I guess, it works indeed without.
I'll remove the flag in the next revision of the patch.
> Thanks,
>
> Maxime
Thanks as well,
Hans-Frieder