| Message ID | 1392300547-23365-1-git-send-email-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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()!
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
> 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 --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; }
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(-)