diff mbox

[V2,11/17] i2c: nomadik: Convert to devm functions

Message ID 1392300547-23365-1-git-send-email-ulf.hansson@linaro.org (mailing list archive)
State Not Applicable
Headers show

Commit Message

Ulf Hansson Feb. 13, 2014, 2:09 p.m. UTC
Use devm_* functions to simplify code and error handling.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

Comments

Wolfram Sang Feb. 15, 2014, 3:03 p.m. UTC | #1
On Thu, Feb 13, 2014 at 03:09:02PM +0100, Ulf Hansson wrote:
> Use devm_* functions to simplify code and error handling.
> 
> Cc: Alessandro Rubini <rubini@unipv.it>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	Rebased on top of latest i2c-nomadik branch.

Since this depends on Linus' patch already, I think it would be cleaner
if I pick this kinda unrelated (but wanted) devm patch, and ack the
PM stuff. If this for some reason makes things more complicated, I can
also simply ack this one.

> -	dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
> +	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
> +				resource_size(&adev->res));
>  	if (!dev->virtbase) {
>  		ret = -ENOMEM;

IS_ERR()!
Ulf Hansson Feb. 16, 2014, 1:44 p.m. UTC | #2
On 15 February 2014 16:03, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Feb 13, 2014 at 03:09:02PM +0100, Ulf Hansson wrote:
>> Use devm_* functions to simplify code and error handling.
>>
>> Cc: Alessandro Rubini <rubini@unipv.it>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Wolfram Sang <wsa@the-dreams.de>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>
>> Changes in v2:
>>       Rebased on top of latest i2c-nomadik branch.
>
> Since this depends on Linus' patch already, I think it would be cleaner
> if I pick this kinda unrelated (but wanted) devm patch, and ack the
> PM stuff. If this for some reason makes things more complicated, I can
> also simply ack this one.

I think we will end up having merge conflicts, especially for the
changes in the probe function if we decide to split it.

Would a way forward be to let you carry all the patches through your
tree? I believe all but patch 17 can be safely merged. It is only this
one that depends on the changes in the amba bus, so we can put this
one on hold for a while.

Another option would be if you drop Linus' patch from you branch and
let him collect all the patches in a pull request instead?

I happy with whatever we thinks are easiest. :-)

>
>> -     dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
>> +     dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
>> +                             resource_size(&adev->res));
>>       if (!dev->virtbase) {
>>               ret = -ENOMEM;
>
> IS_ERR()!

Will fix in a v2!

>

Thanks!

Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Feb. 16, 2014, 5:02 p.m. UTC | #3
> Would a way forward be to let you carry all the patches through your
> tree? I believe all but patch 17 can be safely merged. It is only this
> one that depends on the changes in the amba bus, so we can put this
> one on hold for a while.

I'd favour this. Will do this next week unless somebody objects.
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index bc8ba02..5faf9e2 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -981,7 +981,7 @@  static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		return -ENODEV;
 	}
 
-	dev = kzalloc(sizeof(struct nmk_i2c_dev), GFP_KERNEL);
+	dev = devm_kzalloc(&adev->dev, sizeof(struct nmk_i2c_dev), GFP_KERNEL);
 	if (!dev) {
 		dev_err(&adev->dev, "cannot allocate memory\n");
 		ret = -ENOMEM;
@@ -1011,27 +1011,28 @@  static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	/* If possible, let's go to idle until the first transfer */
 	pinctrl_pm_select_idle_state(&adev->dev);
 
-	dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
+	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
+				resource_size(&adev->res));
 	if (!dev->virtbase) {
 		ret = -ENOMEM;
-		goto err_no_ioremap;
+		goto err_no_mem;
 	}
 
 	dev->irq = adev->irq[0];
-	ret = request_irq(dev->irq, i2c_irq_handler, 0,
+	ret = devm_request_irq(&adev->dev, dev->irq, i2c_irq_handler, 0,
 				DRIVER_NAME, dev);
 	if (ret) {
 		dev_err(&adev->dev, "cannot claim the irq %d\n", dev->irq);
-		goto err_irq;
+		goto err_no_mem;
 	}
 
 	pm_suspend_ignore_children(&adev->dev, true);
 
-	dev->clk = clk_get(&adev->dev, NULL);
+	dev->clk = devm_clk_get(&adev->dev, NULL);
 	if (IS_ERR(dev->clk)) {
 		dev_err(&adev->dev, "could not get i2c clock\n");
 		ret = PTR_ERR(dev->clk);
-		goto err_no_clk;
+		goto err_no_mem;
 	}
 
 	adap = &dev->adap;
@@ -1053,21 +1054,13 @@  static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	ret = i2c_add_adapter(adap);
 	if (ret) {
 		dev_err(&adev->dev, "failed to add adapter\n");
-		goto err_add_adap;
+		goto err_no_mem;
 	}
 
 	pm_runtime_put(&adev->dev);
 
 	return 0;
 
- err_add_adap:
-	clk_put(dev->clk);
- err_no_clk:
-	free_irq(dev->irq, dev);
- err_irq:
-	iounmap(dev->virtbase);
- err_no_ioremap:
-	kfree(dev);
  err_no_mem:
 
 	return ret;
@@ -1084,13 +1077,9 @@  static int nmk_i2c_remove(struct amba_device *adev)
 	clear_all_interrupts(dev);
 	/* disable the controller */
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
-	free_irq(dev->irq, dev);
-	iounmap(dev->virtbase);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
-	clk_put(dev->clk);
 	pm_runtime_disable(&adev->dev);
-	kfree(dev);
 
 	return 0;
 }