[PATCH 00/13] i2c: Use devm_*() functions

[PATCH 00/13] i2c: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Jingoo Han (13):
      i2c: bcm2835: Use devm_request_irq()
      i2c: gpio: Use devm_gpio_request()
      i2c: highlander: Use devm_*() functions
      i2c: isch: Use devm_request_region()
      i2c: mv64xxx: Use devm_request_irq()
      i2c: pmcmsp: Use devm_*() functions
      i2c: pnx: Use devm_*() functions
      i2c: pxa: Use devm_*() functions
      i2c: simtec: Use devm_*() functions
      i2c: sirf: Use devm_clk_get()
      i2c: versatile: Use devm_*() functions
      i2c: viperboard: Use devm_kzalloc() functions
      i2c: xiic: Use devm_*() functions

---
 drivers/i2c/busses/i2c-bcm2835.c    |    7 ++--
 drivers/i2c/busses/i2c-gpio.c       |   25 ++++---------
 drivers/i2c/busses/i2c-highlander.c |   38 ++++++--------------
 drivers/i2c/busses/i2c-isch.c       |    5 ++-
 drivers/i2c/busses/i2c-mv64xxx.c    |    9 ++---
 drivers/i2c/busses/i2c-pmcmsp.c     |   44 ++++-------------------
 drivers/i2c/busses/i2c-pnx.c        |   56 +++++++++--------------------
 drivers/i2c/busses/i2c-pxa.c        |   66 ++++++++++-------------------------
 drivers/i2c/busses/i2c-simtec.c     |   43 ++++-------------------
 drivers/i2c/busses/i2c-sirf.c       |   11 ++----
 drivers/i2c/busses/i2c-versatile.c  |   33 +++++-------------
 drivers/i2c/busses/i2c-viperboard.c |   12 ++-----
 drivers/i2c/busses/i2c-xiic.c       |   59 +++++++++----------------------
 13 files changed, 104 insertions(+), 304 deletions(-)

[PATCH 01/13] i2c: bcm2835: Use devm_request_irq()

[Jingoo Han]

Use devm_request_irq() to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-bcm2835.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 77df97b..af60181 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -285,8 +285,8 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
 	}
 	i2c_dev->irq = irq->start;
 
-	ret = request_irq(i2c_dev->irq, bcm2835_i2c_isr, IRQF_SHARED,
-			  dev_name(&pdev->dev), i2c_dev);
+	ret = devm_request_irq(&pdev->dev, i2c_dev->irq, bcm2835_i2c_isr,
+			       IRQF_SHARED, dev_name(&pdev->dev), i2c_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Could not request IRQ\n");
 		return -ENODEV;
@@ -304,8 +304,6 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
 	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, 0);
 
 	ret = i2c_add_adapter(adap);
-	if (ret)
-		free_irq(i2c_dev->irq, i2c_dev);
 
 	return ret;
 }
@@ -314,7 +312,6 @@ static int bcm2835_i2c_remove(struct platform_device *pdev)
 {
 	struct bcm2835_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
 
-	free_irq(i2c_dev->irq, i2c_dev);
 	i2c_del_adapter(&i2c_dev->adapter);
 
 	return 0;
-- 
1.7.10.4

Re: [PATCH 01/13] i2c: bcm2835: Use devm_request_irq()

[Stephen Warren]

On 12/16/2013 11:46 PM, Jingoo Han wrote:
> Use devm_request_irq() to make cleanup paths simpler.

This may not be safe. The interrupt used by the I2C controllers on the
BCM2835 chip is shared between two controllers. In theory, you could run
into a condition where you're remove()ing the driver for one of the
controller (a), while the driver for controller (b) is still active, yet
the HW for controller (a) comes along and triggers an interrupt after
remove() but before the devm IRQ cleanup. That would result in the IRQ
handler for controller (a) executing after the remove() of the
associated device, which could cause a variety of problems.

You might be able to make this safe by explicitly clearing any IRQ
enable registers in remove(), but leaving the code using plain
request_irq() might be simpler.

Re: [PATCH 01/13] i2c: bcm2835: Use devm_request_irq()

[Jingoo Han]

On Wednesday, December 18, 2013 11:21 AM, Stephen Warren wrote:
> On 12/16/2013 11:46 PM, Jingoo Han wrote:
> > Use devm_request_irq() to make cleanup paths simpler.
> 
> This may not be safe. The interrupt used by the I2C controllers on the
> BCM2835 chip is shared between two controllers. In theory, you could run
> into a condition where you're remove()ing the driver for one of the
> controller (a), while the driver for controller (b) is still active, yet
> the HW for controller (a) comes along and triggers an interrupt after
> remove() but before the devm IRQ cleanup. That would result in the IRQ
> handler for controller (a) executing after the remove() of the
> associated device, which could cause a variety of problems.

Hi Stephen,
I appreciate you detailed and kind description. :-)
If there is possible side effect, the patch should not be
merged.

Wolfram Sang,
Please ignore this patch. Thank you.

Best regards,
Jingoo Han

> You might be able to make this safe by explicitly clearing any IRQ
> enable registers in remove(), but leaving the code using plain
> request_irq() might be simpler.

Re: [PATCH 01/13] i2c: bcm2835: Use devm_request_irq()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]


> If there is possible side effect, the patch should not be
> merged.
> 
> Wolfram Sang,
> Please ignore this patch. Thank you.

Even more so, I will ignore this series unless there is someone having
actually tested the patch on real HW. Thanks for your effort, yet I need
to be sure it really works on the actual HW.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 02/13] i2c: gpio: Use devm_gpio_request()

[Jingoo Han]

Use devm_gpio_request() to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-gpio.c |   25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index d9f7e18..401be4c 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -144,24 +144,22 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 		scl_pin = pdata->scl_pin;
 	}
 
-	ret = gpio_request(sda_pin, "sda");
+	ret = devm_gpio_request(&pdev->dev, sda_pin, "sda");
 	if (ret) {
 		if (ret == -EINVAL)
 			ret = -EPROBE_DEFER;	/* Try again later */
-		goto err_request_sda;
+		return ret;
 	}
-	ret = gpio_request(scl_pin, "scl");
+	ret = devm_gpio_request(&pdev->dev, scl_pin, "scl");
 	if (ret) {
 		if (ret == -EINVAL)
 			ret = -EPROBE_DEFER;	/* Try again later */
-		goto err_request_scl;
+		return ret;
 	}
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
-		ret = -ENOMEM;
-		goto err_add_bus;
-	}
+	if (!priv)
+		return -ENOMEM;
 	adap = &priv->adap;
 	bit_data = &priv->bit_data;
 	pdata = &priv->pdata;
@@ -222,7 +220,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 	adap->nr = pdev->id;
 	ret = i2c_bit_add_numbered_bus(adap);
 	if (ret)
-		goto err_add_bus;
+		return ret;
 
 	platform_set_drvdata(pdev, priv);
 
@@ -232,13 +230,6 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 		 ? ", no clock stretching" : "");
 
 	return 0;
-
-err_add_bus:
-	gpio_free(scl_pin);
-err_request_scl:
-	gpio_free(sda_pin);
-err_request_sda:
-	return ret;
 }
 
 static int i2c_gpio_remove(struct platform_device *pdev)
@@ -252,8 +243,6 @@ static int i2c_gpio_remove(struct platform_device *pdev)
 	pdata = &priv->pdata;
 
 	i2c_del_adapter(adap);
-	gpio_free(pdata->scl_pin);
-	gpio_free(pdata->sda_pin);
 
 	return 0;
 }
-- 
1.7.10.4

Re: [PATCH 02/13] i2c: gpio: Use devm_gpio_request()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]

On Tue, Dec 17, 2013 at 03:48:19PM +0900, Jingoo Han wrote:
> Use devm_gpio_request() to make cleanup paths simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Applied to for-next, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 03/13] i2c: highlander: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-highlander.c |   38 ++++++++++-------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-highlander.c b/drivers/i2c/busses/i2c-highlander.c
index 436b0f2..405dc4a 100644
--- a/drivers/i2c/busses/i2c-highlander.c
+++ b/drivers/i2c/busses/i2c-highlander.c
@@ -369,15 +369,15 @@ static int highlander_i2c_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	dev = kzalloc(sizeof(struct highlander_i2c_dev), GFP_KERNEL);
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct highlander_i2c_dev),
+			   GFP_KERNEL);
 	if (unlikely(!dev))
 		return -ENOMEM;
 
-	dev->base = ioremap_nocache(res->start, resource_size(res));
-	if (unlikely(!dev->base)) {
-		ret = -ENXIO;
-		goto err;
-	}
+	dev->base = devm_ioremap_nocache(&pdev->dev, res->start,
+					 resource_size(res));
+	if (unlikely(!dev->base))
+		return -ENXIO;
 
 	dev->dev = &pdev->dev;
 	platform_set_drvdata(pdev, dev);
@@ -387,10 +387,10 @@ static int highlander_i2c_probe(struct platform_device *pdev)
 		dev->irq = 0;
 
 	if (dev->irq) {
-		ret = request_irq(dev->irq, highlander_i2c_irq, 0,
-				  pdev->name, dev);
+		ret = devm_request_irq(&pdev->dev, dev->irq, highlander_i2c_irq,
+				       0, pdev->name, dev);
 		if (unlikely(ret))
-			goto err_unmap;
+			return ret;
 
 		highlander_i2c_irq_enable(dev);
 	} else {
@@ -417,26 +417,16 @@ static int highlander_i2c_probe(struct platform_device *pdev)
 	ret = highlander_i2c_reset(dev);
 	if (unlikely(ret)) {
 		dev_err(&pdev->dev, "controller didn't come up\n");
-		goto err_free_irq;
+		return ret;
 	}
 
 	ret = i2c_add_numbered_adapter(adap);
 	if (unlikely(ret)) {
 		dev_err(&pdev->dev, "failure adding adapter\n");
-		goto err_free_irq;
+		return ret;
 	}
 
 	return 0;
-
-err_free_irq:
-	if (dev->irq)
-		free_irq(dev->irq, dev);
-err_unmap:
-	iounmap(dev->base);
-err:
-	kfree(dev);
-
-	return ret;
 }
 
 static int highlander_i2c_remove(struct platform_device *pdev)
@@ -445,12 +435,6 @@ static int highlander_i2c_remove(struct platform_device *pdev)
 
 	i2c_del_adapter(&dev->adapter);
 
-	if (dev->irq)
-		free_irq(dev->irq, dev);
-
-	iounmap(dev->base);
-	kfree(dev);
-
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH 04/13] i2c: isch: Use devm_request_region()

[Jingoo Han]

Use devm_request_region() to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-isch.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-isch.c b/drivers/i2c/busses/i2c-isch.c
index 8c38aaa..fb02df2 100644
--- a/drivers/i2c/busses/i2c-isch.c
+++ b/drivers/i2c/busses/i2c-isch.c
@@ -275,7 +275,8 @@ static int smbus_sch_probe(struct platform_device *dev)
 	if (!res)
 		return -EBUSY;
 
-	if (!request_region(res->start, resource_size(res), dev->name)) {
+	if (!devm_request_region(&dev->dev, res->start, resource_size(res),
+				 dev->name)) {
 		dev_err(&dev->dev, "SMBus region 0x%x already in use!\n",
 			sch_smba);
 		return -EBUSY;
@@ -294,7 +295,6 @@ static int smbus_sch_probe(struct platform_device *dev)
 	retval = i2c_add_adapter(&sch_adapter);
 	if (retval) {
 		dev_err(&dev->dev, "Couldn't register adapter!\n");
-		release_region(res->start, resource_size(res));
 		sch_smba = 0;
 	}
 
@@ -307,7 +307,6 @@ static int smbus_sch_remove(struct platform_device *pdev)
 	if (sch_smba) {
 		i2c_del_adapter(&sch_adapter);
 		res = platform_get_resource(pdev, IORESOURCE_IO, 0);
-		release_region(res->start, resource_size(res));
 		sch_smba = 0;
 	}
 
-- 
1.7.10.4

Re: [PATCH 04/13] i2c: isch: Use devm_request_region()

[Jean Delvare]

Hi Jingoo Han,

On Tue, 17 Dec 2013 15:51:28 +0900, Jingoo Han wrote:
> Use devm_request_region() to make cleanup paths simpler.

You need to include <linux/ioport.h> for this function.

> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-isch.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-isch.c b/drivers/i2c/busses/i2c-isch.c
> index 8c38aaa..fb02df2 100644
> --- a/drivers/i2c/busses/i2c-isch.c
> +++ b/drivers/i2c/busses/i2c-isch.c
> @@ -275,7 +275,8 @@ static int smbus_sch_probe(struct platform_device *dev)
>  	if (!res)
>  		return -EBUSY;
>  
> -	if (!request_region(res->start, resource_size(res), dev->name)) {
> +	if (!devm_request_region(&dev->dev, res->start, resource_size(res),
> +				 dev->name)) {
>  		dev_err(&dev->dev, "SMBus region 0x%x already in use!\n",
>  			sch_smba);
>  		return -EBUSY;
> @@ -294,7 +295,6 @@ static int smbus_sch_probe(struct platform_device *dev)
>  	retval = i2c_add_adapter(&sch_adapter);
>  	if (retval) {
>  		dev_err(&dev->dev, "Couldn't register adapter!\n");
> -		release_region(res->start, resource_size(res));
>  		sch_smba = 0;
>  	}
>  
> @@ -307,7 +307,6 @@ static int smbus_sch_remove(struct platform_device *pdev)
>  	if (sch_smba) {
>  		i2c_del_adapter(&sch_adapter);
>  		res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> -		release_region(res->start, resource_size(res));
>  		sch_smba = 0;
>  	}

"res" becomes useless in the remove function as you removed the only
use of it. So you can simplify this function further.

-- 
Jean Delvare

Re: [PATCH 04/13] i2c: isch: Use devm_request_region()

[Jingoo Han]

On Tuesday, December 17, 2013 4:53 PM, Jean Delvare wrote:
> On Tue, 17 Dec 2013 15:51:28 +0900, Jingoo Han wrote:
> > Use devm_request_region() to make cleanup paths simpler.
> 
> You need to include <linux/ioport.h> for this function.

According to the i2c tree, <linux/ioport.h> is already included. :-)

./ drivers/i2c/busses/i2c-isch.c
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/kernel.h>
#include <linux/delay.h>
#include <linux/stddef.h>
#include <linux/ioport.h>      <-- Here!

> 
> > Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  drivers/i2c/busses/i2c-isch.c |    5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)

[.....]

> > @@ -307,7 +307,6 @@ static int smbus_sch_remove(struct platform_device *pdev)
> >  	if (sch_smba) {
> >  		i2c_del_adapter(&sch_adapter);
> >  		res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> > -		release_region(res->start, resource_size(res));
> >  		sch_smba = 0;
> >  	}
> 
> "res" becomes useless in the remove function as you removed the only
> use of it. So you can simplify this function further.

OK, you're right.
I will remove tge unnecessary "res" variable.
I will send V2 patch, soon.
Thank you for your comment. :-)

Best regards,
Jingoo Han

Re: [PATCH 04/13] i2c: isch: Use devm_request_region()

[Jean Delvare]

On Wed, 18 Dec 2013 10:42:39 +0900, Jingoo Han wrote:
> On Tuesday, December 17, 2013 4:53 PM, Jean Delvare wrote:
> > On Tue, 17 Dec 2013 15:51:28 +0900, Jingoo Han wrote:
> > > Use devm_request_region() to make cleanup paths simpler.
> > 
> > You need to include <linux/ioport.h> for this function.
> 
> According to the i2c tree, <linux/ioport.h> is already included. :-)
> 
> ./ drivers/i2c/busses/i2c-isch.c
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/kernel.h>
> #include <linux/delay.h>
> #include <linux/stddef.h>
> #include <linux/ioport.h>      <-- Here!

Oops, you're right, don't know how I managed to miss it, sorry.

-- 
Jean Delvare

[PATCH V2 04/13] i2c: isch: Use devm_request_region()

[Jingoo Han]

Use devm_request_region() to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Changes since V1:
- Remove unnecessary 'res' variable from smbus_sch_remove().

 drivers/i2c/busses/i2c-isch.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-isch.c b/drivers/i2c/busses/i2c-isch.c
index 8c38aaa..af21304 100644
--- a/drivers/i2c/busses/i2c-isch.c
+++ b/drivers/i2c/busses/i2c-isch.c
@@ -275,7 +275,8 @@ static int smbus_sch_probe(struct platform_device *dev)
 	if (!res)
 		return -EBUSY;
 
-	if (!request_region(res->start, resource_size(res), dev->name)) {
+	if (!devm_request_region(&dev->dev, res->start, resource_size(res),
+				 dev->name)) {
 		dev_err(&dev->dev, "SMBus region 0x%x already in use!\n",
 			sch_smba);
 		return -EBUSY;
@@ -294,7 +295,6 @@ static int smbus_sch_probe(struct platform_device *dev)
 	retval = i2c_add_adapter(&sch_adapter);
 	if (retval) {
 		dev_err(&dev->dev, "Couldn't register adapter!\n");
-		release_region(res->start, resource_size(res));
 		sch_smba = 0;
 	}
 
@@ -303,11 +303,8 @@ static int smbus_sch_probe(struct platform_device *dev)
 
 static int smbus_sch_remove(struct platform_device *pdev)
 {
-	struct resource *res;
 	if (sch_smba) {
 		i2c_del_adapter(&sch_adapter);
-		res = platform_get_resource(pdev, IORESOURCE_IO, 0);
-		release_region(res->start, resource_size(res));
 		sch_smba = 0;
 	}
 
-- 
1.7.10.4

Re: [PATCH V2 04/13] i2c: isch: Use devm_request_region()

[Jean Delvare]

On Wed, 18 Dec 2013 10:48:07 +0900, Jingoo Han wrote:
> Use devm_request_region() to make cleanup paths simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changes since V1:
> - Remove unnecessary 'res' variable from smbus_sch_remove().
> 
>  drivers/i2c/busses/i2c-isch.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-isch.c b/drivers/i2c/busses/i2c-isch.c
> index 8c38aaa..af21304 100644
> --- a/drivers/i2c/busses/i2c-isch.c
> +++ b/drivers/i2c/busses/i2c-isch.c
> @@ -275,7 +275,8 @@ static int smbus_sch_probe(struct platform_device *dev)
>  	if (!res)
>  		return -EBUSY;
>  
> -	if (!request_region(res->start, resource_size(res), dev->name)) {
> +	if (!devm_request_region(&dev->dev, res->start, resource_size(res),
> +				 dev->name)) {
>  		dev_err(&dev->dev, "SMBus region 0x%x already in use!\n",
>  			sch_smba);
>  		return -EBUSY;
> @@ -294,7 +295,6 @@ static int smbus_sch_probe(struct platform_device *dev)
>  	retval = i2c_add_adapter(&sch_adapter);
>  	if (retval) {
>  		dev_err(&dev->dev, "Couldn't register adapter!\n");
> -		release_region(res->start, resource_size(res));
>  		sch_smba = 0;
>  	}
>  
> @@ -303,11 +303,8 @@ static int smbus_sch_probe(struct platform_device *dev)
>  
>  static int smbus_sch_remove(struct platform_device *pdev)
>  {
> -	struct resource *res;
>  	if (sch_smba) {
>  		i2c_del_adapter(&sch_adapter);
> -		res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> -		release_region(res->start, resource_size(res));
>  		sch_smba = 0;
>  	}
>  

Reviewed-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>

-- 
Jean Delvare

Re: [PATCH V2 04/13] i2c: isch: Use devm_request_region()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 261 bytes --]

On Wed, Dec 18, 2013 at 10:48:07AM +0900, Jingoo Han wrote:
> Use devm_request_region() to make cleanup paths simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

OK, this got reviewed. Applied to for-next, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 05/13] i2c: mv64xxx: Use devm_request_irq()

[Jingoo Han]

Use devm_request_irq() to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-mv64xxx.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 8be7e42..d94ed5a 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -859,8 +859,8 @@ mv64xxx_i2c_probe(struct platform_device *pd)
 
 	mv64xxx_i2c_hw_init(drv_data);
 
-	rc = request_irq(drv_data->irq, mv64xxx_i2c_intr, 0,
-			 MV64XXX_I2C_CTLR_NAME, drv_data);
+	rc = devm_request_irq(&pd->dev, drv_data->irq, mv64xxx_i2c_intr, 0,
+			      MV64XXX_I2C_CTLR_NAME, drv_data);
 	if (rc) {
 		dev_err(&drv_data->adapter.dev,
 			"mv64xxx: Can't register intr handler irq%d: %d\n",
@@ -869,13 +869,11 @@ mv64xxx_i2c_probe(struct platform_device *pd)
 	} else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) {
 		dev_err(&drv_data->adapter.dev,
 			"mv64xxx: Can't add i2c adapter, rc: %d\n", -rc);
-		goto exit_free_irq;
+		goto exit_clk;
 	}
 
 	return 0;
 
-exit_free_irq:
-	free_irq(drv_data->irq, drv_data);
 exit_clk:
 #if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
@@ -893,7 +891,6 @@ mv64xxx_i2c_remove(struct platform_device *dev)
 	struct mv64xxx_i2c_data		*drv_data = platform_get_drvdata(dev);
 
 	i2c_del_adapter(&drv_data->adapter);
-	free_irq(drv_data->irq, drv_data);
 #if defined(CONFIG_HAVE_CLK)
 	/* Not all platforms have a clk */
 	if (!IS_ERR(drv_data->clk)) {
-- 
1.7.10.4

[PATCH 06/13] i2c: pmcmsp: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-pmcmsp.c |   44 +++++++--------------------------------
 1 file changed, 7 insertions(+), 37 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pmcmsp.c b/drivers/i2c/busses/i2c-pmcmsp.c
index f6389e2..cfbee76 100644
--- a/drivers/i2c/busses/i2c-pmcmsp.c
+++ b/drivers/i2c/busses/i2c-pmcmsp.c
@@ -279,34 +279,19 @@ static int pmcmsptwi_probe(struct platform_device *pldev)
 	res = platform_get_resource(pldev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pldev->dev, "IOMEM resource not found\n");
-		goto ret_err;
+		return rc;
 	}
 
-	/* reserve the memory region */
-	if (!request_mem_region(res->start, resource_size(res),
-				pldev->name)) {
-		dev_err(&pldev->dev,
-			"Unable to get memory/io address region 0x%08x\n",
-			res->start);
-		rc = -EBUSY;
-		goto ret_err;
-	}
-
-	/* remap the memory */
-	pmcmsptwi_data.iobase = ioremap_nocache(res->start,
-						resource_size(res));
-	if (!pmcmsptwi_data.iobase) {
-		dev_err(&pldev->dev,
-			"Unable to ioremap address 0x%08x\n", res->start);
-		rc = -EIO;
-		goto ret_unreserve;
-	}
+	pmcmsptwi_data.iobase = devm_ioremap_resource(&pldev->dev, res);
+	if (IS_ERR(pmcmsptwi_data.iobase))
+		return PTR_ERR(pmcmsptwi_data.iobase);
 
 	/* request the irq */
 	pmcmsptwi_data.irq = platform_get_irq(pldev, 0);
 	if (pmcmsptwi_data.irq) {
-		rc = request_irq(pmcmsptwi_data.irq, &pmcmsptwi_interrupt,
-				 IRQF_SHARED, pldev->name, &pmcmsptwi_data);
+		rc = devm_request_irq(&pldev->dev, pmcmsptwi_data.irq,
+				      &pmcmsptwi_interrupt, IRQF_SHARED,
+				      pldev->name, &pmcmsptwi_data);
 		if (rc == 0) {
 			/*
 			 * Enable 'DONE' interrupt only.
@@ -352,15 +337,8 @@ ret_unmap:
 	if (pmcmsptwi_data.irq) {
 		pmcmsptwi_writel(0,
 			pmcmsptwi_data.iobase + MSP_TWI_INT_MSK_REG_OFFSET);
-		free_irq(pmcmsptwi_data.irq, &pmcmsptwi_data);
 	}
 
-	iounmap(pmcmsptwi_data.iobase);
-
-ret_unreserve:
-	release_mem_region(res->start, resource_size(res));
-
-ret_err:
 	return rc;
 }
 
@@ -369,21 +347,13 @@ ret_err:
  */
 static int pmcmsptwi_remove(struct platform_device *pldev)
 {
-	struct resource *res;
-
 	i2c_del_adapter(&pmcmsptwi_adapter);
 
 	if (pmcmsptwi_data.irq) {
 		pmcmsptwi_writel(0,
 			pmcmsptwi_data.iobase + MSP_TWI_INT_MSK_REG_OFFSET);
-		free_irq(pmcmsptwi_data.irq, &pmcmsptwi_data);
 	}
 
-	iounmap(pmcmsptwi_data.iobase);
-
-	res = platform_get_resource(pldev, IORESOURCE_MEM, 0);
-	release_mem_region(res->start, resource_size(res));
-
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH 07/13] i2c: pnx: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-pnx.c |   56 +++++++++++++-----------------------------
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index c9a352f..f2d7add 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -628,11 +628,9 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	struct resource *res;
 	u32 speed = I2C_PNX_SPEED_KHZ_DEFAULT * 1000;
 
-	alg_data = kzalloc(sizeof(*alg_data), GFP_KERNEL);
-	if (!alg_data) {
-		ret = -ENOMEM;
-		goto err_kzalloc;
-	}
+	alg_data = devm_kzalloc(&pdev->dev, sizeof(*alg_data), GFP_KERNEL);
+	if (!alg_data)
+		return -ENOMEM;
 
 	platform_set_drvdata(pdev, alg_data);
 
@@ -657,11 +655,9 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 		 */
 	}
 #endif
-	alg_data->clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(alg_data->clk)) {
-		ret = PTR_ERR(alg_data->clk);
-		goto out_drvdata;
-	}
+	alg_data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(alg_data->clk))
+		return PTR_ERR(alg_data->clk);
 
 	init_timer(&alg_data->mif.timer);
 	alg_data->mif.timer.function = i2c_pnx_timeout;
@@ -674,29 +670,27 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "Unable to get mem resource.\n");
-		ret = -EBUSY;
-		goto out_clkget;
+		return -EBUSY;
 	}
-	if (!request_mem_region(res->start, I2C_PNX_REGION_SIZE,
-				pdev->name)) {
+	if (!devm_request_mem_region(&pdev->dev, res->start,
+				     I2C_PNX_REGION_SIZE, pdev->name)) {
 		dev_err(&pdev->dev,
 		       "I/O region 0x%08x for I2C already in use.\n",
 		       res->start);
-		ret = -ENOMEM;
-		goto out_clkget;
+		return -ENOMEM;
 	}
 
 	alg_data->base = res->start;
-	alg_data->ioaddr = ioremap(res->start, I2C_PNX_REGION_SIZE);
+	alg_data->ioaddr = devm_ioremap(&pdev->dev, res->start,
+					I2C_PNX_REGION_SIZE);
 	if (!alg_data->ioaddr) {
 		dev_err(&pdev->dev, "Couldn't ioremap I2C I/O region\n");
-		ret = -ENOMEM;
-		goto out_release;
+		return -ENOMEM;
 	}
 
 	ret = clk_enable(alg_data->clk);
 	if (ret)
-		goto out_unmap;
+		return ret;
 
 	freq = clk_get_rate(alg_data->clk);
 
@@ -730,8 +724,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 		ret = alg_data->irq;
 		goto out_clock;
 	}
-	ret = request_irq(alg_data->irq, i2c_pnx_interrupt,
-			0, pdev->name, alg_data);
+	ret = devm_request_irq(&pdev->dev, alg_data->irq, i2c_pnx_interrupt,
+			       0, pdev->name, alg_data);
 	if (ret)
 		goto out_clock;
 
@@ -739,7 +733,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	ret = i2c_add_numbered_adapter(&alg_data->adapter);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "I2C: Failed to add bus\n");
-		goto out_irq;
+		goto out_clock;
 	}
 
 	dev_dbg(&pdev->dev, "%s: Master at %#8x, irq %d.\n",
@@ -747,19 +741,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 
 	return 0;
 
-out_irq:
-	free_irq(alg_data->irq, alg_data);
 out_clock:
 	clk_disable(alg_data->clk);
-out_unmap:
-	iounmap(alg_data->ioaddr);
-out_release:
-	release_mem_region(res->start, I2C_PNX_REGION_SIZE);
-out_clkget:
-	clk_put(alg_data->clk);
-out_drvdata:
-	kfree(alg_data);
-err_kzalloc:
 	return ret;
 }
 
@@ -767,13 +750,8 @@ static int i2c_pnx_remove(struct platform_device *pdev)
 {
 	struct i2c_pnx_algo_data *alg_data = platform_get_drvdata(pdev);
 
-	free_irq(alg_data->irq, alg_data);
 	i2c_del_adapter(&alg_data->adapter);
 	clk_disable(alg_data->clk);
-	iounmap(alg_data->ioaddr);
-	release_mem_region(alg_data->base, I2C_PNX_REGION_SIZE);
-	clk_put(alg_data->clk);
-	kfree(alg_data);
 
 	return 0;
 }
-- 
1.7.10.4

Re: [PATCH 07/13] i2c: pnx: Use devm_*() functions

[Roland Stigge]

On 12/17/2013 07:55 AM, Jingoo Han wrote:
> Use devm_*() functions to make cleanup paths simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-pnx.c |   56 +++++++++++++-----------------------------
>  1 file changed, 17 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
> index c9a352f..f2d7add 100644
> --- a/drivers/i2c/busses/i2c-pnx.c
> +++ b/drivers/i2c/busses/i2c-pnx.c
> @@ -628,11 +628,9 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	u32 speed = I2C_PNX_SPEED_KHZ_DEFAULT * 1000;
>  
> -	alg_data = kzalloc(sizeof(*alg_data), GFP_KERNEL);
> -	if (!alg_data) {
> -		ret = -ENOMEM;
> -		goto err_kzalloc;
> -	}
> +	alg_data = devm_kzalloc(&pdev->dev, sizeof(*alg_data), GFP_KERNEL);
> +	if (!alg_data)
> +		return -ENOMEM;
>  
>  	platform_set_drvdata(pdev, alg_data);
>  
> @@ -657,11 +655,9 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  		 */
>  	}
>  #endif
> -	alg_data->clk = clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(alg_data->clk)) {
> -		ret = PTR_ERR(alg_data->clk);
> -		goto out_drvdata;
> -	}
> +	alg_data->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(alg_data->clk))
> +		return PTR_ERR(alg_data->clk);
>  
>  	init_timer(&alg_data->mif.timer);
>  	alg_data->mif.timer.function = i2c_pnx_timeout;
> @@ -674,29 +670,27 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
>  		dev_err(&pdev->dev, "Unable to get mem resource.\n");
> -		ret = -EBUSY;
> -		goto out_clkget;
> +		return -EBUSY;
>  	}
> -	if (!request_mem_region(res->start, I2C_PNX_REGION_SIZE,
> -				pdev->name)) {
> +	if (!devm_request_mem_region(&pdev->dev, res->start,
> +				     I2C_PNX_REGION_SIZE, pdev->name)) {
>  		dev_err(&pdev->dev,
>  		       "I/O region 0x%08x for I2C already in use.\n",
>  		       res->start);
> -		ret = -ENOMEM;
> -		goto out_clkget;
> +		return -ENOMEM;
>  	}
>  
>  	alg_data->base = res->start;
> -	alg_data->ioaddr = ioremap(res->start, I2C_PNX_REGION_SIZE);
> +	alg_data->ioaddr = devm_ioremap(&pdev->dev, res->start,
> +					I2C_PNX_REGION_SIZE);
>  	if (!alg_data->ioaddr) {
>  		dev_err(&pdev->dev, "Couldn't ioremap I2C I/O region\n");
> -		ret = -ENOMEM;
> -		goto out_release;
> +		return -ENOMEM;
>  	}
>  
>  	ret = clk_enable(alg_data->clk);
>  	if (ret)
> -		goto out_unmap;
> +		return ret;
>  
>  	freq = clk_get_rate(alg_data->clk);
>  
> @@ -730,8 +724,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  		ret = alg_data->irq;
>  		goto out_clock;
>  	}
> -	ret = request_irq(alg_data->irq, i2c_pnx_interrupt,
> -			0, pdev->name, alg_data);
> +	ret = devm_request_irq(&pdev->dev, alg_data->irq, i2c_pnx_interrupt,
> +			       0, pdev->name, alg_data);
>  	if (ret)
>  		goto out_clock;
>  
> @@ -739,7 +733,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  	ret = i2c_add_numbered_adapter(&alg_data->adapter);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "I2C: Failed to add bus\n");
> -		goto out_irq;
> +		goto out_clock;
>  	}
>  
>  	dev_dbg(&pdev->dev, "%s: Master at %#8x, irq %d.\n",
> @@ -747,19 +741,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -out_irq:
> -	free_irq(alg_data->irq, alg_data);
>  out_clock:
>  	clk_disable(alg_data->clk);
> -out_unmap:
> -	iounmap(alg_data->ioaddr);
> -out_release:
> -	release_mem_region(res->start, I2C_PNX_REGION_SIZE);
> -out_clkget:
> -	clk_put(alg_data->clk);
> -out_drvdata:
> -	kfree(alg_data);
> -err_kzalloc:
>  	return ret;
>  }
>  
> @@ -767,13 +750,8 @@ static int i2c_pnx_remove(struct platform_device *pdev)
>  {
>  	struct i2c_pnx_algo_data *alg_data = platform_get_drvdata(pdev);
>  
> -	free_irq(alg_data->irq, alg_data);
>  	i2c_del_adapter(&alg_data->adapter);
>  	clk_disable(alg_data->clk);
> -	iounmap(alg_data->ioaddr);
> -	release_mem_region(alg_data->base, I2C_PNX_REGION_SIZE);
> -	clk_put(alg_data->clk);
> -	kfree(alg_data);
>  
>  	return 0;
>  }
> 

Thanks!

Acked-by: Roland Stigge <stigge-uj/7R2tJ6VmzQB+pC5nmwQ@public.gmane.org>
Tested-by: Roland Stigge <stigge-uj/7R2tJ6VmzQB+pC5nmwQ@public.gmane.org>

Re: [PATCH 07/13] i2c: pnx: Use devm_*() functions

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]


> @@ -674,29 +670,27 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
>  		dev_err(&pdev->dev, "Unable to get mem resource.\n");
> -		ret = -EBUSY;
> -		goto out_clkget;
> +		return -EBUSY;
>  	}
> -	if (!request_mem_region(res->start, I2C_PNX_REGION_SIZE,
> -				pdev->name)) {
> +	if (!devm_request_mem_region(&pdev->dev, res->start,
> +				     I2C_PNX_REGION_SIZE, pdev->name)) {
>  		dev_err(&pdev->dev,
>  		       "I/O region 0x%08x for I2C already in use.\n",
>  		       res->start);
> -		ret = -ENOMEM;
> -		goto out_clkget;
> +		return -ENOMEM;
>  	}
>  
>  	alg_data->base = res->start;
> -	alg_data->ioaddr = ioremap(res->start, I2C_PNX_REGION_SIZE);
> +	alg_data->ioaddr = devm_ioremap(&pdev->dev, res->start,
> +					I2C_PNX_REGION_SIZE);

Why not converting it to devm_ioremap_resource?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH V2 07/13] i2c: pnx: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Changes since V1:
- Use devm_ioremap_resource() to make the code simpler, per Wolfram Sang.

 drivers/i2c/busses/i2c-pnx.c |   60 ++++++++++--------------------------------
 1 file changed, 14 insertions(+), 46 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index c9a352f..5f9cb2d 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -628,11 +628,9 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	struct resource *res;
 	u32 speed = I2C_PNX_SPEED_KHZ_DEFAULT * 1000;
 
-	alg_data = kzalloc(sizeof(*alg_data), GFP_KERNEL);
-	if (!alg_data) {
-		ret = -ENOMEM;
-		goto err_kzalloc;
-	}
+	alg_data = devm_kzalloc(&pdev->dev, sizeof(*alg_data), GFP_KERNEL);
+	if (!alg_data)
+		return -ENOMEM;
 
 	platform_set_drvdata(pdev, alg_data);
 
@@ -657,11 +655,9 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 		 */
 	}
 #endif
-	alg_data->clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(alg_data->clk)) {
-		ret = PTR_ERR(alg_data->clk);
-		goto out_drvdata;
-	}
+	alg_data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(alg_data->clk))
+		return PTR_ERR(alg_data->clk);
 
 	init_timer(&alg_data->mif.timer);
 	alg_data->mif.timer.function = i2c_pnx_timeout;
@@ -674,29 +670,17 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "Unable to get mem resource.\n");
-		ret = -EBUSY;
-		goto out_clkget;
-	}
-	if (!request_mem_region(res->start, I2C_PNX_REGION_SIZE,
-				pdev->name)) {
-		dev_err(&pdev->dev,
-		       "I/O region 0x%08x for I2C already in use.\n",
-		       res->start);
-		ret = -ENOMEM;
-		goto out_clkget;
+		return -EBUSY;
 	}
 
 	alg_data->base = res->start;
-	alg_data->ioaddr = ioremap(res->start, I2C_PNX_REGION_SIZE);
-	if (!alg_data->ioaddr) {
-		dev_err(&pdev->dev, "Couldn't ioremap I2C I/O region\n");
-		ret = -ENOMEM;
-		goto out_release;
-	}
+	alg_data->ioaddr = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(alg_data->ioaddr))
+		return PTR_ERR(alg_data->ioaddr);
 
 	ret = clk_enable(alg_data->clk);
 	if (ret)
-		goto out_unmap;
+		return ret;
 
 	freq = clk_get_rate(alg_data->clk);
 
@@ -730,8 +714,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 		ret = alg_data->irq;
 		goto out_clock;
 	}
-	ret = request_irq(alg_data->irq, i2c_pnx_interrupt,
-			0, pdev->name, alg_data);
+	ret = devm_request_irq(&pdev->dev, alg_data->irq, i2c_pnx_interrupt,
+			       0, pdev->name, alg_data);
 	if (ret)
 		goto out_clock;
 
@@ -739,7 +723,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	ret = i2c_add_numbered_adapter(&alg_data->adapter);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "I2C: Failed to add bus\n");
-		goto out_irq;
+		goto out_clock;
 	}
 
 	dev_dbg(&pdev->dev, "%s: Master at %#8x, irq %d.\n",
@@ -747,19 +731,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 
 	return 0;
 
-out_irq:
-	free_irq(alg_data->irq, alg_data);
 out_clock:
 	clk_disable(alg_data->clk);
-out_unmap:
-	iounmap(alg_data->ioaddr);
-out_release:
-	release_mem_region(res->start, I2C_PNX_REGION_SIZE);
-out_clkget:
-	clk_put(alg_data->clk);
-out_drvdata:
-	kfree(alg_data);
-err_kzalloc:
 	return ret;
 }
 
@@ -767,13 +740,8 @@ static int i2c_pnx_remove(struct platform_device *pdev)
 {
 	struct i2c_pnx_algo_data *alg_data = platform_get_drvdata(pdev);
 
-	free_irq(alg_data->irq, alg_data);
 	i2c_del_adapter(&alg_data->adapter);
 	clk_disable(alg_data->clk);
-	iounmap(alg_data->ioaddr);
-	release_mem_region(alg_data->base, I2C_PNX_REGION_SIZE);
-	clk_put(alg_data->clk);
-	kfree(alg_data);
 
 	return 0;
 }
-- 
1.7.10.4

Re: [PATCH V2 07/13] i2c: pnx: Use devm_*() functions

[Roland Stigge]

On 01/06/2014 03:39 AM, Jingoo Han wrote:
> Use devm_*() functions to make cleanup paths simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changes since V1:
> - Use devm_ioremap_resource() to make the code simpler, per Wolfram Sang.
> 
>  drivers/i2c/busses/i2c-pnx.c |   60 ++++++++++--------------------------------
>  1 file changed, 14 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
> index c9a352f..5f9cb2d 100644
> --- a/drivers/i2c/busses/i2c-pnx.c
> +++ b/drivers/i2c/busses/i2c-pnx.c
> @@ -628,11 +628,9 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	u32 speed = I2C_PNX_SPEED_KHZ_DEFAULT * 1000;
>  
> -	alg_data = kzalloc(sizeof(*alg_data), GFP_KERNEL);
> -	if (!alg_data) {
> -		ret = -ENOMEM;
> -		goto err_kzalloc;
> -	}
> +	alg_data = devm_kzalloc(&pdev->dev, sizeof(*alg_data), GFP_KERNEL);
> +	if (!alg_data)
> +		return -ENOMEM;
>  
>  	platform_set_drvdata(pdev, alg_data);
>  
> @@ -657,11 +655,9 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  		 */
>  	}
>  #endif
> -	alg_data->clk = clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(alg_data->clk)) {
> -		ret = PTR_ERR(alg_data->clk);
> -		goto out_drvdata;
> -	}
> +	alg_data->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(alg_data->clk))
> +		return PTR_ERR(alg_data->clk);
>  
>  	init_timer(&alg_data->mif.timer);
>  	alg_data->mif.timer.function = i2c_pnx_timeout;
> @@ -674,29 +670,17 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
>  		dev_err(&pdev->dev, "Unable to get mem resource.\n");
> -		ret = -EBUSY;
> -		goto out_clkget;
> -	}
> -	if (!request_mem_region(res->start, I2C_PNX_REGION_SIZE,
> -				pdev->name)) {
> -		dev_err(&pdev->dev,
> -		       "I/O region 0x%08x for I2C already in use.\n",
> -		       res->start);
> -		ret = -ENOMEM;
> -		goto out_clkget;
> +		return -EBUSY;
>  	}
>  
>  	alg_data->base = res->start;
> -	alg_data->ioaddr = ioremap(res->start, I2C_PNX_REGION_SIZE);
> -	if (!alg_data->ioaddr) {
> -		dev_err(&pdev->dev, "Couldn't ioremap I2C I/O region\n");
> -		ret = -ENOMEM;
> -		goto out_release;
> -	}
> +	alg_data->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(alg_data->ioaddr))
> +		return PTR_ERR(alg_data->ioaddr);
>  
>  	ret = clk_enable(alg_data->clk);
>  	if (ret)
> -		goto out_unmap;
> +		return ret;
>  
>  	freq = clk_get_rate(alg_data->clk);
>  
> @@ -730,8 +714,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  		ret = alg_data->irq;
>  		goto out_clock;
>  	}
> -	ret = request_irq(alg_data->irq, i2c_pnx_interrupt,
> -			0, pdev->name, alg_data);
> +	ret = devm_request_irq(&pdev->dev, alg_data->irq, i2c_pnx_interrupt,
> +			       0, pdev->name, alg_data);
>  	if (ret)
>  		goto out_clock;
>  
> @@ -739,7 +723,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  	ret = i2c_add_numbered_adapter(&alg_data->adapter);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "I2C: Failed to add bus\n");
> -		goto out_irq;
> +		goto out_clock;
>  	}
>  
>  	dev_dbg(&pdev->dev, "%s: Master at %#8x, irq %d.\n",
> @@ -747,19 +731,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -out_irq:
> -	free_irq(alg_data->irq, alg_data);
>  out_clock:
>  	clk_disable(alg_data->clk);
> -out_unmap:
> -	iounmap(alg_data->ioaddr);
> -out_release:
> -	release_mem_region(res->start, I2C_PNX_REGION_SIZE);
> -out_clkget:
> -	clk_put(alg_data->clk);
> -out_drvdata:
> -	kfree(alg_data);
> -err_kzalloc:
>  	return ret;
>  }
>  
> @@ -767,13 +740,8 @@ static int i2c_pnx_remove(struct platform_device *pdev)
>  {
>  	struct i2c_pnx_algo_data *alg_data = platform_get_drvdata(pdev);
>  
> -	free_irq(alg_data->irq, alg_data);
>  	i2c_del_adapter(&alg_data->adapter);
>  	clk_disable(alg_data->clk);
> -	iounmap(alg_data->ioaddr);
> -	release_mem_region(alg_data->base, I2C_PNX_REGION_SIZE);
> -	clk_put(alg_data->clk);
> -	kfree(alg_data);
>  
>  	return 0;
>  }
> 

Acked-by: Roland Stigge <stigge-uj/7R2tJ6VmzQB+pC5nmwQ@public.gmane.org>

Thanks!

Re: [PATCH V2 07/13] i2c: pnx: Use devm_*() functions

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

On Mon, Jan 06, 2014 at 11:39:29AM +0900, Jingoo Han wrote:
> Use devm_*() functions to make cleanup paths simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changes since V1:
> - Use devm_ioremap_resource() to make the code simpler, per Wolfram Sang.

Yes, you used this function but you did not remove all the stuff which
can go now since you use this function.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V2 07/13] i2c: pnx: Use devm_*() functions

[Jingoo Han]

On Friday, January 10, 2014 6:24 AM, Wolfram Sang wrote:
> On Mon, Jan 06, 2014 at 11:39:29AM +0900, Jingoo Han wrote:
> > Use devm_*() functions to make cleanup paths simpler.
> >
> > Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> > Changes since V1:
> > - Use devm_ioremap_resource() to make the code simpler, per Wolfram Sang.
> 
> Yes, you used this function but you did not remove all the stuff which
> can go now since you use this function.

Hi Wolfram,

Which thing do I have to remove additionally?
You mean the following?

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
	dev_err(&pdev->dev, "Unable to get mem resource.\n");

I think that this error message is not necessary. So, I will
remove it. Please let me know additional things to be remove.
Thank you.

Best regards,
Jingoo Han

Re: [PATCH V2 07/13] i2c: pnx: Use devm_*() functions

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Hi,

> Which thing do I have to remove additionally?

Just have a close look at your patches and what you removed. You'll
figure. Hint: It is related to the resource.

> You mean the following?
> 
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res) {
> 	dev_err(&pdev->dev, "Unable to get mem resource.\n");

That's one thing, there is more.

> 
> I think that this error message is not necessary. So, I will
> remove it.

Yeah, just have a look at the actual implementation of
devm_ioremap_resource and you will *know* that it is not necessary.

> Please let me know additional things to be remove.

I believe in teaching fishing rather than giving fish.

Regards,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V2 07/13] i2c: pnx: Use devm_*() functions

[Jingoo Han]

On Monday, January 13, 2014 9:29 PM, Wolfram Sang wrote:
> 
> > Which thing do I have to remove additionally?
> 
> Just have a close look at your patches and what you removed. You'll
> figure. Hint: It is related to the resource.
> 
> > You mean the following?
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > if (!res) {
> > 	dev_err(&pdev->dev, "Unable to get mem resource.\n");
> 
> That's one thing, there is more.
> 
> >
> > I think that this error message is not necessary. So, I will
> > remove it.
> 
> Yeah, just have a look at the actual implementation of
> devm_ioremap_resource and you will *know* that it is not necessary.

Aha, I see. Sorry, I forgot it.
The return value of platform_get_resource() is checked by
devm_ioremap_resource(); thus, the redundant return value check
of platform_get_resource() should be removed as below:

Also, 'alg_data->base' is not used anymore, so it should be
removed too.

	/* Register I/O resource */
	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	alg_data->ioaddr = devm_ioremap_resource(&pdev->dev, res);
	if (IS_ERR(alg_data->ioaddr))
		return PTR_ERR(alg_data->ioaddr);

> 
> > Please let me know additional things to be remove.
> 
> I believe in teaching fishing rather than giving fish.

Thank you for your guidance. :-)
I will send the v3 patch, soon.

Best regards,
Jingoo Han

[PATCH 08/13] i2c: pxa: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |   66 ++++++++++++------------------------------
 1 file changed, 18 insertions(+), 48 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index bbe6dfb..be392b0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1141,11 +1141,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	struct resource *res = NULL;
 	int ret, irq;
 
-	i2c = kzalloc(sizeof(struct pxa_i2c), GFP_KERNEL);
-	if (!i2c) {
-		ret = -ENOMEM;
-		goto emalloc;
-	}
+	i2c = devm_kzalloc(&dev->dev, sizeof(struct pxa_i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
 
 	/* Default adapter num to device id; i2c_pxa_probe_dt can override. */
 	i2c->adap.nr = dev->id;
@@ -1154,19 +1152,12 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	if (ret > 0)
 		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
 	if (ret < 0)
-		goto eclk;
+		return ret;
 
 	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(dev, 0);
-	if (res == NULL || irq < 0) {
-		ret = -ENODEV;
-		goto eclk;
-	}
-
-	if (!request_mem_region(res->start, resource_size(res), res->name)) {
-		ret = -ENOMEM;
-		goto eclk;
-	}
+	if (res == NULL || irq < 0)
+		return -ENODEV;
 
 	i2c->adap.owner   = THIS_MODULE;
 	i2c->adap.retries = 5;
@@ -1176,17 +1167,14 @@ static int i2c_pxa_probe(struct platform_device *dev)
 
 	strlcpy(i2c->adap.name, "pxa_i2c-i2c", sizeof(i2c->adap.name));
 
-	i2c->clk = clk_get(&dev->dev, NULL);
-	if (IS_ERR(i2c->clk)) {
-		ret = PTR_ERR(i2c->clk);
-		goto eclk;
-	}
+	i2c->clk = devm_clk_get(&dev->dev, NULL);
+	if (IS_ERR(i2c->clk))
+		return PTR_ERR(i2c->clk);
+
+	i2c->reg_base = devm_ioremap_resource(&dev->dev, res);
+	if (IS_ERR(i2c->reg_base))
+		return PTR_ERR(i2c->reg_base);
 
-	i2c->reg_base = ioremap(res->start, resource_size(res));
-	if (!i2c->reg_base) {
-		ret = -EIO;
-		goto eremap;
-	}
 
 	i2c->reg_ibmr = i2c->reg_base + pxa_reg_layout[i2c_type].ibmr;
 	i2c->reg_idbr = i2c->reg_base + pxa_reg_layout[i2c_type].idbr;
@@ -1227,10 +1215,10 @@ static int i2c_pxa_probe(struct platform_device *dev)
 		i2c->adap.algo = &i2c_pxa_pio_algorithm;
 	} else {
 		i2c->adap.algo = &i2c_pxa_algorithm;
-		ret = request_irq(irq, i2c_pxa_handler, IRQF_SHARED,
-				  dev_name(&dev->dev), i2c);
+		ret = devm_request_irq(&dev->dev, irq, i2c_pxa_handler,
+				       IRQF_SHARED, dev_name(&dev->dev), i2c);
 		if (ret)
-			goto ereqirq;
+			goto err;
 	}
 
 	i2c_pxa_reset(i2c);
@@ -1244,7 +1232,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	ret = i2c_add_numbered_adapter(&i2c->adap);
 	if (ret < 0) {
 		printk(KERN_INFO "I2C: Failed to add bus\n");
-		goto eadapt;
+		goto err;
 	}
 
 	platform_set_drvdata(dev, i2c);
@@ -1258,18 +1246,8 @@ static int i2c_pxa_probe(struct platform_device *dev)
 #endif
 	return 0;
 
-eadapt:
-	if (!i2c->use_pio)
-		free_irq(irq, i2c);
-ereqirq:
+err:
 	clk_disable_unprepare(i2c->clk);
-	iounmap(i2c->reg_base);
-eremap:
-	clk_put(i2c->clk);
-eclk:
-	kfree(i2c);
-emalloc:
-	release_mem_region(res->start, resource_size(res));
 	return ret;
 }
 
@@ -1278,15 +1256,7 @@ static int i2c_pxa_remove(struct platform_device *dev)
 	struct pxa_i2c *i2c = platform_get_drvdata(dev);
 
 	i2c_del_adapter(&i2c->adap);
-	if (!i2c->use_pio)
-		free_irq(i2c->irq, i2c);
-
 	clk_disable_unprepare(i2c->clk);
-	clk_put(i2c->clk);
-
-	iounmap(i2c->reg_base);
-	release_mem_region(i2c->iobase, i2c->iosize);
-	kfree(i2c);
 
 	return 0;
 }
-- 
1.7.10.4

[PATCH 09/13] i2c: simtec: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-simtec.c |   43 +++++++--------------------------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/drivers/i2c/busses/i2c-simtec.c b/drivers/i2c/busses/i2c-simtec.c
index 4fc87e7..8236cbb 100644
--- a/drivers/i2c/busses/i2c-simtec.c
+++ b/drivers/i2c/busses/i2c-simtec.c
@@ -74,10 +74,10 @@ static int simtec_i2c_probe(struct platform_device *dev)
 {
 	struct simtec_i2c_data *pd;
 	struct resource *res;
-	int size;
 	int ret;
 
-	pd = kzalloc(sizeof(struct simtec_i2c_data), GFP_KERNEL);
+	pd = devm_kzalloc(&dev->dev, sizeof(struct simtec_i2c_data),
+			  GFP_KERNEL);
 	if (pd == NULL) {
 		dev_err(&dev->dev, "cannot allocate private data\n");
 		return -ENOMEM;
@@ -88,25 +88,12 @@ static int simtec_i2c_probe(struct platform_device *dev)
 	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	if (res == NULL) {
 		dev_err(&dev->dev, "cannot find IO resource\n");
-		ret = -ENOENT;
-		goto err;
+		return -ENOENT;
 	}
 
-	size = resource_size(res);
-
-	pd->ioarea = request_mem_region(res->start, size, dev->name);
-	if (pd->ioarea == NULL) {
-		dev_err(&dev->dev, "cannot request IO\n");
-		ret = -ENXIO;
-		goto err;
-	}
-
-	pd->reg = ioremap(res->start, size);
-	if (pd->reg == NULL) {
-		dev_err(&dev->dev, "cannot map IO\n");
-		ret = -ENXIO;
-		goto err_res;
-	}
+	pd->reg = devm_ioremap_resource(&dev->dev, res);
+	if (IS_ERR(pd->reg))
+		return PTR_ERR(pd->reg);
 
 	/* setup the private data */
 
@@ -126,20 +113,9 @@ static int simtec_i2c_probe(struct platform_device *dev)
 
 	ret = i2c_bit_add_bus(&pd->adap);
 	if (ret)
-		goto err_all;
+		return ret;
 
 	return 0;
-
- err_all:
-	iounmap(pd->reg);
-
- err_res:
-	release_resource(pd->ioarea);
-	kfree(pd->ioarea);
-
- err:
-	kfree(pd);
-	return ret;
 }
 
 static int simtec_i2c_remove(struct platform_device *dev)
@@ -148,11 +124,6 @@ static int simtec_i2c_remove(struct platform_device *dev)
 
 	i2c_del_adapter(&pd->adap);
 
-	iounmap(pd->reg);
-	release_resource(pd->ioarea);
-	kfree(pd->ioarea);
-	kfree(pd);
-
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH 10/13] i2c: sirf: Use devm_clk_get()

[Jingoo Han]

Use devm_clk_get() to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-sirf.c |   11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sirf.c b/drivers/i2c/busses/i2c-sirf.c
index 6784f7f..c8fcddc 100644
--- a/drivers/i2c/busses/i2c-sirf.c
+++ b/drivers/i2c/busses/i2c-sirf.c
@@ -284,17 +284,16 @@ static int i2c_sirfsoc_probe(struct platform_device *pdev)
 	int err;
 	u32 regval;
 
-	clk = clk_get(&pdev->dev, NULL);
+	clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
-		err = PTR_ERR(clk);
 		dev_err(&pdev->dev, "Clock get failed\n");
-		goto err_get_clk;
+		return PTR_ERR(clk);
 	}
 
 	err = clk_prepare(clk);
 	if (err) {
 		dev_err(&pdev->dev, "Clock prepare failed\n");
-		goto err_clk_prep;
+		return err;
 	}
 
 	err = clk_enable(clk);
@@ -387,9 +386,6 @@ out:
 	clk_disable(clk);
 err_clk_en:
 	clk_unprepare(clk);
-err_clk_prep:
-	clk_put(clk);
-err_get_clk:
 	return err;
 }
 
@@ -401,7 +397,6 @@ static int i2c_sirfsoc_remove(struct platform_device *pdev)
 	writel(SIRFSOC_I2C_RESET, siic->base + SIRFSOC_I2C_CTRL);
 	i2c_del_adapter(adapter);
 	clk_unprepare(siic->clk);
-	clk_put(siic->clk);
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH 11/13] i2c: versatile: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-versatile.c |   33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
index 6bb3a89..6bcd477 100644
--- a/drivers/i2c/busses/i2c-versatile.c
+++ b/drivers/i2c/busses/i2c-versatile.c
@@ -71,27 +71,16 @@ static int i2c_versatile_probe(struct platform_device *dev)
 	int ret;
 
 	r = platform_get_resource(dev, IORESOURCE_MEM, 0);
-	if (!r) {
-		ret = -EINVAL;
-		goto err_out;
-	}
+	if (!r)
+		return -EINVAL;
 
-	if (!request_mem_region(r->start, resource_size(r), "versatile-i2c")) {
-		ret = -EBUSY;
-		goto err_out;
-	}
+	i2c = devm_kzalloc(&dev->dev, sizeof(struct i2c_versatile), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
 
-	i2c = kzalloc(sizeof(struct i2c_versatile), GFP_KERNEL);
-	if (!i2c) {
-		ret = -ENOMEM;
-		goto err_release;
-	}
-
-	i2c->base = ioremap(r->start, resource_size(r));
-	if (!i2c->base) {
-		ret = -ENOMEM;
-		goto err_free;
-	}
+	i2c->base = devm_ioremap_resource(&dev->dev, r);
+	if (IS_ERR(i2c->base))
+		return PTR_ERR(i2c->base);
 
 	writel(SCL | SDA, i2c->base + I2C_CONTROLS);
 
@@ -110,12 +99,6 @@ static int i2c_versatile_probe(struct platform_device *dev)
 		return 0;
 	}
 
-	iounmap(i2c->base);
- err_free:
-	kfree(i2c);
- err_release:
-	release_mem_region(r->start, resource_size(r));
- err_out:
 	return ret;
 }
 
-- 
1.7.10.4

[PATCH 12/13] i2c: viperboard: Use devm_kzalloc() functions

[Jingoo Han]

Use devm_kzalloc() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-viperboard.c |   12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-viperboard.c b/drivers/i2c/busses/i2c-viperboard.c
index c68450c..6976e1c 100644
--- a/drivers/i2c/busses/i2c-viperboard.c
+++ b/drivers/i2c/busses/i2c-viperboard.c
@@ -367,7 +367,7 @@ static int vprbrd_i2c_probe(struct platform_device *pdev)
 	int ret;
 	int pipe;
 
-	vb_i2c = kzalloc(sizeof(*vb_i2c), GFP_KERNEL);
+	vb_i2c = devm_kzalloc(&pdev->dev, sizeof(*vb_i2c), GFP_KERNEL);
 	if (vb_i2c == NULL)
 		return -ENOMEM;
 
@@ -394,14 +394,12 @@ static int vprbrd_i2c_probe(struct platform_device *pdev)
 	    if (ret != 1) {
 		dev_err(&pdev->dev,
 			"failure setting i2c_bus_freq to %d\n", i2c_bus_freq);
-		ret = -EIO;
-		goto error;
+		return -EIO;
 	    }
 	} else {
 		dev_err(&pdev->dev,
 			"invalid i2c_bus_freq setting:%d\n", i2c_bus_freq);
-		ret = -EIO;
-		goto error;
+		return -EIO;
 	}
 
 	vb_i2c->i2c.dev.parent = &pdev->dev;
@@ -412,10 +410,6 @@ static int vprbrd_i2c_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, vb_i2c);
 
 	return 0;
-
-error:
-	kfree(vb_i2c);
-	return ret;
 }
 
 static int vprbrd_i2c_remove(struct platform_device *pdev)
-- 
1.7.10.4

Re: [PATCH 12/13] i2c: viperboard: Use devm_kzalloc() functions

[Lars Poeschel]

Am Dienstag, 17. Dezember 2013, 16:01:41 schrieb Jingoo Han:
> Use devm_kzalloc() functions to make cleanup paths simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-viperboard.c |   12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-viperboard.c
> b/drivers/i2c/busses/i2c-viperboard.c index c68450c..6976e1c 100644
> --- a/drivers/i2c/busses/i2c-viperboard.c
> +++ b/drivers/i2c/busses/i2c-viperboard.c
> @@ -367,7 +367,7 @@ static int vprbrd_i2c_probe(struct platform_device
> *pdev) int ret;
>  	int pipe;
> 
> -	vb_i2c = kzalloc(sizeof(*vb_i2c), GFP_KERNEL);
> +	vb_i2c = devm_kzalloc(&pdev->dev, sizeof(*vb_i2c), GFP_KERNEL);
>  	if (vb_i2c == NULL)
>  		return -ENOMEM;
> 
> @@ -394,14 +394,12 @@ static int vprbrd_i2c_probe(struct platform_device
> *pdev) if (ret != 1) {
>  		dev_err(&pdev->dev,
>  			"failure setting i2c_bus_freq to %d\n", i2c_bus_freq);
> -		ret = -EIO;
> -		goto error;
> +		return -EIO;
>  	    }
>  	} else {
>  		dev_err(&pdev->dev,
>  			"invalid i2c_bus_freq setting:%d\n", i2c_bus_freq);
> -		ret = -EIO;
> -		goto error;
> +		return -EIO;
>  	}
> 
>  	vb_i2c->i2c.dev.parent = &pdev->dev;
> @@ -412,10 +410,6 @@ static int vprbrd_i2c_probe(struct platform_device
> *pdev) platform_set_drvdata(pdev, vb_i2c);
> 
>  	return 0;
> -
> -error:
> -	kfree(vb_i2c);
> -	return ret;
>  }
> 
>  static int vprbrd_i2c_remove(struct platform_device *pdev)

Acked-by: Lars Poeschel <poeschel-Xtl8qvBWbHwb1SvskN2V4Q@public.gmane.org>

Re: [PATCH 12/13] i2c: viperboard: Use devm_kzalloc() functions

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

On Tue, Dec 17, 2013 at 04:01:41PM +0900, Jingoo Han wrote:
> Use devm_kzalloc() functions to make cleanup paths simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

OK, this one got an Ack and even fixes a bug (no kfree in remove) what
should have been mentioned in the commit message. Applied to for-next,
thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 13/13] i2c: xiic: Use devm_*() functions

[Jingoo Han]

Use devm_*() functions to make cleanup paths simpler.

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-xiic.c |   59 +++++++++++------------------------------
 1 file changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index fc2716a..ea83dfc 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -696,31 +696,26 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	u8 i;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		goto resource_missing;
+	if (!res) {
+		dev_err(&pdev->dev, "Memory resource is missing\n");
+		return -EINVAL;
+	}
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		goto resource_missing;
+	if (irq < 0) {
+		dev_err(&pdev->dev, "IRQ resource is missing\n");
+		return irq;
+	}
 
 	pdata = dev_get_platdata(&pdev->dev);
 
-	i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
+	i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
 	if (!i2c)
 		return -ENOMEM;
 
-	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
-		dev_err(&pdev->dev, "Memory region busy\n");
-		ret = -EBUSY;
-		goto request_mem_failed;
-	}
-
-	i2c->base = ioremap(res->start, resource_size(res));
-	if (!i2c->base) {
-		dev_err(&pdev->dev, "Unable to map registers\n");
-		ret = -EIO;
-		goto map_failed;
-	}
+	i2c->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(i2c->base))
+		return PTR_ERR(i2c->base);
 
 	/* hook up driver to tree */
 	platform_set_drvdata(pdev, i2c);
@@ -733,17 +728,17 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 
 	spin_lock_init(&i2c->lock);
 	init_waitqueue_head(&i2c->wait);
-	ret = request_irq(irq, xiic_isr, 0, pdev->name, i2c);
+	ret = devm_request_irq(&pdev->dev, irq, xiic_isr, 0, pdev->name, i2c);
 	if (ret) {
 		dev_err(&pdev->dev, "Cannot claim IRQ\n");
-		goto request_irq_failed;
+		goto failed;
 	}
 
 	/* add i2c adapter to i2c tree */
 	ret = i2c_add_adapter(&i2c->adap);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to add adapter\n");
-		goto add_adapter_failed;
+		goto failed;
 	}
 
 	if (pdata) {
@@ -754,42 +749,20 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 
 	return 0;
 
-add_adapter_failed:
-	free_irq(irq, i2c);
-request_irq_failed:
+failed:
 	xiic_deinit(i2c);
-	iounmap(i2c->base);
-map_failed:
-	release_mem_region(res->start, resource_size(res));
-request_mem_failed:
-	kfree(i2c);
-
 	return ret;
-resource_missing:
-	dev_err(&pdev->dev, "IRQ or Memory resource is missing\n");
-	return -ENOENT;
 }
 
 static int xiic_i2c_remove(struct platform_device *pdev)
 {
 	struct xiic_i2c *i2c = platform_get_drvdata(pdev);
-	struct resource *res;
 
 	/* remove adapter & data */
 	i2c_del_adapter(&i2c->adap);
 
 	xiic_deinit(i2c);
 
-	free_irq(platform_get_irq(pdev, 0), i2c);
-
-	iounmap(i2c->base);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (res)
-		release_mem_region(res->start, resource_size(res));
-
-	kfree(i2c);
-
 	return 0;
 }
 
-- 
1.7.10.4

Re: [PATCH 02/13] i2c: gpio: Use devm_gpio_request()

[Violeta Menendez Gonzalez]

Tested on a r8a7790-lager

     Reviewed-by: Violeta Menendez <violeta.menendez-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
     Reviewed-by: Ian Molton <ian.molton-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
     Tested-by: Violeta Menendez <violeta.menendez-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>

Re: [PATCH 02/13] i2c: gpio: Use devm_gpio_request()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

On Wed, Apr 30, 2014 at 02:28:56PM +0100, Violeta Menendez Gonzalez wrote:
> Tested on a r8a7790-lager
> 
>     Reviewed-by: Violeta Menendez <violeta.menendez-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
>     Reviewed-by: Ian Molton <ian.molton-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
>     Tested-by: Violeta Menendez <violeta.menendez-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>

Thanks! Please reply next time to the mail directly, then your tags get
added automatically.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]