[PATCH net-next 0/3] net: Fix return value check for get_phy_device()

[PATCH net-next 0/3] net: Fix return value check for get_phy_device()

[Ruan Jinjie]

The get_phy_device() function returns error pointers and never
returns NULL. Update the checks accordingly.

And get_phy_device() returns -EIO on bus access error and -ENOMEM
on kzalloc failure in addition to -ENODEV, return PTR_ERR is more
sensible.

Ruan Jinjie (3):
  net: mdio: Fix return value check for get_phy_device()
  amd-xgbe: Return proper error code for get_phy_device()
  net: hisilicon: hns: Fix return value check for get_phy_device()

 drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c       | 2 +-
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 4 ++--
 drivers/net/mdio/mdio-xgene.c                     | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH net-next 1/3] net: mdio: Fix return value check for get_phy_device()

[Ruan Jinjie]

The get_phy_device() function returns error pointers and never
returns NULL. Update the checks accordingly.

Fixes: 43b3cf6634a4 ("drivers: net: phy: xgene: Add MDIO driver")
Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
---
 drivers/net/mdio/mdio-xgene.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mdio/mdio-xgene.c b/drivers/net/mdio/mdio-xgene.c
index 683e8f8319ab..4e097ce2a7fd 100644
--- a/drivers/net/mdio/mdio-xgene.c
+++ b/drivers/net/mdio/mdio-xgene.c
@@ -265,7 +265,7 @@ struct phy_device *xgene_enet_phy_register(struct mii_bus *bus, int phy_addr)
 	struct phy_device *phy_dev;
 
 	phy_dev = get_phy_device(bus, phy_addr, false);
-	if (!phy_dev || IS_ERR(phy_dev))
+	if (IS_ERR(phy_dev))
 		return NULL;
 
 	if (phy_device_register(phy_dev))
-- 
2.34.1

[PATCH net-next 2/3] amd-xgbe: Return proper error code for get_phy_device()

[Ruan Jinjie]

get_phy_device() returns -EIO on bus access error and -ENOMEM
on kzalloc failure in addition to -ENODEV, just return -ENODEV is not
sensible, use PTR_ERR(phydev) to fix the issue.

Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
index 6a716337f48b..2f0a014ffc72 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
@@ -1090,7 +1090,7 @@ static int xgbe_phy_find_phy_device(struct xgbe_prv_data *pdata)
 				(phy_data->phydev_mode == XGBE_MDIO_MODE_CL45));
 	if (IS_ERR(phydev)) {
 		netdev_err(pdata->netdev, "get_phy_device failed\n");
-		return -ENODEV;
+		return PTR_ERR(phydev);
 	}
 	netif_dbg(pdata, drv, pdata->netdev, "external PHY id is %#010x\n",
 		  phydev->phy_id);
-- 
2.34.1

[PATCH net-next 3/3] net: hisilicon: hns: Fix return value check for get_phy_device()

[Ruan Jinjie]

The get_phy_device() function returns error pointers and never
returns NULL. Update the checks accordingly.

Fixes: 1d1afa2ebf82 ("net: hns: register phy device in each mac initial sequence")
Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index 928d934cb21a..13b5ee548744 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -705,8 +705,8 @@ hns_mac_register_phydev(struct mii_bus *mdio, struct hns_mac_cb *mac_cb,
 		return -ENODATA;
 
 	phy = get_phy_device(mdio, addr, is_c45);
-	if (!phy || IS_ERR(phy))
-		return -EIO;
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
 
 	phy->irq = mdio->irq[addr];
 
-- 
2.34.1

Re: [PATCH net-next 0/3] net: Fix return value check for get_phy_device()

[Leon Romanovsky]

On Thu, Aug 17, 2023 at 03:39:57PM +0800, Ruan Jinjie wrote:
> The get_phy_device() function returns error pointers and never
> returns NULL. Update the checks accordingly.
> 
> And get_phy_device() returns -EIO on bus access error and -ENOMEM
> on kzalloc failure in addition to -ENODEV, return PTR_ERR is more
> sensible.
> 
> Ruan Jinjie (3):
>   net: mdio: Fix return value check for get_phy_device()
>   amd-xgbe: Return proper error code for get_phy_device()
>   net: hisilicon: hns: Fix return value check for get_phy_device()
> 
>  drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c       | 2 +-
>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 4 ++--
>  drivers/net/mdio/mdio-xgene.c                     | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH net-next 2/3] amd-xgbe: Return proper error code for get_phy_device()

[Shyam Sundar S K]

On 8/17/2023 1:09 PM, Ruan Jinjie wrote:
> get_phy_device() returns -EIO on bus access error and -ENOMEM
> on kzalloc failure in addition to -ENODEV, just return -ENODEV is not
> sensible, use PTR_ERR(phydev) to fix the issue.
> 
> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>

Looks good to me.

Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

Thanks,
Shyam

> ---
>  drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> index 6a716337f48b..2f0a014ffc72 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> @@ -1090,7 +1090,7 @@ static int xgbe_phy_find_phy_device(struct xgbe_prv_data *pdata)
>  				(phy_data->phydev_mode == XGBE_MDIO_MODE_CL45));
>  	if (IS_ERR(phydev)) {
>  		netdev_err(pdata->netdev, "get_phy_device failed\n");
> -		return -ENODEV;
> +		return PTR_ERR(phydev);
>  	}
>  	netif_dbg(pdata, drv, pdata->netdev, "external PHY id is %#010x\n",
>  		  phydev->phy_id);
> 

Re: [PATCH net-next 1/3] net: mdio: Fix return value check for get_phy_device()

[Andrew Lunn]

On Thu, Aug 17, 2023 at 03:39:58PM +0800, Ruan Jinjie wrote:
> The get_phy_device() function returns error pointers and never
> returns NULL. Update the checks accordingly.
> 
> Fixes: 43b3cf6634a4 ("drivers: net: phy: xgene: Add MDIO driver")
> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>

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

    Andrew

Re: [PATCH net-next 2/3] amd-xgbe: Return proper error code for get_phy_device()

[Andrew Lunn]

On Thu, Aug 17, 2023 at 03:39:59PM +0800, Ruan Jinjie wrote:
> get_phy_device() returns -EIO on bus access error and -ENOMEM
> on kzalloc failure in addition to -ENODEV, just return -ENODEV is not
> sensible, use PTR_ERR(phydev) to fix the issue.

Rather than say 'not sensible', it would be better to say 'Best
practice is to return these error codes'.

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


    Andrew

---
pw-bot: cr

Re: [PATCH net-next 3/3] net: hisilicon: hns: Fix return value check for get_phy_device()

[Andrew Lunn]

On Thu, Aug 17, 2023 at 03:40:00PM +0800, Ruan Jinjie wrote:
> The get_phy_device() function returns error pointers and never
> returns NULL. Update the checks accordingly.
> 
> Fixes: 1d1afa2ebf82 ("net: hns: register phy device in each mac initial sequence")
> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>

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

    Andrew

Re: [PATCH net-next 2/3] amd-xgbe: Return proper error code for get_phy_device()

[Ruan Jinjie]

On 2023/8/17 20:28, Andrew Lunn wrote:
> On Thu, Aug 17, 2023 at 03:39:59PM +0800, Ruan Jinjie wrote:
>> get_phy_device() returns -EIO on bus access error and -ENOMEM
>> on kzalloc failure in addition to -ENODEV, just return -ENODEV is not
>> sensible, use PTR_ERR(phydev) to fix the issue.
> 
> Rather than say 'not sensible', it would be better to say 'Best
> practice is to return these error codes'.

Thank you! I'll watch the wording next time.

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