net: xilinx: axienet: Use common error handling code in axienet_mdio_write()

Commit Message

Markus Elfring Sept. 20, 2024, 11:01 a.m. UTC

From: Markus Elfring <[e-mail]>
Date: Fri, 20 Sep 2024 12:43:39 +0200
Subject: [PATCH] net: xilinx: axienet: Use common error handling code in axienet_mdio_write()

Add a label so that a bit of exception handling can be better reused
at the end of this function implementation.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <[e-mail]>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

--
2.46.0

Comments

Simon Horman Sept. 22, 2024, 5:05 p.m. UTC | #1

On Fri, Sep 20, 2024 at 01:01:45PM +0200, Markus Elfring wrote:
> From: Markus Elfring <[e-mail]>
> Date: Fri, 20 Sep 2024 12:43:39 +0200
> Subject: [PATCH] net: xilinx: axienet: Use common error handling code in axienet_mdio_write()
> 
> Add a label so that a bit of exception handling can be better reused
> at the end of this function implementation.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <[e-mail]>


Hi Markus,

This change seems reasonable to me.  However, I am assuming that as a
non-bug-fix, this is targeted at net-next.  And net-next is currently
closed for the v6.12 merge window.  Please consider reposting this patch
once net-next reopens.  That will occur after v6.12-rc1 has been released.
Which I expect to be about a week from now.

Also, for networking patches please tag non-bug fixes for
net-next (and bug fixes for net, being sure to include a Fixes tag).

	Subject: [PATCH net-next] ...

Please see https://docs.kernel.org/process/maintainer-netdev.html

...

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c

...

> @@ -153,12 +151,9 @@ static int axienet_mdio_write(struct mii_bus *bus, int phy_id, int reg,
>  		     XAE_MDIO_MCR_OP_WRITE_MASK));
> 
>  	ret = axienet_mdio_wait_until_ready(lp);
> -	if (ret < 0) {
> -		axienet_mdio_mdc_disable(lp);
> -		return ret;
> -	}

Please add a blank line here.

> +disable_mdc:
>  	axienet_mdio_mdc_disable(lp);
> -	return 0;
> +	return ret;
>  }
> 
>  /**

Markus Elfring Sept. 22, 2024, 5:55 p.m. UTC | #2

>> Add a label so that a bit of exception handling can be better reused
>> at the end of this function implementation.
…
> This change seems reasonable to me.

Thanks for this positive feedback.


>                                      However, I am assuming that as a
> non-bug-fix,

Would you like to categorise my update suggestion as a correction for
a coding style issue?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.11#n526


>              this is targeted at net-next.  And net-next is currently
> closed for the v6.12 merge window.  Please consider reposting this patch
> once net-next reopens.

Will a patch resend really be needed for the proposed adjustment?

Can both development branches benefit from this refactoring?

Regards,
Markus

Patch

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
index 9ca2643c921e..0c7b931b2e66 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
@@ -138,10 +138,8 @@  static int axienet_mdio_write(struct mii_bus *bus, int phy_id, int reg,
 	axienet_mdio_mdc_enable(lp);

 	ret = axienet_mdio_wait_until_ready(lp);
-	if (ret < 0) {
-		axienet_mdio_mdc_disable(lp);
-		return ret;
-	}
+	if (ret < 0)
+		goto disable_mdc;

 	axienet_iow(lp, XAE_MDIO_MWD_OFFSET, (u32)val);
 	axienet_iow(lp, XAE_MDIO_MCR_OFFSET,
@@ -153,12 +151,9 @@  static int axienet_mdio_write(struct mii_bus *bus, int phy_id, int reg,
 		     XAE_MDIO_MCR_OP_WRITE_MASK));

 	ret = axienet_mdio_wait_until_ready(lp);
-	if (ret < 0) {
-		axienet_mdio_mdc_disable(lp);
-		return ret;
-	}
+disable_mdc:
 	axienet_mdio_mdc_disable(lp);
-	return 0;
+	return ret;
 }

 /**