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

[Oleksij Rempel <o.rempel@pengutronix.de>]

Hi,

On Fri, Sep 08, 2023 at 01:59:05PM +0200, Ahmad Fatoum wrote:
> 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?

This code imitate real MDIO bus access. If you trying to read at not
existing PHY address, you will get 0xffff instead of negative error.
May be it is better to add a comment :)

Regards,
Oleksij
-- 
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 |