Re: [PATCH 3/4] net: ksz9477: propagate phy read error

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 08.09.23 12:16, Gerald Loacker wrote:
> In case of an error we should not return an arbitrary value, propagate the
> error code instead.
> Fix return value in case of address error.
> 
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
> ---
>  drivers/net/ksz9477.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ksz9477.c b/drivers/net/ksz9477.c
> index 9665e0f723..d664b7cf01 100644
> --- a/drivers/net/ksz9477.c
> +++ b/drivers/net/ksz9477.c
> @@ -29,14 +29,15 @@ static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg)
>  {
>  	struct device *dev = ds->dev;
>  	struct ksz_switch *priv = dev_get_priv(dev);
> -	u16 val = 0xffff;
> +	u16 val;
> +	int ret;
>  
>  	if (addr >= priv->phy_port_cnt)
> -		return val;
> +		return -EINVAL;

Looks sensible IMO, but shouldn't we do the same in dsa_slave_phy_read
if no callback is defined?

@Oleksij, why did you decide for 0xffff instead of a negative error code?


>  
> -	ksz_pread16(priv, addr, 0x100 + (reg << 1), &val);
> +	ret = ksz_pread16(priv, addr, 0x100 + (reg << 1), &val);
>  
> -	return val;
> +	return (ret < 0) ? ret : val;
>  }
>  
>  static int ksz9477_phy_write16(struct dsa_switch *ds, int addr, int reg,
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |