| Message ID | 1396279100-2920-3-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 03/31/2014 05:18 PM, Ulf Hansson wrote: > Converting to devm functions to simplify error handling in ->probe() and > to cleanup ->remove(). > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/mmc/host/mmci.c | 51 ++++++++++++++++++----------------------------- > 1 file changed, 19 insertions(+), 32 deletions(-) > > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > index b0b81ac..d6f20ba 100644 > --- a/drivers/mmc/host/mmci.c > +++ b/drivers/mmc/host/mmci.c > @@ -13,6 +13,7 @@ > #include <linux/init.h> > #include <linux/ioport.h> > #include <linux/device.h> > +#include <linux/io.h> > #include <linux/interrupt.h> > #include <linux/kernel.h> > #include <linux/slab.h> > @@ -1456,15 +1457,13 @@ static int mmci_probe(struct amba_device *dev, > if (np) > mmci_dt_populate_generic_pdata(np, plat); > > - ret = amba_request_regions(dev, DRIVER_NAME); > - if (ret) > - goto out; > + if (!devm_request_mem_region(&dev->dev, dev->res.start, > + resource_size(&dev->res), DRIVER_NAME)) > + return -ENOMEM; here. Look below. > > mmc = mmc_alloc_host(sizeof(struct mmci_host), &dev->dev); > - if (!mmc) { > - ret = -ENOMEM; > - goto rel_regions; > - } > + if (!mmc) > + return -ENOMEM; > > host = mmc_priv(mmc); > host->mmc = mmc; > @@ -1500,8 +1499,10 @@ static int mmci_probe(struct amba_device *dev, > dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", > host->mclk); > } > + > host->phybase = dev->res.start; > - host->base = ioremap(dev->res.start, resource_size(&dev->res)); > + host->base = devm_ioremap(&dev->dev, host->phybase, > + resource_size(&dev->res)); Isn't it better to use devm_ioremap_resource directly? You will get correct error return values too. Thanks, Michal
On 4 April 2014 12:40, Michal Simek <monstr@monstr.eu> wrote: > On 03/31/2014 05:18 PM, Ulf Hansson wrote: >> Converting to devm functions to simplify error handling in ->probe() and >> to cleanup ->remove(). >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> --- >> drivers/mmc/host/mmci.c | 51 ++++++++++++++++++----------------------------- >> 1 file changed, 19 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c >> index b0b81ac..d6f20ba 100644 >> --- a/drivers/mmc/host/mmci.c >> +++ b/drivers/mmc/host/mmci.c >> @@ -13,6 +13,7 @@ >> #include <linux/init.h> >> #include <linux/ioport.h> >> #include <linux/device.h> >> +#include <linux/io.h> >> #include <linux/interrupt.h> >> #include <linux/kernel.h> >> #include <linux/slab.h> >> @@ -1456,15 +1457,13 @@ static int mmci_probe(struct amba_device *dev, >> if (np) >> mmci_dt_populate_generic_pdata(np, plat); >> >> - ret = amba_request_regions(dev, DRIVER_NAME); >> - if (ret) >> - goto out; >> + if (!devm_request_mem_region(&dev->dev, dev->res.start, >> + resource_size(&dev->res), DRIVER_NAME)) >> + return -ENOMEM; > > here. Look below. > >> >> mmc = mmc_alloc_host(sizeof(struct mmci_host), &dev->dev); >> - if (!mmc) { >> - ret = -ENOMEM; >> - goto rel_regions; >> - } >> + if (!mmc) >> + return -ENOMEM; >> >> host = mmc_priv(mmc); >> host->mmc = mmc; >> @@ -1500,8 +1499,10 @@ static int mmci_probe(struct amba_device *dev, >> dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", >> host->mclk); >> } >> + >> host->phybase = dev->res.start; >> - host->base = ioremap(dev->res.start, resource_size(&dev->res)); >> + host->base = devm_ioremap(&dev->dev, host->phybase, >> + resource_size(&dev->res)); > Hi Michal, Appreciate your review - you are right! I will convert to devm_ioremap_resource(). Kind regards Ulf Hansson > Isn't it better to use devm_ioremap_resource directly? > You will get correct error return values too. > > Thanks, > Michal > > > -- > Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 > w: www.monstr.eu p: +42-0-721842854 > Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ > Maintainer of Linux kernel - Xilinx Zynq ARM architecture > Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" 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/mmci.c b/drivers/mmc/host/mmci.c index b0b81ac..d6f20ba 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -13,6 +13,7 @@ #include <linux/init.h> #include <linux/ioport.h> #include <linux/device.h> +#include <linux/io.h> #include <linux/interrupt.h> #include <linux/kernel.h> #include <linux/slab.h> @@ -1456,15 +1457,13 @@ static int mmci_probe(struct amba_device *dev, if (np) mmci_dt_populate_generic_pdata(np, plat); - ret = amba_request_regions(dev, DRIVER_NAME); - if (ret) - goto out; + if (!devm_request_mem_region(&dev->dev, dev->res.start, + resource_size(&dev->res), DRIVER_NAME)) + return -ENOMEM; mmc = mmc_alloc_host(sizeof(struct mmci_host), &dev->dev); - if (!mmc) { - ret = -ENOMEM; - goto rel_regions; - } + if (!mmc) + return -ENOMEM; host = mmc_priv(mmc); host->mmc = mmc; @@ -1500,8 +1499,10 @@ static int mmci_probe(struct amba_device *dev, dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", host->mclk); } + host->phybase = dev->res.start; - host->base = ioremap(dev->res.start, resource_size(&dev->res)); + host->base = devm_ioremap(&dev->dev, host->phybase, + resource_size(&dev->res)); if (!host->base) { ret = -ENOMEM; goto clk_disable; @@ -1592,34 +1593,35 @@ static int mmci_probe(struct amba_device *dev, if (plat->gpio_cd == -EPROBE_DEFER) { ret = -EPROBE_DEFER; - goto err_gpio_cd; + goto clk_disable; } if (gpio_is_valid(plat->gpio_cd)) { ret = mmc_gpio_request_cd(mmc, plat->gpio_cd, 0); if (ret) - goto err_gpio_cd; + goto clk_disable; } if (plat->gpio_wp == -EPROBE_DEFER) { ret = -EPROBE_DEFER; - goto err_gpio_cd; + goto clk_disable; } if (gpio_is_valid(plat->gpio_wp)) { ret = mmc_gpio_request_ro(mmc, plat->gpio_wp); if (ret) - goto err_gpio_cd; + goto clk_disable; } - ret = request_irq(dev->irq[0], mmci_irq, IRQF_SHARED, DRIVER_NAME " (cmd)", host); + ret = devm_request_irq(&dev->dev, dev->irq[0], mmci_irq, IRQF_SHARED, + DRIVER_NAME " (cmd)", host); if (ret) - goto err_gpio_cd; + goto clk_disable; if (!dev->irq[1]) host->singleirq = true; else { - ret = request_irq(dev->irq[1], mmci_pio_irq, IRQF_SHARED, - DRIVER_NAME " (pio)", host); + ret = devm_request_irq(&dev->dev, dev->irq[1], mmci_pio_irq, + IRQF_SHARED, DRIVER_NAME " (pio)", host); if (ret) - goto irq0_free; + goto clk_disable; } writel(MCI_IRQENABLE, host->base + MMCIMASK0); @@ -1641,17 +1643,10 @@ static int mmci_probe(struct amba_device *dev, return 0; - irq0_free: - free_irq(dev->irq[0], host); - err_gpio_cd: - iounmap(host->base); clk_disable: clk_disable_unprepare(host->clk); host_free: mmc_free_host(mmc); - rel_regions: - amba_release_regions(dev); - out: return ret; } @@ -1677,16 +1672,8 @@ static int mmci_remove(struct amba_device *dev) writel(0, host->base + MMCIDATACTRL); mmci_dma_release(host); - free_irq(dev->irq[0], host); - if (!host->singleirq) - free_irq(dev->irq[1], host); - - iounmap(host->base); clk_disable_unprepare(host->clk); - mmc_free_host(mmc); - - amba_release_regions(dev); } return 0;
Converting to devm functions to simplify error handling in ->probe() and to cleanup ->remove(). Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/mmc/host/mmci.c | 51 ++++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 32 deletions(-)