diff mbox series

[net-next,v1,5/7] net: usb: asix: add error handling for asix_mdio_* functions

Message ID 20210604134244.2467-6-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Headers show
Series port asix ax88772 to the PHYlib | expand

Commit Message

Oleksij Rempel June 4, 2021, 1:42 p.m. UTC 
This usb devices can be removed at any time, so we need to forward
correct error value if device was detached.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/usb/asix_common.c | 38 +++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 11 deletions(-)

Comments

Andrew Lunn June 4, 2021, 11:31 p.m. UTC | #1 
> -void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
> +static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,
> +			     int val)
>  {
>  	struct usbnet *dev = netdev_priv(netdev);
>  	__le16 res = cpu_to_le16(val);
> @@ -517,13 +522,24 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
>  	} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
>  	if (ret == -ENODEV) {
>  		mutex_unlock(&dev->phy_mutex);
> -		return;
> +		return ret;

Now that you have added an out: it might be better to use a goto?

Otherwise

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

    Andrew
Oleksij Rempel June 7, 2021, 7:55 a.m. UTC | #2 
On Sat, Jun 05, 2021 at 01:31:14AM +0200, Andrew Lunn wrote:
> > -void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
> > +static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,
> > +			     int val)
> >  {
> >  	struct usbnet *dev = netdev_priv(netdev);
> >  	__le16 res = cpu_to_le16(val);
> > @@ -517,13 +522,24 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
> >  	} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
> >  	if (ret == -ENODEV) {
> >  		mutex_unlock(&dev->phy_mutex);
> > -		return;
> > +		return ret;
> 
> Now that you have added an out: it might be better to use a goto?

ack, done

> Otherwise
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew
> 
>
diff mbox series

Patch

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 6b94c27576b7..7cce8f7d79b6 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -484,18 +484,23 @@  int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
 		return ret;
 	}
 
-	asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
-				(__u16)loc, 2, &res, 0);
-	asix_set_hw_mii(dev, 0);
+	ret = asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id, (__u16)loc, 2,
+			    &res, 0);
+	if (ret < 0)
+		goto out;
+
+	ret = asix_set_hw_mii(dev, 0);
+out:
 	mutex_unlock(&dev->phy_mutex);
 
 	netdev_dbg(dev->net, "asix_mdio_read() phy_id=0x%02x, loc=0x%02x, returns=0x%04x\n",
 			phy_id, loc, le16_to_cpu(res));
 
-	return le16_to_cpu(res);
+	return ret < 0 ? ret : le16_to_cpu(res);
 }
 
-void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
+static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,
+			     int val)
 {
 	struct usbnet *dev = netdev_priv(netdev);
 	__le16 res = cpu_to_le16(val);
@@ -517,13 +522,24 @@  void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
 	} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
 	if (ret == -ENODEV) {
 		mutex_unlock(&dev->phy_mutex);
-		return;
+		return ret;
 	}
 
-	asix_write_cmd(dev, AX_CMD_WRITE_MII_REG, phy_id,
-		       (__u16)loc, 2, &res, 0);
-	asix_set_hw_mii(dev, 0);
+	ret = asix_write_cmd(dev, AX_CMD_WRITE_MII_REG, phy_id, (__u16)loc, 2,
+			     &res, 0);
+	if (ret < 0)
+		goto out;
+
+	ret = asix_set_hw_mii(dev, 0);
+out:
 	mutex_unlock(&dev->phy_mutex);
+
+	return ret < 0 ? ret : 0;
+}
+
+void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
+{
+	__asix_mdio_write(netdev, phy_id, loc, val);
 }
 
 /* MDIO read and write wrappers for phylib */
@@ -535,8 +551,8 @@  int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
 
 int asix_mdio_bus_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
 {
-	asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id, regnum, val);
-	return 0;
+	return __asix_mdio_write(((struct usbnet *)bus->priv)->net, phy_id,
+				 regnum, val);
 }
 
 int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)