netdev - Re: [PATCH net-next 2/3] net: phy: realtek: use generic MDIO constants

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]

Date: Thu, 8 Feb 2024 08:18:56 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Marek Behún <kabel@...nel.org>,
 Russell King - ARM Linux <linux@...linux.org.uk>,
 Jakub Kicinski <kuba@...nel.org>, David Miller <davem@...emloft.net>,
 Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/3] net: phy: realtek: use generic MDIO
 constants

On 07.02.2024 23:31, Andrew Lunn wrote:
>>>>>>> @@ -674,11 +666,11 @@ static int rtl822x_get_features(struct phy_device *phydev)
>>>>>>>  		return val;
>>>>>>>  
>>>>>>>  	linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>>>>>>> -			 phydev->supported, val & RTL_SUPPORTS_2500FULL);
>>>>>>> +			 phydev->supported, val & MDIO_PMA_SPEED_2_5G);
>>>>>>>  	linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>>>>>>> -			 phydev->supported, val & RTL_SUPPORTS_5000FULL);
>>>>>>> +			 phydev->supported, val & MDIO_PMA_SPEED_5G);
>>>>>>>  	linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
>>>>>>> -			 phydev->supported, val & RTL_SUPPORTS_10000FULL);
>>>>>>> +			 phydev->supported, val & MDIO_SPEED_10G);
>>>>>>
>>>>>> Now that this only using generic constants, should it move into mdio.h
>>>>>> as a shared helper? Is this a standard register defined in 802.3, just
>>>>>> at a different address?
>>>>>>
>>>>> This is register 1.4 (PMA/PMD speed ability), mapped to a vendor-specific
>>>>> register. There's very few users of this register, and nothing where such
>>>>> a helper could be reused.
>>>>>
>>
>> When looking a little closer at creating a helper for it, I stumbled across
>> the following. This register just states that the PHY can operate at a certain
>> speed, it leaves open which mode(s) are supported at that speed.
> 
> O.K, yes, i agree. All it says is speed, nothing more.
> 
> But that also means this driver should not really be doing this. Is
> there a full list of registers which are implemented? Is there a way
> to implement genphy_c45_pma_read_abilities(), or at least the subset
> needed for this device? That appears to be MDIO_MMD_PMAPMD:MDIO_PMA_NG_EXTABLE and
> MDIO_MMD_PMAPMD:MDIO_PMA_EXTABLE?
> 
I don't have access to Realtek datasheets, and AFAIK the datasheets don't document
the mapping of c45 standard registers to vendor-specific registers. But let me
check with my contact at Realtek.

> 	Andrew
Heiner