[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

[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!