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 <[email protected]>
> ---
>  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

Reply via email to