resend - sh mmcif driver cleanup patch series

resend - sh mmcif driver cleanup patch series

[Ben Dooks]

This is a re-send of the mmcif driver cleanups that seem to have
been missed. If possible, could this be merged for the current merge
window?

These should be available at:

	http://git.codethink.co.uk/linux.git bjdooks/315-rc8/mmcif

[PATCH 1/6] mmc: sh-mmcif: update to print version and bus clock rate on probe

[Ben Dooks]

Change the initial print to show chip version and the bus rate it is
working at instead of the driver version. This is more useful information
as we already know which driver version from the kernel it is in.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/mmc/host/sh_mmcif.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 54730f4..7f819f1 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1468,16 +1468,17 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 	mutex_init(&host->thread_lock);
 
-	clk_disable_unprepare(host->hclk);
 	ret = mmc_add_host(mmc);
 	if (ret < 0)
 		goto emmcaddh;
 
 	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
 
-	dev_info(&pdev->dev, "driver version %s\n", DRIVER_VERSION);
-	dev_dbg(&pdev->dev, "chip ver H'%04x\n",
-		sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0x0000ffff);
+	dev_info(&pdev->dev, "Chip version 0x%04x, clock rate %luMHz\n",
+		 sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0xffff,
+		 clk_get_rate(host->hclk) / 1000000UL);
+
+	clk_disable_unprepare(host->hclk);
 	return ret;
 
 emmcaddh:
-- 
2.0.0.rc2

Re: [PATCH 1/6] mmc: sh-mmcif: update to print version and bus clock rate on probe

[Ulf Hansson]

On 4 June 2014 13:42, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> Change the initial print to show chip version and the bus rate it is
> working at instead of the driver version. This is more useful information
> as we already know which driver version from the kernel it is in.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/mmc/host/sh_mmcif.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 54730f4..7f819f1 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1468,16 +1468,17 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>
>         mutex_init(&host->thread_lock);
>
> -       clk_disable_unprepare(host->hclk);
>         ret = mmc_add_host(mmc);
>         if (ret < 0)
>                 goto emmcaddh;
>
>         dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
>
> -       dev_info(&pdev->dev, "driver version %s\n", DRIVER_VERSION);
> -       dev_dbg(&pdev->dev, "chip ver H'%04x\n",
> -               sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0x0000ffff);
> +       dev_info(&pdev->dev, "Chip version 0x%04x, clock rate %luMHz\n",
> +                sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0xffff,
> +                clk_get_rate(host->hclk) / 1000000UL);
> +
> +       clk_disable_unprepare(host->hclk);
>         return ret;
>
>  emmcaddh:
> --
> 2.0.0.rc2
>

[PATCH 2/6] mmc: sh-mmcif: use devm_ for ioremap

[Ben Dooks]

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>
---
Fixes from v1:
	- return the PTR_ERR() from devm_ioremap
	- remove error print as it already shows an error
---
 drivers/mmc/host/sh_mmcif.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 7f819f1..3ed54c8 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1377,22 +1377,15 @@ 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) {
-		dev_err(&pdev->dev, "ioremap error.\n");
-		return -ENOMEM;
-	}
+	reg = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
 
 	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)
@@ -1497,8 +1490,6 @@ eclkget:
 	pm_runtime_disable(&pdev->dev);
 eofparse:
 	mmc_free_host(mmc);
-ealloch:
-	iounmap(reg);
 	return ret;
 }
 
@@ -1523,9 +1514,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);
 
-- 
2.0.0.rc2

Re: [PATCH 2/6] mmc: sh-mmcif: use devm_ for ioremap

[Ulf Hansson]

