[PATCH] mfd: twl-core: One function call less in add_numbered_child() after error detection

[PATCH] mfd: twl-core: One function call less in add_numbered_child() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 29 Dec 2015 19:29:08 +0100

The platform_device_put() function was called in one case by the
add_numbered_child() function during error handling even if the passed
variable "pdev" contained a null pointer.

Implementation details could be improved by the adjustment of jump targets
according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

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

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 831696e..0d9350c 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -625,7 +625,7 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
 	if (!pdev) {
 		dev_dbg(&twl->client->dev, "can't alloc dev\n");
 		status = -ENOMEM;
-		goto err;
+		goto report_failure;
 	}
 
 	pdev->dev.parent = &twl->client->dev;
@@ -634,7 +634,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;
 		}
 	}
 
@@ -647,21 +647,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)
+	if (!status) {
 		device_init_wakeup(&pdev->dev, can_wakeup);
-
-err:
-	if (status < 0) {
-		platform_device_put(pdev);
-		dev_err(&twl->client->dev, "can't add %s dev\n", name);
-		return ERR_PTR(status);
+		return &pdev->dev;
 	}
-	return &pdev->dev;
+put_device:
+	platform_device_put(pdev);
+report_failure:
+	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,
-- 
2.6.3

Re: [PATCH] mfd: twl-core: One function call less in add_numbered_child() after error detection

[Lee Jones]

On Tue, 29 Dec 2015, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 29 Dec 2015 19:29:08 +0100
> 
> The platform_device_put() function was called in one case by the
> add_numbered_child() function during error handling even if the passed
> variable "pdev" contained a null pointer.
> 
> Implementation details could be improved by the adjustment of jump targets
> according to the Linux coding style convention.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mfd/twl-core.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 831696e..0d9350c 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -625,7 +625,7 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
>  	if (!pdev) {
>  		dev_dbg(&twl->client->dev, "can't alloc dev\n");

Change this to be dev_err()

>  		status = -ENOMEM;
> -		goto err;
> +		goto report_failure;

... and just return status from here.

>  	}
>  
>  	pdev->dev.parent = &twl->client->dev;
> @@ -634,7 +634,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;
>  		}
>  	}
>  
> @@ -647,21 +647,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)
> +	if (!status) {

You've changed the way you handle errors from this point.  To be more
consistent it would be better if you checked for status, then jump to
put_device in the case of a failure, as you do above.

>  		device_init_wakeup(&pdev->dev, can_wakeup);
> -
> -err:
> -	if (status < 0) {
> -		platform_device_put(pdev);
> -		dev_err(&twl->client->dev, "can't add %s dev\n", name);
> -		return ERR_PTR(status);
> +		return &pdev->dev;
>  	}
> -	return &pdev->dev;
> +put_device:
> +	platform_device_put(pdev);
> +report_failure:
> +	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,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] mfd: twl-core: Fine-tuning for add_numbered_child()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 May 2016 19:55:30 +0200

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

Markus Elfring (2):
  Return directly after a failed platform_device_alloc() in add_numbered_child()
  Refactoring for add_numbered_child()

 drivers/mfd/twl-core.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

-- 
2.8.2

[PATCH 1/2] mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 May 2016 19:20:28 +0200

The platform_device_put() function was called in one case by the
add_numbered_child() function during error handling even if the passed
variable "pdev" contained a null pointer.

* Change an error message.

* Return directly in this case.

This issue was detected by using the Coccinelle software.

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

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 831696e..dc34e69 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -623,9 +623,10 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
 
 	pdev = platform_device_alloc(name, num);
 	if (!pdev) {
-		dev_dbg(&twl->client->dev, "can't alloc dev\n");
-		status = -ENOMEM;
-		goto err;
+		dev_err(&twl->client->dev,
+			"Allocation failed for device: %s\n",
+			name);
+		return ERR_PTR(-ENOMEM);
 	}
 
 	pdev->dev.parent = &twl->client->dev;
-- 
2.8.2

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

[SF Markus Elfring]

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);
 	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,
-- 
2.8.2

Re: [PATCH 1/2] mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

[Julia Lawall]

On Mon, 16 May 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 15 May 2016 19:20:28 +0200
> 
> The platform_device_put() function was called in one case by the
> add_numbered_child() function during error handling even if the passed
> variable "pdev" contained a null pointer.
> 
> * Change an error message.

Why?  Is dev_err needed?  Doesn't it already print out the device name?

