[v1] lan78xx: Preparations for PHYlink

[PATCH net-next v1 08/11] net: usb: lan78xx: Use function-specific label in lan78xx_mac_reset

Posted by Oleksij Rempel 1 year ago

Rename the generic `done` label to the function-specific
`mac_reset_done` label in `lan78xx_mac_reset`. This improves clarity and
aligns with best practices for error handling and cleanup labels.

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

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index c66e404f51ac..fdeb95db529b 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1604,16 +1604,16 @@ static int lan78xx_mac_reset(struct lan78xx_net *dev)
 	 */
 	ret = lan78xx_phy_wait_not_busy(dev);
 	if (ret < 0)
-		goto done;
+		goto mac_reset_done;
 
 	ret = lan78xx_read_reg(dev, MAC_CR, &val);
 	if (ret < 0)
-		goto done;
+		goto mac_reset_done;
 
 	val |= MAC_CR_RST_;
 	ret = lan78xx_write_reg(dev, MAC_CR, val);
 	if (ret < 0)
-		goto done;
+		goto mac_reset_done;
 
 	/* Wait for the reset to complete before allowing any further
 	 * MAC register accesses otherwise the MAC may lock up.
@@ -1621,16 +1621,16 @@ static int lan78xx_mac_reset(struct lan78xx_net *dev)
 	do {
 		ret = lan78xx_read_reg(dev, MAC_CR, &val);
 		if (ret < 0)
-			goto done;
+			goto mac_reset_done;
 
 		if (!(val & MAC_CR_RST_)) {
 			ret = 0;
-			goto done;
+			goto mac_reset_done;
 		}
 	} while (!time_after(jiffies, start_time + HZ));
 
 	ret = -ETIMEDOUT;
-done:
+mac_reset_done:
 	mutex_unlock(&dev->phy_mutex);
 
 	return ret;
-- 
2.39.5

Re: [PATCH net-next v1 08/11] net: usb: lan78xx: Use function-specific label in lan78xx_mac_reset

Posted by Jakub Kicinski 1 year ago

On Mon,  9 Dec 2024 14:07:48 +0100 Oleksij Rempel wrote:
> Rename the generic `done` label to the function-specific
> `mac_reset_done` label in `lan78xx_mac_reset`. This improves clarity and
> aligns with best practices for error handling and cleanup labels.

What rules do you refer to?
AFAIK the best practice for exits is to name the label after what
you're jumping to. In this case I'd use exit_unlock.

> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index c66e404f51ac..fdeb95db529b 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1604,16 +1604,16 @@ static int lan78xx_mac_reset(struct lan78xx_net *dev)
>  	 */
>  	ret = lan78xx_phy_wait_not_busy(dev);
>  	if (ret < 0)
> -		goto done;
> +		goto mac_reset_done;

...

>  	ret = -ETIMEDOUT;
> -done:
> +mac_reset_done:
>  	mutex_unlock(&dev->phy_mutex);
>  
>  	return ret;

Re: [PATCH net-next v1 08/11] net: usb: lan78xx: Use function-specific label in lan78xx_mac_reset

Posted by Andrew Lunn 1 year ago

On Mon, Dec 09, 2024 at 02:07:48PM +0100, Oleksij Rempel wrote:
> Rename the generic `done` label to the function-specific
> `mac_reset_done` label in `lan78xx_mac_reset`. This improves clarity and
> aligns with best practices for error handling and cleanup labels.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

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

    Andrew