Message ID | 1396347954-13740-3-git-send-email-ben.dooks@codethink.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Apr 1, 2014 at 12:25 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > + reg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(reg)) { > dev_err(&pdev->dev, "ioremap error.\n"); devm_ioremap_resource() already calls dev_err() for the various error cases, so you can drop this line. > return -ENOMEM; return PTR_ERR(reg); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/04/14 11:55, Geert Uytterhoeven wrote: > On Tue, Apr 1, 2014 at 12:25 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote: >> + reg = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(reg)) { >> dev_err(&pdev->dev, "ioremap error.\n"); > > devm_ioremap_resource() already calls dev_err() for the various error > cases, so you can drop this line. > >> return -ENOMEM; Oops. Will fix.
Hi Ben, Thank you for the patch. On Tuesday 01 April 2014 11:25:50 Ben Dooks wrote: > Start tidying the probe/release code by using devm_ioremap_resource() to > map the IO registers. > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > --- > drivers/mmc/host/sh_mmcif.c | 20 +++++--------------- > 1 file changed, 5 insertions(+), 15 deletions(-) > > diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c > index c48df98..be6be2b 100644 > --- a/drivers/mmc/host/sh_mmcif.c > +++ b/drivers/mmc/host/sh_mmcif.c > @@ -1379,22 +1379,17 @@ static int sh_mmcif_probe(struct platform_device > *pdev) dev_err(&pdev->dev, "Get irq error\n"); > return -ENXIO; > } > + > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (!res) { > - dev_err(&pdev->dev, "platform_get_resource error.\n"); > - return -ENXIO; > - } > - reg = ioremap(res->start, resource_size(res)); > - if (!reg) { > + reg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(reg)) { > dev_err(&pdev->dev, "ioremap error.\n"); devm_ioremap_resource already prints an error message on failure, you can this remove this one. > return -ENOMEM; What about returning PTR_ERR(reg) instead ? > } > > mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), &pdev->dev); > - if (!mmc) { > - ret = -ENOMEM; > - goto ealloch; > - } > + if (!mmc) > + return -ENOMEM; > > ret = mmc_of_parse(mmc); > if (ret < 0) > @@ -1499,8 +1494,6 @@ eclkget: > pm_runtime_disable(&pdev->dev); > eofparse: > mmc_free_host(mmc); > -ealloch: > - iounmap(reg); > return ret; > } > > @@ -1525,9 +1518,6 @@ static int sh_mmcif_remove(struct platform_device > *pdev) */ > cancel_delayed_work_sync(&host->timeout_work); > > - if (host->addr) > - iounmap(host->addr); > - > irq[0] = platform_get_irq(pdev, 0); > irq[1] = platform_get_irq(pdev, 1);
Hello. On 01-04-2014 14:25, Ben Dooks wrote: > Start tidying the probe/release code by using devm_ioremap_resource() to > map the IO registers. > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > --- > drivers/mmc/host/sh_mmcif.c | 20 +++++--------------- > 1 file changed, 5 insertions(+), 15 deletions(-) > diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c > index c48df98..be6be2b 100644 > --- a/drivers/mmc/host/sh_mmcif.c > +++ b/drivers/mmc/host/sh_mmcif.c > @@ -1379,22 +1379,17 @@ static int sh_mmcif_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "Get irq error\n"); > return -ENXIO; > } > + > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (!res) { > - dev_err(&pdev->dev, "platform_get_resource error.\n"); > - return -ENXIO; > - } > - reg = ioremap(res->start, resource_size(res)); > - if (!reg) { > + reg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(reg)) { > dev_err(&pdev->dev, "ioremap error.\n"); > return -ENOMEM; Should propagate devm_ioremap_resource() error instead. [...] WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c index c48df98..be6be2b 100644 --- a/drivers/mmc/host/sh_mmcif.c +++ b/drivers/mmc/host/sh_mmcif.c @@ -1379,22 +1379,17 @@ static int sh_mmcif_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Get irq error\n"); return -ENXIO; } + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) { - dev_err(&pdev->dev, "platform_get_resource error.\n"); - return -ENXIO; - } - reg = ioremap(res->start, resource_size(res)); - if (!reg) { + reg = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(reg)) { dev_err(&pdev->dev, "ioremap error.\n"); return -ENOMEM; } mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), &pdev->dev); - if (!mmc) { - ret = -ENOMEM; - goto ealloch; - } + if (!mmc) + return -ENOMEM; ret = mmc_of_parse(mmc); if (ret < 0) @@ -1499,8 +1494,6 @@ eclkget: pm_runtime_disable(&pdev->dev); eofparse: mmc_free_host(mmc); -ealloch: - iounmap(reg); return ret; } @@ -1525,9 +1518,6 @@ static int sh_mmcif_remove(struct platform_device *pdev) */ cancel_delayed_work_sync(&host->timeout_work); - if (host->addr) - iounmap(host->addr); - irq[0] = platform_get_irq(pdev, 0); irq[1] = platform_get_irq(pdev, 1);
Start tidying the probe/release code by using devm_ioremap_resource() to map the IO registers. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- drivers/mmc/host/sh_mmcif.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-)