Message ID | 1393923710-10377-3-git-send-email-peter.ujfalusi@ti.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
0JLRgtC+0YDQvdC40LosICA0INC80LDRgNGC0LAgMjAxNCwgMTE6MDEgKzAyOjAwINC+0YIgUGV0 ZXIgVWpmYWx1c2kgPHBldGVyLnVqZmFsdXNpQHRpLmNvbT46Cj4gV2UgY2FuIHRoZW4gcmVtb3Zl IHRoZSBpb3VubWFwKCkgY2FsbHMgZnJvbSBwcm9iZSBhbmQgcmVtb3ZlLgo+IFNpbmNlIHRoZSBk cml2ZXIgcmVxdWVzdHMgdGhlIHJlc291cmNlcyB2aWEgaW5kZXggd2UgY2FuIGRvIHRoZSBtZW0g cmVzb3VyY2UKPiByZXF1ZXN0IHdpdGhpbiBhIGZvciBsb29wLgo+IAo+IFNpZ25lZC1vZmYtYnk6 IFBldGVyIFVqZmFsdXNpIDxwZXRlci51amZhbHVzaUB0aS5jb20+Cj4gUmV2aWV3ZWQtYnk6IFNh bnRvc2ggU2hpbGlta2FyIDxzYW50b3NoLnNoaWxpbWthckB0aT4KPiAtLS0KCj4gKwkvKiBHZXQg bWVtIHJlc291cmNlcyAqLwo+ICsJZm9yIChpID0gMDsgaSA8IDM7IGkrKykgewo+ICsJCXN0cnVj dCByZXNvdXJjZQkqcmVzID0gcGxhdGZvcm1fZ2V0X3Jlc291cmNlKHBkZXYsCj4gKwkJCQkJCQkg ICAgIElPUkVTT1VSQ0VfTUVNLCBpKTsKPiArCQlpZiAoIXJlcykgewo+ICsJCQlkZXZfZXJyKCZw ZGV2LT5kZXYsICJjb3VsZG4ndCBmaW5kIHJlc291cmNlICVkXG4iLCBpKTsKPiArCQkJcmV0dXJu IC1FTk9ERVY7Cj4gKwkJfQoKTm8gbmVlZCB0byBjaGVjayAicmVzIi4gZGV2bV9yZXF1ZXN0X2Fu ZF9pb3JlbWFwKCkgZG8gYWxsIGZvciB1cy4KCj4gLQlsMy0+bDNfYmFzZVsyXSA9IGlvcmVtYXAo cmVzLT5zdGFydCwgcmVzb3VyY2Vfc2l6ZShyZXMpKTsKPiAtCWlmICghbDMtPmwzX2Jhc2VbMl0p IHsKPiAtCQlkZXZfZXJyKCZwZGV2LT5kZXYsICJpb3JlbWFwIGZhaWxlZFxuIik7Cj4gLQkJcmV0 ID0gLUVOT01FTTsKPiAtCQlnb3RvIGVycjI7Cj4gKwkJbDMtPmwzX2Jhc2VbaV0gPSBkZXZtX3Jl cXVlc3RfYW5kX2lvcmVtYXAoJnBkZXYtPmRldiwgcmVzKTsKPiArCQlpZiAoIWwzLT5sM19iYXNl W2ldKSB7CgppZiAoSVNfRVJSKGwzLT5sM19iYXNlW2ldKSkKCj4gKwkJCWRldl9lcnIoJnBkZXYt PmRldiwgImlvcmVtYXAgJWQgZmFpbGVkXG4iLCBpKTsKClVubmVjZXNzYXJ5LgoKPiArCQkJcmV0 dXJuIC1FTk9NRU07CgpyZXR1cm4gUFRSX0VSUihsMy0+bDNfYmFzZVtpXSk7CgotLS0K -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
???????, 4 ????? 2014, 13:12 +04:00 ?? Alexander Shiyan <shc_work@mail.ru>: > ???????, 4 ????? 2014, 11:01 +02:00 ?? Peter Ujfalusi <peter.ujfalusi@ti.com>: > > We can then remove the iounmap() calls from probe and remove. > > Since the driver requests the resources via index we can do the mem resource > > request within a for loop. > > > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti> > > --- > > > + /* Get mem resources */ > > + for (i = 0; i < 3; i++) { > > + struct resource *res = platform_get_resource(pdev, > > + IORESOURCE_MEM, i); > > + if (!res) { > > + dev_err(&pdev->dev, "couldn't find resource %d\n", i); > > + return -ENODEV; > > + } > > No need to check "res". devm_request_and_ioremap() do all for us. > > > - l3->l3_base[2] = ioremap(res->start, resource_size(res)); > > - if (!l3->l3_base[2]) { > > - dev_err(&pdev->dev, "ioremap failed\n"); > > - ret = -ENOMEM; > > - goto err2; > > + l3->l3_base[i] = devm_request_and_ioremap(&pdev->dev, res); > > + if (!l3->l3_base[i]) { > > if (IS_ERR(l3->l3_base[i])) Ahh, I messed up this with devm_ioremap_resource(). However, if there is reason to use devm_request_and_ioremap() here? ---
On 03/04/2014 11:23 AM, Alexander Shiyan wrote: >>> - l3->l3_base[2] = ioremap(res->start, resource_size(res)); >>> - if (!l3->l3_base[2]) { >>> - dev_err(&pdev->dev, "ioremap failed\n"); >>> - ret = -ENOMEM; >>> - goto err2; >>> + l3->l3_base[i] = devm_request_and_ioremap(&pdev->dev, res); >>> + if (!l3->l3_base[i]) { >> >> if (IS_ERR(l3->l3_base[i])) > > Ahh, I messed up this with devm_ioremap_resource(). > However, if there is reason to use devm_request_and_ioremap() here? devm_request_and_ioremap() is just a wrapper for devm_ioremap_resource(). It returns NULL in case of error (eats up the error code at the same time). I don't have strong preference on this so I can change it to devm_ioremap_resource()
On 03/04/2014 11:12 AM, Alexander Shiyan wrote: > ???????, 4 ????? 2014, 11:01 +02:00 ?? Peter Ujfalusi <peter.ujfalusi@ti.com>: >> We can then remove the iounmap() calls from probe and remove. >> Since the driver requests the resources via index we can do the mem resource >> request within a for loop. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti> >> --- > >> + /* Get mem resources */ >> + for (i = 0; i < 3; i++) { >> + struct resource *res = platform_get_resource(pdev, >> + IORESOURCE_MEM, i); >> + if (!res) { >> + dev_err(&pdev->dev, "couldn't find resource %d\n", i); >> + return -ENODEV; >> + } > > No need to check "res". devm_request_and_ioremap() do all for us. True.
On 03/04/2014 11:12 AM, Alexander Shiyan wrote: > ???????, 4 ????? 2014, 11:01 +02:00 ?? Peter Ujfalusi <peter.ujfalusi@ti.com>: >> We can then remove the iounmap() calls from probe and remove. >> Since the driver requests the resources via index we can do the mem resource >> request within a for loop. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti> >> --- > >> + /* Get mem resources */ >> + for (i = 0; i < 3; i++) { >> + struct resource *res = platform_get_resource(pdev, >> + IORESOURCE_MEM, i); >> + if (!res) { >> + dev_err(&pdev->dev, "couldn't find resource %d\n", i); >> + return -ENODEV; >> + } > > No need to check "res". devm_request_and_ioremap() do all for us. > >> - l3->l3_base[2] = ioremap(res->start, resource_size(res)); >> - if (!l3->l3_base[2]) { >> - dev_err(&pdev->dev, "ioremap failed\n"); >> - ret = -ENOMEM; >> - goto err2; >> + l3->l3_base[i] = devm_request_and_ioremap(&pdev->dev, res); >> + if (!l3->l3_base[i]) { > > if (IS_ERR(l3->l3_base[i])) > >> + dev_err(&pdev->dev, "ioremap %d failed\n", i); > > Unnecessary. I'm going to keep this since it tells which resource of the three has failed. > >> + return -ENOMEM; > > return PTR_ERR(l3->l3_base[i]); > > --- >
diff --git a/drivers/bus/omap_l3_noc.c b/drivers/bus/omap_l3_noc.c index d25d727e7cfb..ca95d3db5055 100644 --- a/drivers/bus/omap_l3_noc.c +++ b/drivers/bus/omap_l3_noc.c @@ -131,52 +131,28 @@ static irqreturn_t l3_interrupt_handler(int irq, void *_l3) static int omap4_l3_probe(struct platform_device *pdev) { static struct omap4_l3 *l3; - struct resource *res; - int ret; + int ret, i; l3 = devm_kzalloc(&pdev->dev, sizeof(*l3), GFP_KERNEL); if (!l3) return -ENOMEM; platform_set_drvdata(pdev, l3); - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) { - dev_err(&pdev->dev, "couldn't find resource 0\n"); - return -ENODEV; - } - - l3->l3_base[0] = ioremap(res->start, resource_size(res)); - if (!l3->l3_base[0]) { - dev_err(&pdev->dev, "ioremap failed\n"); - return -ENOMEM; - } - - res = platform_get_resource(pdev, IORESOURCE_MEM, 1); - if (!res) { - dev_err(&pdev->dev, "couldn't find resource 1\n"); - ret = -ENODEV; - goto err1; - } - l3->l3_base[1] = ioremap(res->start, resource_size(res)); - if (!l3->l3_base[1]) { - dev_err(&pdev->dev, "ioremap failed\n"); - ret = -ENOMEM; - goto err1; - } - - res = platform_get_resource(pdev, IORESOURCE_MEM, 2); - if (!res) { - dev_err(&pdev->dev, "couldn't find resource 2\n"); - ret = -ENODEV; - goto err2; - } + /* Get mem resources */ + for (i = 0; i < 3; i++) { + struct resource *res = platform_get_resource(pdev, + IORESOURCE_MEM, i); + if (!res) { + dev_err(&pdev->dev, "couldn't find resource %d\n", i); + return -ENODEV; + } - l3->l3_base[2] = ioremap(res->start, resource_size(res)); - if (!l3->l3_base[2]) { - dev_err(&pdev->dev, "ioremap failed\n"); - ret = -ENOMEM; - goto err2; + l3->l3_base[i] = devm_request_and_ioremap(&pdev->dev, res); + if (!l3->l3_base[i]) { + dev_err(&pdev->dev, "ioremap %d failed\n", i); + return -ENOMEM; + } } /* @@ -189,7 +165,7 @@ static int omap4_l3_probe(struct platform_device *pdev) if (ret) { pr_crit("L3: request_irq failed to register for 0x%x\n", l3->debug_irq); - goto err3; + return ret; } l3->app_irq = platform_get_irq(pdev, 1); @@ -206,12 +182,6 @@ static int omap4_l3_probe(struct platform_device *pdev) err4: free_irq(l3->debug_irq, l3); -err3: - iounmap(l3->l3_base[2]); -err2: - iounmap(l3->l3_base[1]); -err1: - iounmap(l3->l3_base[0]); return ret; } @@ -221,9 +191,6 @@ static int omap4_l3_remove(struct platform_device *pdev) free_irq(l3->app_irq, l3); free_irq(l3->debug_irq, l3); - iounmap(l3->l3_base[0]); - iounmap(l3->l3_base[1]); - iounmap(l3->l3_base[2]); return 0; }