In any case, the only source of failure is failure of a kzalloc in 
platform_device_alloc, which means that a complete backtrace would be 
generated, so it is not clear that any message is needed at all.

julia

> 
> * Return directly in this case.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mfd/twl-core.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 831696e..dc34e69 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -623,9 +623,10 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
>  
>  	pdev = platform_device_alloc(name, num);
>  	if (!pdev) {
> -		dev_dbg(&twl->client->dev, "can't alloc dev\n");
> -		status = -ENOMEM;
> -		goto err;
> +		dev_err(&twl->client->dev,
> +			"Allocation failed for device: %s\n",
> +			name);
> +		return ERR_PTR(-ENOMEM);
>  	}
>  
>  	pdev->dev.parent = &twl->client->dev;
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

[SF Markus Elfring]

>> * Change an error message.
> 
> Why?  Is dev_err needed?

I interpreted Lee's response in this way.
https://lkml.org/lkml/2016/1/11/104

Regards,
Markus

Re: mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

[Julia Lawall]

On Mon, 16 May 2016, SF Markus Elfring wrote:

> >> * Change an error message.
> > 
> > Why?  Is dev_err needed?
> 
> I interpreted Lee's response in this way.
> https://lkml.org/lkml/2016/1/11/104

OK.  He didn't ask for the message to be changed though.  It's a bit 
unfortunate that it now takes up multiple lines.  And I believe it also 
prints redundant information.  Perhaps he will have some further thoughts 
on the matter.

julia

Re: mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

[Lee Jones]

On Mon, 16 May 2016, Julia Lawall wrote:

> On Mon, 16 May 2016, SF Markus Elfring wrote:
> 
> > >> * Change an error message.
> > > 
> > > Why?  Is dev_err needed?
> > 
> > I interpreted Lee's response in this way.
> > https://lkml.org/lkml/2016/1/11/104
> 
> OK.  He didn't ask for the message to be changed though.  It's a bit 
> unfortunate that it now takes up multiple lines.  And I believe it also 
> prints redundant information.  Perhaps he will have some further thoughts 
> on the matter.

Yes, Julia is right.  We normally don't print anything for OOM
errors since Linux reports on them already.

Please remove the print altogether.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

[SF Markus Elfring]

> Please remove the print altogether.

Would you like to omit any extra logging statements at more source code places?

Regards,
Markus

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

[Lee Jones]

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>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 0/6] mfd: Fine-tuning for three function implementations

[SF Markus Elfring]

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(-)

-- 
2.9.0

[PATCH 1/6] mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 26 Jun 2016 12:25:36 +0200

The platform_device_put() function was called in one case by the
add_numbered_child() function during error handling even if the passed
variable "pdev" contained a null pointer.
Return directly in this case.

This issue was detected by using the Coccinelle software.

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

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 831696e..9458c6d 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -622,11 +622,8 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
 	twl = &twl_priv->twl_modules[sid];
 
 	pdev = platform_device_alloc(name, num);
-	if (!pdev) {
-		dev_dbg(&twl->client->dev, "can't alloc dev\n");
-		status = -ENOMEM;
-		goto err;
-	}
+	if (!pdev)
+		return ERR_PTR(-ENOMEM);
 
 	pdev->dev.parent = &twl->client->dev;
 
-- 
2.9.0

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

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 26 Jun 2016 13:03:59 +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>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/twl-core.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 9458c6d..a49d3db 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -631,7 +631,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;
 		}
 	}
 
@@ -644,21 +644,22 @@ 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;
+
+	device_init_wakeup(&pdev->dev, can_wakeup);
 
-err:
-	if (status < 0) {
-		platform_device_put(pdev);
-		dev_err(&twl->client->dev, "can't add %s dev\n", name);
-		return ERR_PTR(status);
-	}
 	return &pdev->dev;
+
+put_device:
+	platform_device_put(pdev);
+	dev_err(&twl->client->dev, "failed to add device %s\n", name);
+	return ERR_PTR(status);
 }
 
 static inline struct device *add_child(unsigned mod_no, const char *name,
-- 
2.9.0

[PATCH 3/6] mfd: dm355evm_msp: Return directly after a failed platform_device_alloc() in add_child()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 26 Jun 2016 13:40:35 +0200

The platform_device_put() function was called in one case by the
add_child() function during error handling even if the passed
variable "pdev" contained a null pointer.
Return directly in this case.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mfd/dm355evm_msp.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/dm355evm_msp.c b/drivers/mfd/dm355evm_msp.c
index 14661ec..270e19c 100644
--- a/drivers/mfd/dm355evm_msp.c
+++ b/drivers/mfd/dm355evm_msp.c
@@ -199,11 +199,8 @@ static struct device *add_child(struct i2c_client *client, const char *name,
 	int			status;
 
 	pdev = platform_device_alloc(name, -1);
-	if (!pdev) {
-		dev_dbg(&client->dev, "can't alloc dev\n");
-		status = -ENOMEM;
-		goto err;
-	}
+	if (!pdev)
+		return ERR_PTR(-ENOMEM);
 
 	device_init_wakeup(&pdev->dev, can_wakeup);
 	pdev->dev.parent = &client->dev;
-- 
2.9.0

[PATCH 4/6] mfd: dm355evm_msp: Refactoring for add_child()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 26 Jun 2016 13:56:58 +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/dm355evm_msp.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/dm355evm_msp.c b/drivers/mfd/dm355evm_msp.c
index 270e19c..baf6075 100644
--- a/drivers/mfd/dm355evm_msp.c
+++ b/drivers/mfd/dm355evm_msp.c
@@ -209,7 +209,7 @@ static struct device *add_child(struct i2c_client *client, const char *name,
 		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;
 		}
 	}
 
@@ -222,19 +222,21 @@ static struct device *add_child(struct i2c_client *client, const char *name,
 		status = platform_device_add_resources(pdev, &r, 1);
 		if (status < 0) {
 			dev_dbg(&pdev->dev, "can't add irq\n");
-			goto err;
+			goto put_device;
 		}
 	}
 
 	status = platform_device_add(pdev);
 
-err:
-	if (status < 0) {
-		platform_device_put(pdev);
-		dev_err(&client->dev, "can't add %s dev\n", name);
-		return ERR_PTR(status);
-	}
+	if (status)
+		goto put_device;
+
 	return &pdev->dev;
+
+put_device:
+	platform_device_put(pdev);
+	dev_err(&client->dev, "failed to add device %s\n", name);
+	return ERR_PTR(status);
 }
 
 static int add_children(struct i2c_client *client)
-- 
2.9.0

[PATCH 5/6] mfd: smsc-ece1099: Delete an unnecessary variable initialisation in smsc_i2c_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 26 Jun 2016 14:14:54 +0200

The variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mfd/smsc-ece1099.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/smsc-ece1099.c b/drivers/mfd/smsc-ece1099.c
index 7f89e89..2aaf89f 100644
--- a/drivers/mfd/smsc-ece1099.c
+++ b/drivers/mfd/smsc-ece1099.c
@@ -36,7 +36,7 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
 {
 	struct smsc *smsc;
 	int devid, rev, venid_l, venid_h;
-	int ret = 0;
+	int ret;
 
 	smsc = devm_kzalloc(&i2c->dev, sizeof(struct smsc),
 				GFP_KERNEL);
-- 
2.9.0

[PATCH 6/6] mfd: smsc-ece1099: Return directly after a function failure in smsc_i2c_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 26 Jun 2016 14:30:46 +0200

This issue was detected by using the Coccinelle software.

Return directly if a call of the function "devm_regmap_init_i2c"
or "regmap_write" failed.

Delete the jump label "err" then.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mfd/smsc-ece1099.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/smsc-ece1099.c b/drivers/mfd/smsc-ece1099.c
index 2aaf89f..cd18c09 100644
--- a/drivers/mfd/smsc-ece1099.c
+++ b/drivers/mfd/smsc-ece1099.c
@@ -46,10 +46,8 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
 	}
 
 	smsc->regmap = devm_regmap_init_i2c(i2c, &smsc_regmap_config);
-	if (IS_ERR(smsc->regmap)) {
-		ret = PTR_ERR(smsc->regmap);
-		goto err;
-	}
+	if (IS_ERR(smsc->regmap))
+		return PTR_ERR(smsc->regmap);
 
 	i2c_set_clientdata(i2c, smsc);
 	smsc->dev = &i2c->dev;
@@ -68,7 +66,7 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
 
 	ret = regmap_write(smsc->regmap, SMSC_CLK_CTRL, smsc->clk);
 	if (ret)
-		goto err;
+		return ret;
 
 #ifdef CONFIG_OF
 	if (i2c->dev.of_node)
@@ -76,7 +74,6 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
 					   NULL, NULL, &i2c->dev);
 #endif
 
-err:
 	return ret;
 }
 
-- 
2.9.0

Re: [PATCH 0/6] mfd: Fine-tuning for three function implementations

[Lee Jones]

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).

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

[Lee Jones]

On Sun, 26 Jun 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 26 Jun 2016 12:25:36 +0200

Please use `git send-email` when sending patches to the list.

> The platform_device_put() function was called in one case by the
> add_numbered_child() function during error handling even if the passed
> variable "pdev" contained a null pointer.
> Return directly in this case.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mfd/twl-core.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Applied though, thanks.

> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 831696e..9458c6d 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -622,11 +622,8 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
>  	twl = &twl_priv->twl_modules[sid];
>  
>  	pdev = platform_device_alloc(name, num);
> -	if (!pdev) {
> -		dev_dbg(&twl->client->dev, "can't alloc dev\n");
> -		status = -ENOMEM;
> -		goto err;
> -	}
> +	if (!pdev)
> +		return ERR_PTR(-ENOMEM);
>  
>  	pdev->dev.parent = &twl->client->dev;
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Lee Jones]

On Sun, 26 Jun 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 26 Jun 2016 13:03:59 +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>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/twl-core.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 9458c6d..a49d3db 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -631,7 +631,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;
>  		}
>  	}
>  
> @@ -644,21 +644,22 @@ 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;
> +
> +	device_init_wakeup(&pdev->dev, can_wakeup);
>  
> -err:
> -	if (status < 0) {
> -		platform_device_put(pdev);
> -		dev_err(&twl->client->dev, "can't add %s dev\n", name);
> -		return ERR_PTR(status);
> -	}
>  	return &pdev->dev;
> +
> +put_device:
> +	platform_device_put(pdev);
> +	dev_err(&twl->client->dev, "failed to add device %s\n", name);
> +	return ERR_PTR(status);
>  }
>  
>  static inline struct device *add_child(unsigned mod_no, const char *name,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] mfd: dm355evm_msp: Return directly after a failed platform_device_alloc() in add_child()

[Lee Jones]

On Sun, 26 Jun 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 26 Jun 2016 13:40:35 +0200
> 
> The platform_device_put() function was called in one case by the
> add_child() function during error handling even if the passed
> variable "pdev" contained a null pointer.
> Return directly in this case.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mfd/dm355evm_msp.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/dm355evm_msp.c b/drivers/mfd/dm355evm_msp.c
> index 14661ec..270e19c 100644
> --- a/drivers/mfd/dm355evm_msp.c
> +++ b/drivers/mfd/dm355evm_msp.c
> @@ -199,11 +199,8 @@ static struct device *add_child(struct i2c_client *client, const char *name,
>  	int			status;
>  
>  	pdev = platform_device_alloc(name, -1);
> -	if (!pdev) {
> -		dev_dbg(&client->dev, "can't alloc dev\n");
> -		status = -ENOMEM;
> -		goto err;
> -	}
> +	if (!pdev)
> +		return ERR_PTR(-ENOMEM);
>  
>  	device_init_wakeup(&pdev->dev, can_wakeup);
>  	pdev->dev.parent = &client->dev;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 4/6] mfd: dm355evm_msp: Refactoring for add_child()

[Lee Jones]

On Sun, 26 Jun 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 26 Jun 2016 13:56:58 +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/dm355evm_msp.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mfd/dm355evm_msp.c b/drivers/mfd/dm355evm_msp.c
> index 270e19c..baf6075 100644
> --- a/drivers/mfd/dm355evm_msp.c
> +++ b/drivers/mfd/dm355evm_msp.c
> @@ -209,7 +209,7 @@ static struct device *add_child(struct i2c_client *client, const char *name,
>  		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;
>  		}
>  	}
>  
> @@ -222,19 +222,21 @@ static struct device *add_child(struct i2c_client *client, const char *name,
>  		status = platform_device_add_resources(pdev, &r, 1);
>  		if (status < 0) {
>  			dev_dbg(&pdev->dev, "can't add irq\n");
> -			goto err;
> +			goto put_device;
>  		}
>  	}
>  
>  	status = platform_device_add(pdev);
>  

Remove this line too.

> -err:
> -	if (status < 0) {
> -		platform_device_put(pdev);
> -		dev_err(&client->dev, "can't add %s dev\n", name);
> -		return ERR_PTR(status);
> -	}
> +	if (status)
> +		goto put_device;
> +
>  	return &pdev->dev;
> +
> +put_device:
> +	platform_device_put(pdev);
> +	dev_err(&client->dev, "failed to add device %s\n", name);
> +	return ERR_PTR(status);
>  }
>  
>  static int add_children(struct i2c_client *client)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] mfd: smsc-ece1099: Delete an unnecessary variable initialisation in smsc_i2c_probe()

[Lee Jones]

On Sun, 26 Jun 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 26 Jun 2016 14:14:54 +0200
> 
> The variable "ret" will be set to an appropriate value a bit later.
> Thus omit the explicit initialisation at the beginning.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mfd/smsc-ece1099.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

> diff --git a/drivers/mfd/smsc-ece1099.c b/drivers/mfd/smsc-ece1099.c
> index 7f89e89..2aaf89f 100644
> --- a/drivers/mfd/smsc-ece1099.c
> +++ b/drivers/mfd/smsc-ece1099.c
> @@ -36,7 +36,7 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
>  {
>  	struct smsc *smsc;
>  	int devid, rev, venid_l, venid_h;
> -	int ret = 0;
> +	int ret;
>  
>  	smsc = devm_kzalloc(&i2c->dev, sizeof(struct smsc),
>  				GFP_KERNEL);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] mfd: smsc-ece1099: Return directly after a function failure in smsc_i2c_probe()

[Lee Jones]

On Sun, 26 Jun 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 26 Jun 2016 14:30:46 +0200
> 
> This issue was detected by using the Coccinelle software.
> 
> Return directly if a call of the function "devm_regmap_init_i2c"
> or "regmap_write" failed.
> 
> Delete the jump label "err" then.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mfd/smsc-ece1099.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/smsc-ece1099.c b/drivers/mfd/smsc-ece1099.c
> index 2aaf89f..cd18c09 100644
> --- a/drivers/mfd/smsc-ece1099.c
> +++ b/drivers/mfd/smsc-ece1099.c
> @@ -46,10 +46,8 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
>  	}
>  
>  	smsc->regmap = devm_regmap_init_i2c(i2c, &smsc_regmap_config);
> -	if (IS_ERR(smsc->regmap)) {
> -		ret = PTR_ERR(smsc->regmap);
> -		goto err;
> -	}
> +	if (IS_ERR(smsc->regmap))
> +		return PTR_ERR(smsc->regmap);
>  
>  	i2c_set_clientdata(i2c, smsc);
>  	smsc->dev = &i2c->dev;
> @@ -68,7 +66,7 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
>  
>  	ret = regmap_write(smsc->regmap, SMSC_CLK_CTRL, smsc->clk);
>  	if (ret)
> -		goto err;
> +		return ret;
>  
>  #ifdef CONFIG_OF
>  	if (i2c->dev.of_node)
> @@ -76,7 +74,6 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
>  					   NULL, NULL, &i2c->dev);
>  #endif
>  
> -err:
>  	return ret;
>  }
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 4/6] mfd: dm355evm_msp: Refactoring for add_child()

[SF Markus Elfring]

>> @@ -222,19 +222,21 @@ static struct device *add_child(struct i2c_client *client, const char *name,
>>  		status = platform_device_add_resources(pdev, &r, 1);
>>  		if (status < 0) {
>>  			dev_dbg(&pdev->dev, "can't add irq\n");
>> -			goto err;
>> +			goto put_device;
>>  		}
>>  	}
>>  
>>  	status = platform_device_add(pdev);
>>  
> 
> Remove this line too.

Do you propose the deletion of a blank line here?

Did you skip this update suggestion while the other patches were finally accepted?

Regards,
Markus

Re: [PATCH 4/6] mfd: dm355evm_msp: Refactoring for add_child()

[Lee Jones]

On Tue, 28 Jun 2016, SF Markus Elfring wrote:

> >> @@ -222,19 +222,21 @@ static struct device *add_child(struct i2c_client *client, const char *name,
> >>  		status = platform_device_add_resources(pdev, &r, 1);
> >>  		if (status < 0) {
> >>  			dev_dbg(&pdev->dev, "can't add irq\n");
> >> -			goto err;
> >> +			goto put_device;
> >>  		}
> >>  	}
> >>  
> >>  	status = platform_device_add(pdev);
> >>  
> > 
> > Remove this line too.
> 
> Do you propose the deletion of a blank line here?

Yes.

> Did you skip this update suggestion while the other patches were finally accepted?

I don't know what this means.

The other patches in the set have been accepted.  Please fix this one
and send it again on its own.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog