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

Message-ID: <40337728-a364-4955-a782-6ce4e178a097@lunn.ch>
Date: Wed, 7 Feb 2024 23:31:57 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Heiner Kallweit <hkallweit1@...il.com>
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

> >>>>> @@ -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?

	Andrew