[PATCH] nfc: pn533: remove redundant assignment

[PATCH] nfc: pn533: remove redundant assignment

[samirweng1979]

From: wengjianfeng <wengjianfeng@yulong.com>

In many places,first assign a value to a variable and then return
the variable. which is redundant, we should directly return the value.
in pn533_rf_field funciton,return statement in the if statement is
redundant, we just delete it.

Signed-off-by: wengjianfeng <wengjianfeng@yulong.com>
---
 drivers/nfc/pn533/i2c.c   |  8 ++------
 drivers/nfc/pn533/pn533.c | 16 +++-------------
 drivers/nfc/pn533/uart.c  |  3 +--
 3 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 0207e66..795da9b 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -40,11 +40,8 @@ static int pn533_i2c_send_ack(struct pn533 *dev, gfp_t flags)
 	struct i2c_client *client = phy->i2c_dev;
 	static const u8 ack[6] = {0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
 	/* spec 6.2.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
-	int rc;
-
-	rc = i2c_master_send(client, ack, 6);
 
-	return rc;
+	return i2c_master_send(client, ack, 6);
 }
 
 static int pn533_i2c_send_frame(struct pn533 *dev,
@@ -199,8 +196,7 @@ static int pn533_i2c_probe(struct i2c_client *client,
 				&phy->i2c_dev->dev);
 
 	if (IS_ERR(priv)) {
-		r = PTR_ERR(priv);
-		return r;
+		return PTR_ERR(priv);
 	}
 
 	phy->priv = priv;
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index f1469ac..61ab4c0 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -489,12 +489,8 @@ static int pn533_send_data_async(struct pn533 *dev, u8 cmd_code,
 				 pn533_send_async_complete_t complete_cb,
 				 void *complete_cb_context)
 {
-	int rc;
-
-	rc = __pn533_send_async(dev, cmd_code, req, complete_cb,
+	return __pn533_send_async(dev, cmd_code, req, complete_cb,
 				complete_cb_context);
-
-	return rc;
 }
 
 static int pn533_send_cmd_async(struct pn533 *dev, u8 cmd_code,
@@ -502,12 +498,8 @@ static int pn533_send_cmd_async(struct pn533 *dev, u8 cmd_code,
 				pn533_send_async_complete_t complete_cb,
 				void *complete_cb_context)
 {
-	int rc;
-
-	rc = __pn533_send_async(dev, cmd_code, req, complete_cb,
+	return __pn533_send_async(dev, cmd_code, req, complete_cb,
 				complete_cb_context);
-
-	return rc;
 }
 
 /*
@@ -2614,7 +2606,6 @@ static int pn533_rf_field(struct nfc_dev *nfc_dev, u8 rf)
 				     (u8 *)&rf_field, 1);
 	if (rc) {
 		nfc_err(dev->dev, "Error on setting RF field\n");
-		return rc;
 	}
 
 	return rc;
@@ -2791,7 +2782,6 @@ struct pn533 *pn53x_common_init(u32 device_type,
 				struct device *dev)
 {
 	struct pn533 *priv;
-	int rc = -ENOMEM;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -2833,7 +2823,7 @@ struct pn533 *pn53x_common_init(u32 device_type,
 
 error:
 	kfree(priv);
-	return ERR_PTR(rc);
+	return ERR_PTR(-ENOMEM);
 }
 EXPORT_SYMBOL_GPL(pn53x_common_init);
 
diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
index a0665d8..6465348 100644
--- a/drivers/nfc/pn533/uart.c
+++ b/drivers/nfc/pn533/uart.c
@@ -239,9 +239,8 @@ static int pn532_uart_probe(struct serdev_device *serdev)
 {
 	struct pn532_uart_phy *pn532;
 	struct pn533 *priv;
-	int err;
+	int err = -ENOMEM;
 
-	err = -ENOMEM;
 	pn532 = kzalloc(sizeof(*pn532), GFP_KERNEL);
 	if (!pn532)
 		goto err_exit;
-- 
1.9.1

Re: [PATCH] nfc: pn533: remove redundant assignment

[Jakub Kicinski]

On Fri,  9 Apr 2021 19:50:09 +0800 samirweng1979 wrote:
> From: wengjianfeng <wengjianfeng@yulong.com>
> 
> In many places,first assign a value to a variable and then return
> the variable. which is redundant, we should directly return the value.
> in pn533_rf_field funciton,return statement in the if statement is
> redundant, we just delete it.
> 
> Signed-off-by: wengjianfeng <wengjianfeng@yulong.com>

Thank you for the changes, please see comments below.

> diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
> index f1469ac..61ab4c0 100644
> --- a/drivers/nfc/pn533/pn533.c
> +++ b/drivers/nfc/pn533/pn533.c
> @@ -489,12 +489,8 @@ static int pn533_send_data_async(struct pn533 *dev, u8 cmd_code,
>  				 pn533_send_async_complete_t complete_cb,
>  				 void *complete_cb_context)
>  {
> -	int rc;
> -
> -	rc = __pn533_send_async(dev, cmd_code, req, complete_cb,
> +	return __pn533_send_async(dev, cmd_code, req, complete_cb,
>  				complete_cb_context);

Please realign the continuation line so it starts after the opening
bracket.

> -
> -	return rc;
>  }
>  
>  static int pn533_send_cmd_async(struct pn533 *dev, u8 cmd_code,
> @@ -502,12 +498,8 @@ static int pn533_send_cmd_async(struct pn533 *dev, u8 cmd_code,
>  				pn533_send_async_complete_t complete_cb,
>  				void *complete_cb_context)
>  {
> -	int rc;
> -
> -	rc = __pn533_send_async(dev, cmd_code, req, complete_cb,
> +	return __pn533_send_async(dev, cmd_code, req, complete_cb,
>  				complete_cb_context);

Same here.

> -	return rc;
>  }
>  
>  /*
> @@ -2614,7 +2606,6 @@ static int pn533_rf_field(struct nfc_dev *nfc_dev, u8 rf)
>  				     (u8 *)&rf_field, 1);
>  	if (rc) {
>  		nfc_err(dev->dev, "Error on setting RF field\n");
> -		return rc;
>  	}
>  
>  	return rc;

In case some code is added between the check and the return statement
it'd be better to fix this by replacing the second return rc with
return 0:

	if (rc) {
		nfc_err(...);
		return rc;
	}

	return 0;

> diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
> index a0665d8..6465348 100644
> --- a/drivers/nfc/pn533/uart.c
> +++ b/drivers/nfc/pn533/uart.c
> @@ -239,9 +239,8 @@ static int pn532_uart_probe(struct serdev_device *serdev)
>  {
>  	struct pn532_uart_phy *pn532;
>  	struct pn533 *priv;
> -	int err;
> +	int err = -ENOMEM;
>  
> -	err = -ENOMEM;
>  	pn532 = kzalloc(sizeof(*pn532), GFP_KERNEL);

IMO having the assignment before the call is more readable, please
leave as is.

>  	if (!pn532)
>  		goto err_exit;