On 4 June 2014 13:42, Ben Dooks <ben.dooks@codethink.co.uk> 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>

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
> Fixes from v1:
>         - return the PTR_ERR() from devm_ioremap
>         - remove error print as it already shows an error
> ---
>  drivers/mmc/host/sh_mmcif.c | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 7f819f1..3ed54c8 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1377,22 +1377,15 @@ 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) {
> -               dev_err(&pdev->dev, "ioremap error.\n");
> -               return -ENOMEM;
> -       }
> +       reg = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(reg))
> +               return PTR_ERR(reg);
>
>         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)
> @@ -1497,8 +1490,6 @@ eclkget:
>         pm_runtime_disable(&pdev->dev);
>  eofparse:
>         mmc_free_host(mmc);
> -ealloch:
> -       iounmap(reg);
>         return ret;
>  }
>
> @@ -1523,9 +1514,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);
>
> --
> 2.0.0.rc2
>

[PATCH 3/6] mmc: sh-mmcif: use devm_ for clock management

[Ben Dooks]

Use the devm_clk_get() code to get the clock and allow it to be freed
automatically on release.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/mmc/host/sh_mmcif.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 3ed54c8..ae278a5 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1389,7 +1389,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 	ret = mmc_of_parse(mmc);
 	if (ret < 0)
-		goto eofparse;
+		goto err_host;
 
 	host		= mmc_priv(mmc);
 	host->mmc	= mmc;
@@ -1419,19 +1419,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	host->power = false;
 
-	host->hclk = clk_get(&pdev->dev, NULL);
+	host->hclk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->hclk)) {
 		ret = PTR_ERR(host->hclk);
 		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
-		goto eclkget;
+		goto err_pm;
 	}
 	ret = sh_mmcif_clk_update(host);
 	if (ret < 0)
-		goto eclkupdate;
+		goto err_pm;
 
 	ret = pm_runtime_resume(&pdev->dev);
 	if (ret < 0)
-		goto eresume;
+		goto err_clk;
 
 	INIT_DELAYED_WORK(&host->timeout_work, mmcif_timeout_work);
 
@@ -1482,13 +1482,11 @@ ereqirq1:
 	free_irq(irq[0], host);
 ereqirq0:
 	pm_runtime_suspend(&pdev->dev);
-eresume:
+err_clk:
 	clk_disable_unprepare(host->hclk);
-eclkupdate:
-	clk_put(host->hclk);
-eclkget:
+err_pm:
 	pm_runtime_disable(&pdev->dev);
-eofparse:
+err_host:
 	mmc_free_host(mmc);
 	return ret;
 }
-- 
2.0.0.rc2

Re: [PATCH 3/6] mmc: sh-mmcif: use devm_ for clock management

[Ulf Hansson]

On 4 June 2014 13:42, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> Use the devm_clk_get() code to get the clock and allow it to be freed
> automatically on release.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/mmc/host/sh_mmcif.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 3ed54c8..ae278a5 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1389,7 +1389,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>
>         ret = mmc_of_parse(mmc);
>         if (ret < 0)
> -               goto eofparse;
> +               goto err_host;
>
>         host            = mmc_priv(mmc);
>         host->mmc       = mmc;
> @@ -1419,19 +1419,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>         pm_runtime_enable(&pdev->dev);
>         host->power = false;
>
> -       host->hclk = clk_get(&pdev->dev, NULL);
> +       host->hclk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(host->hclk)) {
>                 ret = PTR_ERR(host->hclk);
>                 dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
> -               goto eclkget;
> +               goto err_pm;
>         }
>         ret = sh_mmcif_clk_update(host);
>         if (ret < 0)
> -               goto eclkupdate;
> +               goto err_pm;
>
>         ret = pm_runtime_resume(&pdev->dev);
>         if (ret < 0)
> -               goto eresume;
> +               goto err_clk;
>
>         INIT_DELAYED_WORK(&host->timeout_work, mmcif_timeout_work);
>
> @@ -1482,13 +1482,11 @@ ereqirq1:
>         free_irq(irq[0], host);
>  ereqirq0:
>         pm_runtime_suspend(&pdev->dev);
> -eresume:
> +err_clk:
>         clk_disable_unprepare(host->hclk);
> -eclkupdate:
> -       clk_put(host->hclk);
> -eclkget:
> +err_pm:
>         pm_runtime_disable(&pdev->dev);
> -eofparse:
> +err_host:
>         mmc_free_host(mmc);
>         return ret;
>  }
> --
> 2.0.0.rc2
>

[PATCH 4/6] mmc: sh-mmcif: use devm_ for irq management

[Ben Dooks]

Use devm_request_threaded_irq() for the host interrupt handlers so we
do not have to worry about freeing them on exit or error. Tidies up the
exit path code for the driver.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/mmc/host/sh_mmcif.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index ae278a5..cc6858d 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1439,17 +1439,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
 
 	name = irq[1] < 0 ? dev_name(&pdev->dev) : "sh_mmc:error";
-	ret = request_threaded_irq(irq[0], sh_mmcif_intr, sh_mmcif_irqt, 0, name, host);
+	ret = devm_request_threaded_irq(&pdev->dev, irq[0], sh_mmcif_intr,
+					sh_mmcif_irqt, 0, name, host);
 	if (ret) {
 		dev_err(&pdev->dev, "request_irq error (%s)\n", name);
-		goto ereqirq0;
+		goto err_irq;
 	}
 	if (irq[1] >= 0) {
-		ret = request_threaded_irq(irq[1], sh_mmcif_intr, sh_mmcif_irqt,
-					   0, "sh_mmc:int", host);
+		ret = devm_request_threaded_irq(&pdev->dev, irq[1],
+						sh_mmcif_intr, sh_mmcif_irqt,
+						0, "sh_mmc:int", host);
 		if (ret) {
 			dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
-			goto ereqirq1;
+			goto err_irq;
 		}
 	}
 
@@ -1476,11 +1478,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 emmcaddh:
 erqcd:
-	if (irq[1] >= 0)
-		free_irq(irq[1], host);
-ereqirq1:
-	free_irq(irq[0], host);
-ereqirq0:
+err_irq:
 	pm_runtime_suspend(&pdev->dev);
 err_clk:
 	clk_disable_unprepare(host->hclk);
