From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH net-next 1/2] net: phy: use phy_id_mask value zero for exact match Date: Thu, 8 Nov 2018 11:44:26 -0800 Message-ID: <827c53cd-c9f5-7b1c-84fd-af1a49317fe7@gmail.com> References: <08d52675-f308-4ebb-4ec4-f6c7ac0b6b06@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: "netdev@vger.kernel.org" To: Heiner Kallweit , Andrew Lunn , David Miller Return-path: Received: from mail-yb1-f193.google.com ([209.85.219.193]:39391 "EHLO mail-yb1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725723AbeKIFVc (ORCPT ); Fri, 9 Nov 2018 00:21:32 -0500 Received: by mail-yb1-f193.google.com with SMTP id w17-v6so1346113ybl.6 for ; Thu, 08 Nov 2018 11:44:35 -0800 (PST) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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 > --- > 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