diff mbox

[2/2] mfd: twl-core: Refactoring for add_numbered_child()

Message ID de3598c4-f85a-6807-303a-55ee0ac707f2@users.sourceforge.net (mailing list archive)
State New, archived
Headers show

Commit Message

SF Markus Elfring May 16, 2016, 6:28 a.m. UTC
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 May 2016 19:50:55 +0200

Adjust jump targets according to the Linux coding style convention.
Another check for the variable "status" can be omitted then at the end.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mfd/twl-core.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

Comments

Lee Jones June 8, 2016, 11:14 a.m. UTC | #1
On Mon, 16 May 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 15 May 2016 19:50:55 +0200
> 
> Adjust jump targets according to the Linux coding style convention.
> Another check for the variable "status" can be omitted then at the end.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mfd/twl-core.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index dc34e69..3e4f4e4 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -635,7 +635,7 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
>  		status = platform_device_add_data(pdev, pdata, pdata_len);
>  		if (status < 0) {
>  			dev_dbg(&pdev->dev, "can't add platform_data\n");
> -			goto err;
> +			goto put_device;
>  		}
>  	}
>  
> @@ -648,21 +648,20 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
>  		status = platform_device_add_resources(pdev, r, irq1 ? 2 : 1);
>  		if (status < 0) {
>  			dev_dbg(&pdev->dev, "can't add irqs\n");
> -			goto err;
> +			goto put_device;
>  		}
>  	}
>  
>  	status = platform_device_add(pdev);
> -	if (status == 0)
> -		device_init_wakeup(&pdev->dev, can_wakeup);
> +	if (status)
> +		goto put_device;
>  
> -err:
> -	if (status < 0) {
> -		platform_device_put(pdev);
> -		dev_err(&twl->client->dev, "can't add %s dev\n", name);
> -		return ERR_PTR(status);
> -	}
> +	device_init_wakeup(&pdev->dev, can_wakeup);

Nit: Place a '\n' here.

>  	return &pdev->dev;

Nit: Place a '\n' here.

> +put_device:
> +	platform_device_put(pdev);
> +	dev_err(&twl->client->dev, "can't add %s dev\n", name);

Nit: "failed to add device %s\n"

> +	return ERR_PTR(status);
>  }
>  
>  static inline struct device *add_child(unsigned mod_no, const char *name,

Once you've fixed those, please re-submit with my:

Acked-by: Lee Jones <lee.jones@linaro.org>
SF Markus Elfring June 26, 2016, 1:34 p.m. UTC | #2
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 26 Jun 2016 15:25:43 +0200

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (6):
  twl-core: Return directly after a failed platform_device_alloc()
    in add_numbered_child()
  twl-core: Refactoring for add_numbered_child()
  dm355evm_msp: Return directly after a failed platform_device_alloc()
    in add_child()
  dm355evm_msp: Refactoring for add_child()
  smsc-ece1099: Delete an unnecessary variable initialisation
    in smsc_i2c_probe()
  smsc-ece1099: Return directly after a function failure
    in smsc_i2c_probe()

 drivers/mfd/dm355evm_msp.c | 25 ++++++++++++-------------
 drivers/mfd/smsc-ece1099.c | 11 ++++-------
 drivers/mfd/twl-core.c     | 28 +++++++++++++---------------
 3 files changed, 29 insertions(+), 35 deletions(-)
Lee Jones June 28, 2016, 3:01 p.m. UTC | #3
On Sun, 26 Jun 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 26 Jun 2016 15:25:43 +0200
> 
> Several update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (6):
>   twl-core: Return directly after a failed platform_device_alloc()
>     in add_numbered_child()
>   twl-core: Refactoring for add_numbered_child()
>   dm355evm_msp: Return directly after a failed platform_device_alloc()
>     in add_child()
>   dm355evm_msp: Refactoring for add_child()
>   smsc-ece1099: Delete an unnecessary variable initialisation
>     in smsc_i2c_probe()
>   smsc-ece1099: Return directly after a function failure
>     in smsc_i2c_probe()
> 
>  drivers/mfd/dm355evm_msp.c | 25 ++++++++++++-------------
>  drivers/mfd/smsc-ece1099.c | 11 ++++-------
>  drivers/mfd/twl-core.c     | 28 +++++++++++++---------------
>  3 files changed, 29 insertions(+), 35 deletions(-)

What is this set?  A different but related one to the set you tagged
it on to?  Probably best not to do that.  I now have a huge entangled
thread in my inbox, which is going to become out of control rather
quickly (if it isn't already).
diff mbox

Patch

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index dc34e69..3e4f4e4 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -635,7 +635,7 @@  add_numbered_child(unsigned mod_no, const char *name, int num,
 		status = platform_device_add_data(pdev, pdata, pdata_len);
 		if (status < 0) {
 			dev_dbg(&pdev->dev, "can't add platform_data\n");
-			goto err;
+			goto put_device;
 		}
 	}
 
@@ -648,21 +648,20 @@  add_numbered_child(unsigned mod_no, const char *name, int num,
 		status = platform_device_add_resources(pdev, r, irq1 ? 2 : 1);
 		if (status < 0) {
 			dev_dbg(&pdev->dev, "can't add irqs\n");
-			goto err;
+			goto put_device;
 		}
 	}
 
 	status = platform_device_add(pdev);
-	if (status == 0)
-		device_init_wakeup(&pdev->dev, can_wakeup);
+	if (status)
+		goto put_device;
 
-err:
-	if (status < 0) {
-		platform_device_put(pdev);
-		dev_err(&twl->client->dev, "can't add %s dev\n", name);
-		return ERR_PTR(status);
-	}
+	device_init_wakeup(&pdev->dev, can_wakeup);
 	return &pdev->dev;
+put_device:
+	platform_device_put(pdev);
+	dev_err(&twl->client->dev, "can't add %s dev\n", name);
+	return ERR_PTR(status);
 }
 
 static inline struct device *add_child(unsigned mod_no, const char *name,