@@ -1494,7 +1492,6 @@ err_host:
 static int sh_mmcif_remove(struct platform_device *pdev)
 {
 	struct sh_mmcif_host *host = platform_get_drvdata(pdev);
-	int irq[2];
 
 	host->dying = true;
 	clk_prepare_enable(host->hclk);
@@ -1512,13 +1509,6 @@ static int sh_mmcif_remove(struct platform_device *pdev)
 	 */
 	cancel_delayed_work_sync(&host->timeout_work);
 
-	irq[0] = platform_get_irq(pdev, 0);
-	irq[1] = platform_get_irq(pdev, 1);
-
-	free_irq(irq[0], host);
-	if (irq[1] >= 0)
-		free_irq(irq[1], host);
-
 	clk_disable_unprepare(host->hclk);
 	mmc_free_host(host->mmc);
 	pm_runtime_put_sync(&pdev->dev);
-- 
2.0.0.rc2

Re: [PATCH 4/6] mmc: sh-mmcif: use devm_ for irq management

[Ulf Hansson]

On 4 June 2014 13:42, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> Use devm_request_threaded_irq() for the host interrupt handlers so we
> do not have to worry about freeing them on exit or error. Tidies up the
> exit path code for the driver.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/mmc/host/sh_mmcif.c | 26 ++++++++------------------
>  1 file changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index ae278a5..cc6858d 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1439,17 +1439,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>         sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
>
>         name = irq[1] < 0 ? dev_name(&pdev->dev) : "sh_mmc:error";
> -       ret = request_threaded_irq(irq[0], sh_mmcif_intr, sh_mmcif_irqt, 0, name, host);
> +       ret = devm_request_threaded_irq(&pdev->dev, irq[0], sh_mmcif_intr,
> +                                       sh_mmcif_irqt, 0, name, host);
>         if (ret) {
>                 dev_err(&pdev->dev, "request_irq error (%s)\n", name);
> -               goto ereqirq0;
> +               goto err_irq;
>         }
>         if (irq[1] >= 0) {
> -               ret = request_threaded_irq(irq[1], sh_mmcif_intr, sh_mmcif_irqt,
> -                                          0, "sh_mmc:int", host);
> +               ret = devm_request_threaded_irq(&pdev->dev, irq[1],
> +                                               sh_mmcif_intr, sh_mmcif_irqt,
> +                                               0, "sh_mmc:int", host);
>                 if (ret) {
>                         dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
> -                       goto ereqirq1;
> +                       goto err_irq;
>                 }
>         }
>
> @@ -1476,11 +1478,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>
>  emmcaddh:
>  erqcd:
> -       if (irq[1] >= 0)
> -               free_irq(irq[1], host);
> -ereqirq1:
> -       free_irq(irq[0], host);
> -ereqirq0:
> +err_irq:
>         pm_runtime_suspend(&pdev->dev);
>  err_clk:
>         clk_disable_unprepare(host->hclk);
> @@ -1494,7 +1492,6 @@ err_host:
>  static int sh_mmcif_remove(struct platform_device *pdev)
>  {
>         struct sh_mmcif_host *host = platform_get_drvdata(pdev);
> -       int irq[2];
>
>         host->dying = true;
>         clk_prepare_enable(host->hclk);
> @@ -1512,13 +1509,6 @@ static int sh_mmcif_remove(struct platform_device *pdev)
>          */
>         cancel_delayed_work_sync(&host->timeout_work);
>
> -       irq[0] = platform_get_irq(pdev, 0);
> -       irq[1] = platform_get_irq(pdev, 1);
> -
> -       free_irq(irq[0], host);
> -       if (irq[1] >= 0)
> -               free_irq(irq[1], host);
> -
>         clk_disable_unprepare(host->hclk);
>         mmc_free_host(host->mmc);
>         pm_runtime_put_sync(&pdev->dev);
> --
> 2.0.0.rc2
>

[PATCH 5/6] mmc: sh-mmcif: no need to call pm_runtime_suspend on error

[Ben Dooks]

The pm_runtime call should implicitly disable the device once the
probe is over if there is no explicit reference gained. There is no
need to call pm_runtime_suspend() before the pm_runtime_disable()
call.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/mmc/host/sh_mmcif.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index cc6858d..b378aa0 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1443,7 +1443,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 					sh_mmcif_irqt, 0, name, host);
 	if (ret) {
 		dev_err(&pdev->dev, "request_irq error (%s)\n", name);
-		goto err_irq;
+		goto err_clk;
 	}
 	if (irq[1] >= 0) {
 		ret = devm_request_threaded_irq(&pdev->dev, irq[1],
@@ -1451,7 +1451,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 						0, "sh_mmc:int", host);
 		if (ret) {
 			dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
-			goto err_irq;
+			goto err_clk;
 		}
 	}
 
@@ -1478,8 +1478,6 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 emmcaddh:
 erqcd:
-err_irq:
-	pm_runtime_suspend(&pdev->dev);
 err_clk:
 	clk_disable_unprepare(host->hclk);
 err_pm:
-- 
2.0.0.rc2

Re: [PATCH 5/6] mmc: sh-mmcif: no need to call pm_runtime_suspend on error

[Ulf Hansson]

On 4 June 2014 13:42, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> The pm_runtime call should implicitly disable the device once the
> probe is over if there is no explicit reference gained. There is no
> need to call pm_runtime_suspend() before the pm_runtime_disable()
> call.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/mmc/host/sh_mmcif.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index cc6858d..b378aa0 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1443,7 +1443,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>                                         sh_mmcif_irqt, 0, name, host);
>         if (ret) {
>                 dev_err(&pdev->dev, "request_irq error (%s)\n", name);
> -               goto err_irq;
> +               goto err_clk;
>         }
>         if (irq[1] >= 0) {
>                 ret = devm_request_threaded_irq(&pdev->dev, irq[1],
> @@ -1451,7 +1451,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>                                                 0, "sh_mmc:int", host);
>                 if (ret) {
>                         dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
> -                       goto err_irq;
> +                       goto err_clk;
>                 }
>         }
>
> @@ -1478,8 +1478,6 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>
>  emmcaddh:
>  erqcd:
> -err_irq:
> -       pm_runtime_suspend(&pdev->dev);
>  err_clk:
>         clk_disable_unprepare(host->hclk);
>  err_pm:
> --
> 2.0.0.rc2
>
> --
> 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

