[PATCH net-next] net: phy: at803x: simplify custom phy id matching

[PATCH net-next] net: phy: at803x: simplify custom phy id matching

[Russell King]

The at803x driver contains a function, at803x_match_phy_id(), which
tests whether the PHY ID matches the value passed, comparing phy_id
with phydev->phy_id and testing all bits that in the driver's mask.

This is the same test that is used to match the driver, with phy_id
replaced with the driver specified ID, phydev->drv->phy_id.

Hence, we already know the value of the bits being tested if we look
at phydev->drv->phy_id directly, and we do not require a complicated
test to check them. Test directly against phydev->drv->phy_id instead.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/at803x.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 5d62b85a4024..0790ffcd3db6 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -532,12 +532,6 @@ static int at8031_register_regulators(struct phy_device *phydev)
 	return 0;
 }
 
-static bool at803x_match_phy_id(struct phy_device *phydev, u32 phy_id)
-{
-	return (phydev->phy_id & phydev->drv->phy_id_mask)
-		== (phy_id & phydev->drv->phy_id_mask);
-}
-
 static int at803x_parse_dt(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
@@ -602,8 +596,8 @@ static int at803x_parse_dt(struct phy_device *phydev)
 		 *   to the AR8030 so there might be a good chance it works on
 		 *   the AR8030 too.
 		 */
-		if (at803x_match_phy_id(phydev, ATH8030_PHY_ID) ||
-		    at803x_match_phy_id(phydev, ATH8035_PHY_ID)) {
+		if (phydev->drv->phy_id == ATH8030_PHY_ID ||
+		    phydev->drv->phy_id == ATH8035_PHY_ID) {
 			priv->clk_25m_reg &= AT8035_CLK_OUT_MASK;
 			priv->clk_25m_mask &= AT8035_CLK_OUT_MASK;
 		}
@@ -631,7 +625,7 @@ static int at803x_parse_dt(struct phy_device *phydev)
 	/* Only supported on AR8031/AR8033, the AR8030/AR8035 use strapping
 	 * options.
 	 */
-	if (at803x_match_phy_id(phydev, ATH8031_PHY_ID)) {
+	if (phydev->drv->phy_id == ATH8031_PHY_ID) {
 		if (of_property_read_bool(node, "qca,keep-pll-enabled"))
 			priv->flags |= AT803X_KEEP_PLL_ENABLED;
 
@@ -676,7 +670,7 @@ static int at803x_probe(struct phy_device *phydev)
 	 * Switch to the copper page, as otherwise we read
 	 * the PHY capabilities from the fiber side.
 	 */
-	if (at803x_match_phy_id(phydev, ATH8031_PHY_ID)) {
+	if (phydev->drv->phy_id == ATH8031_PHY_ID) {
 		phy_lock_mdio_bus(phydev);
 		ret = at803x_write_page(phydev, AT803X_PAGE_COPPER);
 		phy_unlock_mdio_bus(phydev);
@@ -820,7 +814,7 @@ static int at803x_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	if (at803x_match_phy_id(phydev, ATH8031_PHY_ID)) {
+	if (phydev->drv->phy_id == ATH8031_PHY_ID) {
 		ret = at8031_pll_config(phydev);
 		if (ret < 0)
 			return ret;
-- 
2.20.1

Re: [PATCH net-next] net: phy: at803x: simplify custom phy id matching

[Fabio Estevam]

Hi Russell,

On Tue, Jul 20, 2021 at 10:40 AM Russell King
<rmk+kernel@armlinux.org.uk> wrote:
>
> The at803x driver contains a function, at803x_match_phy_id(), which
> tests whether the PHY ID matches the value passed, comparing phy_id
> with phydev->phy_id and testing all bits that in the driver's mask.
>
> This is the same test that is used to match the driver, with phy_id
> replaced with the driver specified ID, phydev->drv->phy_id.
>
> Hence, we already know the value of the bits being tested if we look
> at phydev->drv->phy_id directly, and we do not require a complicated
> test to check them. Test directly against phydev->drv->phy_id instead.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Yes, this makes it simpler:

Reviewed-by: Fabio Estevam <festevam@gmail.com>

Re: [PATCH net-next] net: phy: at803x: simplify custom phy id matching

[Andrew Lunn]

On Tue, Jul 20, 2021 at 02:33:49PM +0100, Russell King wrote:
> The at803x driver contains a function, at803x_match_phy_id(), which
> tests whether the PHY ID matches the value passed, comparing phy_id
> with phydev->phy_id and testing all bits that in the driver's mask.
> 
> This is the same test that is used to match the driver, with phy_id
> replaced with the driver specified ID, phydev->drv->phy_id.
> 
> Hence, we already know the value of the bits being tested if we look
> at phydev->drv->phy_id directly, and we do not require a complicated
> test to check them. Test directly against phydev->drv->phy_id instead.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next] net: phy: at803x: simplify custom phy id matching

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Tue, 20 Jul 2021 14:33:49 +0100 you wrote:
> The at803x driver contains a function, at803x_match_phy_id(), which
> tests whether the PHY ID matches the value passed, comparing phy_id
> with phydev->phy_id and testing all bits that in the driver's mask.
> 
> This is the same test that is used to match the driver, with phy_id
> replaced with the driver specified ID, phydev->drv->phy_id.
> 
> [...]

Here is the summary with links:
  - [net-next] net: phy: at803x: simplify custom phy id matching
    https://git.kernel.org/netdev/net-next/c/8887ca5474bd

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next] net: phy: at803x: simplify custom phy id matching

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3858 bytes --]

Hi Russell,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Russell-King/net-phy-at803x-simplify-custom-phy-id-matching/20210720-214010
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git e93abb840a2c356ed2809c31fcedb058601ac2e4
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/2f590b7f42d30d029ec56cec8429effe8505cfb9
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Russell-King/net-phy-at803x-simplify-custom-phy-id-matching/20210720-214010
        git checkout 2f590b7f42d30d029ec56cec8429effe8505cfb9
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/net/phy/at803x.c: In function 'at803x_get_features':
>> drivers/net/phy/at803x.c:706:7: error: implicit declaration of function 'at803x_match_phy_id' [-Werror=implicit-function-declaration]
     706 |  if (!at803x_match_phy_id(phydev, ATH8031_PHY_ID))
         |       ^~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/at803x_match_phy_id +706 drivers/net/phy/at803x.c

2318ca8aef3877 Michael Walle 2020-01-30  697  
b856150c8098f1 David Bauer   2021-06-27  698  static int at803x_get_features(struct phy_device *phydev)
b856150c8098f1 David Bauer   2021-06-27  699  {
b856150c8098f1 David Bauer   2021-06-27  700  	int err;
b856150c8098f1 David Bauer   2021-06-27  701  
b856150c8098f1 David Bauer   2021-06-27  702  	err = genphy_read_abilities(phydev);
b856150c8098f1 David Bauer   2021-06-27  703  	if (err)
b856150c8098f1 David Bauer   2021-06-27  704  		return err;
b856150c8098f1 David Bauer   2021-06-27  705  
b856150c8098f1 David Bauer   2021-06-27 @706  	if (!at803x_match_phy_id(phydev, ATH8031_PHY_ID))
b856150c8098f1 David Bauer   2021-06-27  707  		return 0;
b856150c8098f1 David Bauer   2021-06-27  708  
b856150c8098f1 David Bauer   2021-06-27  709  	/* AR8031/AR8033 have different status registers
b856150c8098f1 David Bauer   2021-06-27  710  	 * for copper and fiber operation. However, the
b856150c8098f1 David Bauer   2021-06-27  711  	 * extended status register is the same for both
b856150c8098f1 David Bauer   2021-06-27  712  	 * operation modes.
b856150c8098f1 David Bauer   2021-06-27  713  	 *
b856150c8098f1 David Bauer   2021-06-27  714  	 * As a result of that, ESTATUS_1000_XFULL is set
b856150c8098f1 David Bauer   2021-06-27  715  	 * to 1 even when operating in copper TP mode.
b856150c8098f1 David Bauer   2021-06-27  716  	 *
b856150c8098f1 David Bauer   2021-06-27  717  	 * Remove this mode from the supported link modes,
b856150c8098f1 David Bauer   2021-06-27  718  	 * as this driver currently only supports copper
b856150c8098f1 David Bauer   2021-06-27  719  	 * operation.
b856150c8098f1 David Bauer   2021-06-27  720  	 */
b856150c8098f1 David Bauer   2021-06-27  721  	linkmode_clear_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
b856150c8098f1 David Bauer   2021-06-27  722  			   phydev->supported);
b856150c8098f1 David Bauer   2021-06-27  723  	return 0;
b856150c8098f1 David Bauer   2021-06-27  724  }
b856150c8098f1 David Bauer   2021-06-27  725  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 73470 bytes --]

Re: [PATCH net-next] net: phy: at803x: simplify custom phy id matching

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1995 bytes --]

On 20.07.2021 14:33:49, Russell King wrote:
> The at803x driver contains a function, at803x_match_phy_id(), which
> tests whether the PHY ID matches the value passed, comparing phy_id
> with phydev->phy_id and testing all bits that in the driver's mask.
> 
> This is the same test that is used to match the driver, with phy_id
> replaced with the driver specified ID, phydev->drv->phy_id.
> 
> Hence, we already know the value of the bits being tested if we look
> at phydev->drv->phy_id directly, and we do not require a complicated
> test to check them. Test directly against phydev->drv->phy_id instead.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> ---
>  drivers/net/phy/at803x.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 5d62b85a4024..0790ffcd3db6 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -532,12 +532,6 @@ static int at8031_register_regulators(struct phy_device *phydev)
>  	return 0;
>  }
>  
> -static bool at803x_match_phy_id(struct phy_device *phydev, u32 phy_id)
> -{
> -	return (phydev->phy_id & phydev->drv->phy_id_mask)
> -		== (phy_id & phydev->drv->phy_id_mask);
> -}
> -

Seems you've missed a conversion:

| net/phy/at803x.c: In function ‘at803x_get_features’:                     
| net/phy/at803x.c:706:7: error: implicit declaration of function ‘at803x_match_phy_id’ [-Werror=implicit-function-declaration]                                                                            
|   706 |  if (!at803x_match_phy_id(phydev, ATH8031_PHY_ID))
|       |       ^~~~~~~~~~~~~~~~~~~

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH net-next] net: phy: at803x: simplify custom phy id matching

[Russell King (Oracle)]

On Wed, Jul 21, 2021 at 12:22:39PM +0200, Marc Kleine-Budde wrote:
> Seems you've missed a conversion:
> 
> | net/phy/at803x.c: In function ‘at803x_get_features’:                     
> | net/phy/at803x.c:706:7: error: implicit declaration of function ‘at803x_match_phy_id’ [-Werror=implicit-function-declaration]                                                                            
> |   706 |  if (!at803x_match_phy_id(phydev, ATH8031_PHY_ID))
> |       |       ^~~~~~~~~~~~~~~~~~~

Sorry, yes, sadly.

net-next very quickly picked this patch despite it being the first
visibility on the mailing lists. Vladimir quickly noticed this and
sent a patch that we're waiting to be picked up.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!