Re: [PATCH] mmc: davinci: Fix and simplify probe failure path

From: Ulf Hansson
Date: Mon Oct 27 2014 - 07:18:43 EST


On 26 September 2014 12:11, Pramod Gurav <pramod.gurav@xxxxxxxxxxxxxxx> wrote:
> The sequence of resource release in probe failure path in this
> driver was wrong and needed fixes to cleanly unload the driver.
> This changes does the same by switching to managed resources and
> fixes return path to release resource in proper sequence.
>
> Cc: Chris Ball <chris@xxxxxxxxxx>
> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Cc: linux-mmc@xxxxxxxxxxxxxxx
> Signed-off-by: Pramod Gurav <pramod.gurav@xxxxxxxxxxxxxxx>
>
> ---
> drivers/mmc/host/davinci_mmc.c | 91 +++++++++++++++-------------------------
> 1 file changed, 33 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> index 5d4c5e0..160b7e8 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -1242,22 +1242,20 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
> return -ENOENT;
> }
>
> - ret = -ENODEV;
> r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> irq = platform_get_irq(pdev, 0);
> if (!r || irq == NO_IRQ)
> - goto out;
> + return -ENODEV;
>
> - ret = -EBUSY;
> mem_size = resource_size(r);
> - mem = request_mem_region(r->start, mem_size, pdev->name);
> + mem = devm_request_mem_region(&pdev->dev, r->start, mem_size,
> + pdev->name);
> if (!mem)
> - goto out;
> + return -EBUSY;
>
> - ret = -ENOMEM;
> mmc = mmc_alloc_host(sizeof(struct mmc_davinci_host), &pdev->dev);
> if (!mmc)
> - goto out;
> + return -ENOMEM;
>
> host = mmc_priv(mmc);
> host->mmc = mmc; /* Important */
> @@ -1275,15 +1273,16 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
> host->txdma = r->start;
>
> host->mem_res = mem;
> - host->base = ioremap(mem->start, mem_size);
> - if (!host->base)
> - goto out;
> + host->base = devm_ioremap(&pdev->dev, mem->start, mem_size);
> + if (IS_ERR(host->base)) {

devm_ioremap() doesn't return an error which you could check with IS_ERR().

> + ret = PTR_ERR(host->base);
> + goto err_ioremap;
> + }
>
> - ret = -ENXIO;
> - host->clk = clk_get(&pdev->dev, "MMCSDCLK");
> + host->clk = devm_clk_get(&pdev->dev, "MMCSDCLK");
> if (IS_ERR(host->clk)) {
> ret = PTR_ERR(host->clk);
> - goto out;
> + goto err_ioremap;
> }
> clk_enable(host->clk);
> host->mmc_input_clk = clk_get_rate(host->clk);
> @@ -1350,20 +1349,22 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
> ret = mmc_davinci_cpufreq_register(host);
> if (ret) {
> dev_err(&pdev->dev, "failed to register cpufreq\n");
> - goto cpu_freq_fail;
> + goto err_cpu_freq;
> }
>
> ret = mmc_add_host(mmc);
> if (ret < 0)
> - goto out;
> + goto err_mmc_add;
>
> - ret = request_irq(irq, mmc_davinci_irq, 0, mmc_hostname(mmc), host);
> + ret = devm_request_irq(&pdev->dev, irq, mmc_davinci_irq, 0,
> + mmc_hostname(mmc), host);
> if (ret)
> - goto out;
> + goto err_request_irq;
>
> if (host->sdio_irq >= 0) {
> - ret = request_irq(host->sdio_irq, mmc_davinci_sdio_irq, 0,
> - mmc_hostname(mmc), host);
> + ret = devm_request_irq(&pdev->dev, host->sdio_irq,
> + mmc_davinci_sdio_irq, 0,
> + mmc_hostname(mmc), host);
> if (!ret)
> mmc->caps |= MMC_CAP_SDIO_IRQ;
> }
> @@ -1376,27 +1377,15 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
>
> return 0;
>
> -out:
> +err_request_irq:
> + mmc_free_host(mmc);
> +err_mmc_add:
> mmc_davinci_cpufreq_deregister(host);
> -cpu_freq_fail:
> - if (host) {
> - davinci_release_dma_channels(host);
> -
> - if (host->clk) {
> - clk_disable(host->clk);
> - clk_put(host->clk);
> - }
> -
> - if (host->base)
> - iounmap(host->base);
> - }
> -
> - if (mmc)
> - mmc_free_host(mmc);
> -
> - if (mem)
> - release_resource(mem);
> -
> +err_cpu_freq:
> + davinci_release_dma_channels(host);
> + clk_disable(host->clk);
> +err_ioremap:
> + mmc_remove_host(mmc);
> dev_dbg(&pdev->dev, "probe err %d\n", ret);
>
> return ret;
> @@ -1406,25 +1395,11 @@ static int __exit davinci_mmcsd_remove(struct platform_device *pdev)
> {
> struct mmc_davinci_host *host = platform_get_drvdata(pdev);
>
> - if (host) {
> - mmc_davinci_cpufreq_deregister(host);
> -
> - mmc_remove_host(host->mmc);
> - free_irq(host->mmc_irq, host);
> - if (host->mmc->caps & MMC_CAP_SDIO_IRQ)
> - free_irq(host->sdio_irq, host);
> -
> - davinci_release_dma_channels(host);
> -
> - clk_disable(host->clk);
> - clk_put(host->clk);
> -
> - iounmap(host->base);
> -
> - release_resource(host->mem_res);
> -
> - mmc_free_host(host->mmc);
> - }
> + mmc_free_host(host->mmc);
> + mmc_davinci_cpufreq_deregister(host);
> + davinci_release_dma_channels(host);
> + clk_disable(host->clk);
> + mmc_remove_host(host->mmc);

mmc_remove_host() needs to be invoked earlier.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/