[PATCH 6/6] mmc: sh-mmcif: final error path cleanup

[Ben Dooks]

Remove the error path items that are no longer needed. The mmc card-detect
code cleans up after itself (and registers with devm) and the host error
is the same as the clock disable.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/mmc/host/sh_mmcif.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index b378aa0..35c77ad 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1458,14 +1458,14 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	if (pd && pd->use_cd_gpio) {
 		ret = mmc_gpio_request_cd(mmc, pd->cd_gpio, 0);
 		if (ret < 0)
-			goto erqcd;
+			goto err_clk;
 	}
 
 	mutex_init(&host->thread_lock);
 
 	ret = mmc_add_host(mmc);
 	if (ret < 0)
-		goto emmcaddh;
+		goto err_clk;
 
 	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
 
@@ -1476,8 +1476,6 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	clk_disable_unprepare(host->hclk);
 	return ret;
 
-emmcaddh:
-erqcd:
 err_clk:
 	clk_disable_unprepare(host->hclk);
 err_pm:
-- 
2.0.0.rc2

Re: [PATCH 6/6] mmc: sh-mmcif: final error path cleanup

[Ulf Hansson]

On 4 June 2014 13:42, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> Remove the error path items that are no longer needed. The mmc card-detect
> code cleans up after itself (and registers with devm) and the host error
> is the same as the clock disable.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/mmc/host/sh_mmcif.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index b378aa0..35c77ad 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1458,14 +1458,14 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>         if (pd && pd->use_cd_gpio) {
>                 ret = mmc_gpio_request_cd(mmc, pd->cd_gpio, 0);
>                 if (ret < 0)
> -                       goto erqcd;
> +                       goto err_clk;
>         }
>
>         mutex_init(&host->thread_lock);
>
>         ret = mmc_add_host(mmc);
>         if (ret < 0)
> -               goto emmcaddh;
> +               goto err_clk;
>
>         dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
>
> @@ -1476,8 +1476,6 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>         clk_disable_unprepare(host->hclk);
>         return ret;
>
> -emmcaddh:
> -erqcd:
>  err_clk:
>         clk_disable_unprepare(host->hclk);
>  err_pm:
> --
> 2.0.0.rc2
>

Re: resend - sh mmcif driver cleanup patch series

[Ulf Hansson]

On 4 June 2014 13:42, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> This is a re-send of the mmcif driver cleanups that seem to have
> been missed. If possible, could this be merged for the current merge
> window?
>
> These should be available at:
>
>         http://git.codethink.co.uk/linux.git bjdooks/315-rc8/mmcif
>

Thanks! Applied for next.

Kind regards
Ulf Hansson

> --
> 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

cleanups for sh-mmcif driver

[Ben Dooks]

Some cleanups and updates for the sh mmcif driver.

[PATCH 3/6] mmc: sh-mmcif: use devm_ for clock management

[Ben Dooks]

---
 drivers/mmc/host/sh_mmcif.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index be6be2b..c006310 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1393,7 +1393,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 	ret = mmc_of_parse(mmc);
 	if (ret < 0)
