netdev - Re: [PATCH net-next] net: phy: add helpers for comparing phy IDs

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: Fri, 19 May 2023 17:59:02 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Russell King <rmk+kernel@...linux.org.uk>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: phy: add helpers for comparing phy IDs

On Fri, May 19, 2023 at 02:03:59PM +0100, Russell King wrote:
> There are several places which open code comparing PHY IDs. Provide a
> couple of helpers to assist with this, using a slightly simpler test
> than the original:
> 
> - phy_id_compare() compares two arbitary PHY IDs and a mask of the
>   significant bits in the ID.
> - phydev_id_compare() compares the bound phydev with the specified
>   PHY ID, using the bound driver's mask.

Hi Russell

I think these are useful, but i'm wondering about naming. In the PHY
drivers we use these macros:

#define PHY_ID_MATCH_EXACT(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 0)
#define PHY_ID_MATCH_MODEL(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 4)
#define PHY_ID_MATCH_VENDOR(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 10)

when creating the tables. 

> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 3f81bb8dac44..2094d49025a7 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -637,7 +637,7 @@ static int ksz8051_ksz8795_match_phy_device(struct phy_device *phydev,
>  {
>  	int ret;
>  
> -	if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8051)
> +	if (!phy_id_compare(phydev->phy_id, PHY_ID_KSZ8051, MICREL_PHY_ID_MASK))
>  		return 0;

This could be phy_id_compare_model() 

phy_id_compare_exact() could be used in a number of places, eg.

vitesse.c:			(phydev->drv->phy_id == PHY_ID_VSC8234 ||
vitesse.c:			 phydev->drv->phy_id == PHY_ID_VSC8244 ||
vitesse.c:			 phydev->drv->phy_id == PHY_ID_VSC8572 ||
vitesse.c:			 phydev->drv->phy_id == PHY_ID_VSC8601) ?
motorcomm.c:	} else if (phydev->drv->phy_id == PHY_ID_YT8531S) {
marvell10g.c:	if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310)

etc.

	Andrew