Re: [PATCH net-next v2 1/2] net: phy: c45 scanning: Don't consider -ENODEV fatal

[Charles Perry <charles.perry@microchip.com>]

On Sun, Feb 04, 2024 at 05:14:14PM -0600, Andrew Lunn wrote:
> When scanning the MDIO bus for C22 devices, the driver returning
> -ENODEV is not considered fatal, it just indicates the MDIO bus master
> knows there is no device at that address, maybe because of hardware
> limitation.
> 
> Make the C45 scan code act on -ENODEV the same way, to make C22 and
> C45 more uniform.
> 
> It is expected all reads for a given address will return -ENODEV, so
> within get_phy_c45_ids() only the first place a read occurs has been
> changed.
> 
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/phy/phy_device.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 52828d1c64f7..962ab53c23ff 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -780,7 +780,7 @@ static int get_phy_c45_devs_in_pkg(struct mii_bus *bus, int addr, int dev_addr,
>   * and identifiers in @c45_ids.
>   *
>   * Returns zero on success, %-EIO on bus access error, or %-ENODEV if
> - * the "devices in package" is invalid.
> + * the "devices in package" is invalid or no device responds.
>   */
>  static int get_phy_c45_ids(struct mii_bus *bus, int addr,
>  			   struct phy_c45_device_ids *c45_ids)
> @@ -803,7 +803,11 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr,
>  			 */
>  			ret = phy_c45_probe_present(bus, addr, i);
>  			if (ret < 0)
> -				return -EIO;
> +				/* returning -ENODEV doesn't stop bus
> +				 * scanning
> +				 */
> +				return (phy_reg == -EIO ||
> +					phy_reg == -ENODEV) ? -ENODEV : -EIO;

Hello Andrew,

Did you meant "ret" instead of "phy_reg" here?

"phy_reg" doesn't make sense here because you would have already returned
in the previous iteration of the loop.

Thanks,
Charles

>  
>  			if (!ret)
>  				continue;
> 
> -- 
> 2.43.0
> 
>