-		goto eofparse;
+		goto err_host;
 
 	host		= mmc_priv(mmc);
 	host->mmc	= mmc;
@@ -1423,19 +1423,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	host->power = false;
 
-	host->hclk = clk_get(&pdev->dev, NULL);
+	host->hclk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->hclk)) {
 		ret = PTR_ERR(host->hclk);
 		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
-		goto eclkget;
+		goto err_pm;
 	}
 	ret = sh_mmcif_clk_update(host);
 	if (ret < 0)
-		goto eclkupdate;
+		goto err_clk;
 
 	ret = pm_runtime_resume(&pdev->dev);
 	if (ret < 0)
-		goto eresume;
+		goto err_clk;
 
 	INIT_DELAYED_WORK(&host->timeout_work, mmcif_timeout_work);
 
@@ -1486,13 +1486,11 @@ ereqirq1:
 	free_irq(irq[0], host);
 ereqirq0:
 	pm_runtime_suspend(&pdev->dev);
-eresume:
+err_clk:
 	clk_disable_unprepare(host->hclk);
-eclkupdate:
-	clk_put(host->hclk);
-eclkget:
+err_pm:
 	pm_runtime_disable(&pdev->dev);
-eofparse:
+err_host:
 	mmc_free_host(mmc);
 	return ret;
 }
-- 
1.9.0

Re: [PATCH 3/6] mmc: sh-mmcif: use devm_ for clock management

[Geert Uytterhoeven]

On Tue, Apr 1, 2014 at 12:25 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> -       host->hclk = clk_get(&pdev->dev, NULL);
> +       host->hclk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(host->hclk)) {
>                 ret = PTR_ERR(host->hclk);
>                 dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
> -               goto eclkget;
> +               goto err_pm;
>         }
>         ret = sh_mmcif_clk_update(host);
>         if (ret < 0)
> -               goto eclkupdate;
> +               goto err_clk;

This goto doesn't look correct to me. If sh_mmcif_clk_update() failed,
the clock was not enabled nor prepared. "goto err_pm"?

>         ret = pm_runtime_resume(&pdev->dev);
>         if (ret < 0)
> -               goto eresume;
> +               goto err_clk;
>
>         INIT_DELAYED_WORK(&host->timeout_work, mmcif_timeout_work);
>
> @@ -1486,13 +1486,11 @@ ereqirq1:
>         free_irq(irq[0], host);
>  ereqirq0:
>         pm_runtime_suspend(&pdev->dev);
> -eresume:
> +err_clk:
>         clk_disable_unprepare(host->hclk);
> -eclkupdate:
> -       clk_put(host->hclk);
> -eclkget:
> +err_pm:
>         pm_runtime_disable(&pdev->dev);
> -eofparse:
> +err_host:
>         mmc_free_host(mmc);
>         return ret;

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

Re: [PATCH 3/6] mmc: sh-mmcif: use devm_ for clock management

[Ben Dooks]

On 01/04/14 11:59, Geert Uytterhoeven wrote:
> On Tue, Apr 1, 2014 at 12:25 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> -       host->hclk = clk_get(&pdev->dev, NULL);
>> +       host->hclk = devm_clk_get(&pdev->dev, NULL);
>>          if (IS_ERR(host->hclk)) {
>>                  ret = PTR_ERR(host->hclk);
>>                  dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
>> -               goto eclkget;
>> +               goto err_pm;
>>          }
>>          ret = sh_mmcif_clk_update(host);
>>          if (ret < 0)
>> -               goto eclkupdate;
>> +               goto err_clk;
>
> This goto doesn't look correct to me. If sh_mmcif_clk_update() failed,
> the clock was not enabled nor prepared. "goto err_pm"?

Yes, I think the clk_prepare_enable() call there is the only failure
mode.

As a note, we should probably check all systems that use it and see if
we can remove the clock management code in here as the pm_runtime will
also be doing this.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius