Re: [PATCH net-next 1/2] net: phy: use phy_id_mask value zero for exact match

[Florian Fainelli <f.fainelli@gmail.com>]

On 11/7/18 12:53 PM, Heiner Kallweit wrote:
> A phy_id_mask value zero means every PHYID matches, therefore
> value zero isn't used. So we can safely redefine the semantics
> of value zero to mean "exact match". This allows to avoid some
> boilerplate code in PHY driver configs.

Having run recently into some ethtool quirkyness about how masks are
supposed to be specified between ntuple/nfc, where the meaning of 0 is
either don't care or match, I would rather we stick with the current
behavior where every bit set to 0 is a don't care and bits set t 1 are not.

Maybe we can find a clever way with a macro to specify only the PHY OUI
and compute a suitable mask automatically?

> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/phy/phy_device.c | 21 +++++++++++++++------
>  include/linux/phy.h          |  2 +-
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index ab33d1777..d165a2c82 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -483,15 +483,24 @@ static int phy_bus_match(struct device *dev, struct device_driver *drv)
>  			if (!(phydev->c45_ids.devices_in_package & (1 << i)))
>  				continue;
>  
> -			if ((phydrv->phy_id & phydrv->phy_id_mask) ==
> -			    (phydev->c45_ids.device_ids[i] &
> -			     phydrv->phy_id_mask))
> -				return 1;
> +			if (!phydrv->phy_id_mask) {
> +				if (phydrv->phy_id ==
> +				    phydev->c45_ids.device_ids[i])
> +					return 1;
> +			} else {
> +				if ((phydrv->phy_id & phydrv->phy_id_mask) ==
> +				    (phydev->c45_ids.device_ids[i] &
> +				     phydrv->phy_id_mask))
> +					return 1;
> +			}
>  		}
>  		return 0;
>  	} else {
> -		return (phydrv->phy_id & phydrv->phy_id_mask) ==
> -			(phydev->phy_id & phydrv->phy_id_mask);
> +		if (!phydrv->phy_id_mask)
> +			return phydrv->phy_id == phydev->phy_id;
> +		else
> +			return (phydrv->phy_id & phydrv->phy_id_mask) ==
> +			       (phydev->phy_id & phydrv->phy_id_mask);
>  	}
>  }
>  
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 2090277ea..e30ca2fdd 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -500,7 +500,7 @@ struct phy_driver {
>  	struct mdio_driver_common mdiodrv;
>  	u32 phy_id;
>  	char *name;
> -	u32 phy_id_mask;
> +	u32 phy_id_mask; /* value 0 means exact match */
>  	const unsigned long * const features;
>  	u32 flags;
>  	const void *driver_data;
> 


-- 
Florian