[PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[Alexey Charkov]

From: Helmut Stengele <lautriv@coldplug.net>

Current code calls release_mem_region upon module unload
without requesting the region first. This patch fixes it
by moving to a devres-based ioremap implementation, which
handles requesting and releasing memory region internally.

This also makes the error/unload paths a bit simpler.

Signed-off-by: Helmut Stengele <lautriv@coldplug.net>
(edited commit message)
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
Dear Chris, Ulf,

This is a pretty trivial fix to a bug in wmt-sdmmc.c that is only
visible upon module unload. Is there any chance to get it merged to
the current fixes tree?

I think I'd also like to convert other resource requests here to devres,
but that would probably be a separate patch to go into -next. That's why
we didn't rename the failX: labels, for example (as most of them could be
dropped altogether if the respective requesting functions are converted to
devres infrastructure).

Thanks a lot,
Alexey

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

diff --git a/drivers/mmc/host/wmt-sdmmc.c b/drivers/mmc/host/wmt-sdmmc.c
index 54181b4..02b5e6b 100644
--- a/drivers/mmc/host/wmt-sdmmc.c
+++ b/drivers/mmc/host/wmt-sdmmc.c
@@ -759,6 +759,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
 	const struct wmt_mci_caps *wmt_caps;
 	int ret;
 	int regular_irq, dma_irq;
+	struct resource *res;
 
 	if (!of_id || !of_id->data) {
 		dev_err(&pdev->dev, "Controller capabilities data missing\n");
@@ -781,6 +782,11 @@ static int wmt_mci_probe(struct platform_device *pdev)
 		goto fail1;
 	}
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -ENODEV;
+		goto fail1;
+	}
 	mmc = mmc_alloc_host(sizeof(struct wmt_mci_priv), &pdev->dev);
 	if (!mmc) {
 		dev_err(&pdev->dev, "Failed to allocate mmc_host\n");
@@ -813,7 +819,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
 	if (of_get_property(np, "cd-inverted", NULL))
 		priv->cd_inverted = 1;
 
-	priv->sdmmc_base = of_iomap(np, 0);
+	priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);
 	if (!priv->sdmmc_base) {
 		dev_err(&pdev->dev, "Failed to map IO space\n");
 		ret = -ENOMEM;
@@ -826,7 +832,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
 	ret = request_irq(regular_irq, wmt_mci_regular_isr, 0, "sdmmc", priv);
 	if (ret) {
 		dev_err(&pdev->dev, "Register regular IRQ fail\n");
-		goto fail3;
+		goto fail2;
 	}
 
 	ret = request_irq(dma_irq, wmt_mci_dma_isr, 0, "sdmmc", priv);
@@ -869,8 +875,6 @@ fail5:
 	free_irq(dma_irq, priv);
 fail4:
 	free_irq(regular_irq, priv);
-fail3:
-	iounmap(priv->sdmmc_base);
 fail2:
 	mmc_free_host(mmc);
 fail1:
@@ -881,7 +885,6 @@ static int wmt_mci_remove(struct platform_device *pdev)
 {
 	struct mmc_host *mmc;
 	struct wmt_mci_priv *priv;
-	struct resource *res;
 	u32 reg_tmp;
 
 	mmc = platform_get_drvdata(pdev);
@@ -904,14 +907,9 @@ static int wmt_mci_remove(struct platform_device *pdev)
 	free_irq(priv->irq_regular, priv);
 	free_irq(priv->irq_dma, priv);
 
-	iounmap(priv->sdmmc_base);
-
 	clk_disable_unprepare(priv->clk_sdmmc);
 	clk_put(priv->clk_sdmmc);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(res->start, resource_size(res));
-
 	mmc_free_host(mmc);
 
 	dev_info(&pdev->dev, "WMT MCI device removed\n");
-- 
2.1.0

Re: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[Fabio Estevam]

On Sun, Nov 9, 2014 at 7:12 PM, Alexey Charkov <alchark@gmail.com> wrote:

> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res) {
> +               ret = -ENODEV;
> +               goto fail1;

You could save this NULL check...

> +       }
>         mmc = mmc_alloc_host(sizeof(struct wmt_mci_priv), &pdev->dev);
>         if (!mmc) {
>                 dev_err(&pdev->dev, "Failed to allocate mmc_host\n");
> @@ -813,7 +819,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
>         if (of_get_property(np, "cd-inverted", NULL))
>                 priv->cd_inverted = 1;
>
> -       priv->sdmmc_base = of_iomap(np, 0);

If you move ' res = platform_get_resource' right here as
devm_ioremap_resource() already does the NULL check.

> +       priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);
>         if (!priv->sdmmc_base) {
>                 dev_err(&pdev->dev, "Failed to map IO space\n");
>                 ret = -ENOMEM;

Re: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[lautriv]

On 11/09/2014 10:55 PM, Fabio Estevam wrote:
> On Sun, Nov 9, 2014 at 7:12 PM, Alexey Charkov <alchark@gmail.com> wrote:
>
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       if (!res) {
>> +               ret = -ENODEV;
>> +               goto fail1;
> You could save this NULL check...
>
>> +       }
>>          mmc = mmc_alloc_host(sizeof(struct wmt_mci_priv), &pdev->dev);
>>          if (!mmc) {
>>                  dev_err(&pdev->dev, "Failed to allocate mmc_host\n");
>> @@ -813,7 +819,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
>>          if (of_get_property(np, "cd-inverted", NULL))
>>                  priv->cd_inverted = 1;
>>
>> -       priv->sdmmc_base = of_iomap(np, 0);
> If you move ' res = platform_get_resource' right here as
> devm_ioremap_resource() already does the NULL check.
>
>> +       priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);
>>          if (!priv->sdmmc_base) {
>>                  dev_err(&pdev->dev, "Failed to map IO space\n");
>>                  ret = -ENOMEM;
This was the original intention but it would fall between the failX 
jumps, i discussed that with Alexey and we decided to change it with the 
cleanup to hold this patch small.

Re: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[Alexey Charkov]

2014-11-10 1:18 GMT+03:00 lautriv <lautriv@coldplug.net>:
> On 11/09/2014 10:55 PM, Fabio Estevam wrote:
>>
>> On Sun, Nov 9, 2014 at 7:12 PM, Alexey Charkov <alchark@gmail.com> wrote:
>>
>>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       if (!res) {
>>> +               ret = -ENODEV;
>>> +               goto fail1;
>>
>> You could save this NULL check...
>>
>>> +       }
>>>          mmc = mmc_alloc_host(sizeof(struct wmt_mci_priv), &pdev->dev);
>>>          if (!mmc) {
>>>                  dev_err(&pdev->dev, "Failed to allocate mmc_host\n");
>>> @@ -813,7 +819,7 @@ static int wmt_mci_probe(struct platform_device
>>> *pdev)
>>>          if (of_get_property(np, "cd-inverted", NULL))
>>>                  priv->cd_inverted = 1;
>>>
>>> -       priv->sdmmc_base = of_iomap(np, 0);
>>
>> If you move ' res = platform_get_resource' right here as
>> devm_ioremap_resource() already does the NULL check.
>>
>>> +       priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);
>>>          if (!priv->sdmmc_base) {
>>>                  dev_err(&pdev->dev, "Failed to map IO space\n");
>>>                  ret = -ENOMEM;
>
> This was the original intention but it would fall between the failX jumps, i
> discussed that with Alexey and we decided to change it with the cleanup to
> hold this patch small.

I think Fabio means something slightly different than what we
discussed. We haven't realized that the first thing
devm_ioremap_resource does is exactly that (!res) check, so we can
skip ours altogether.

Looking at the function code a bit closer, we should also make the
check look like "if (IS_ERR(priv->sdmmc_base)", kill the dev_err call
(as it's also done internally), and then just do "ret =
PTR_ERR(priv->sdmmc_base)" and jump to the label.

Thanks a lot,
Alexey

Re: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[lautriv]

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

On 11/10/2014 06:15 AM, Alexey Charkov wrote:
> 2014-11-10 1:18 GMT+03:00 lautriv <lautriv@coldplug.net>:
>> On 11/09/2014 10:55 PM, Fabio Estevam wrote:
>>> On Sun, Nov 9, 2014 at 7:12 PM, Alexey Charkov <alchark@gmail.com> wrote:
>>>
>>>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +       if (!res) {
>>>> +               ret = -ENODEV;
>>>> +               goto fail1;
>>> You could save this NULL check...
>>>
>>>> +       }
>>>>           mmc = mmc_alloc_host(sizeof(struct wmt_mci_priv), &pdev->dev);
>>>>           if (!mmc) {
>>>>                   dev_err(&pdev->dev, "Failed to allocate mmc_host\n");
>>>> @@ -813,7 +819,7 @@ static int wmt_mci_probe(struct platform_device
>>>> *pdev)
>>>>           if (of_get_property(np, "cd-inverted", NULL))
>>>>                   priv->cd_inverted = 1;
>>>>
>>>> -       priv->sdmmc_base = of_iomap(np, 0);
>>> If you move ' res = platform_get_resource' right here as
>>> devm_ioremap_resource() already does the NULL check.
>>>
>>>> +       priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);
>>>>           if (!priv->sdmmc_base) {
>>>>                   dev_err(&pdev->dev, "Failed to map IO space\n");
>>>>                   ret = -ENOMEM;
>> This was the original intention but it would fall between the failX jumps, i
>> discussed that with Alexey and we decided to change it with the cleanup to
>> hold this patch small.
> I think Fabio means something slightly different than what we
> discussed. We haven't realized that the first thing
> devm_ioremap_resource does is exactly that (!res) check, so we can
> skip ours altogether.
>
> Looking at the function code a bit closer, we should also make the
> check look like "if (IS_ERR(priv->sdmmc_base)", kill the dev_err call
> (as it's also done internally), and then just do "ret =
> PTR_ERR(priv->sdmmc_base)" and jump to the label.
>
> Thanks a lot,
> Alexey
>
Sorry for the delay, 8880's are used to crash a lot since
we have not enought information ;)
Attached a changed patch which will hopefully meet all needs.


[-- Attachment #2: 0001-mmc-vt8500-change-resource-mapping-to-devm_.patch --]
[-- Type: text/x-patch, Size: 2730 bytes --]

>From 7c8ebb55fde8786cd16d95659ff73675bedc287c Mon Sep 17 00:00:00 2001
From: Helmut Stengele <lautriv@coldplug.net>
Date: Mon, 10 Nov 2014 11:26:51 +0100
Subject: [PATCH] mmc: vt8500: change resource-mapping to devm_*

Current code calls release_mem_region upon module unload
without requesting the region first. This patch fixes it
by moving to a devres-based ioremap implementation, which
handles requesting and releasing memory region internally.

This also makes the error/unload paths a bit simpler.

Signed-off-by: Helmut Stengele <lautriv@coldplug.net>
---
 drivers/mmc/host/wmt-sdmmc.c |   16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/wmt-sdmmc.c b/drivers/mmc/host/wmt-sdmmc.c
index 54181b4..f874d36 100644
--- a/drivers/mmc/host/wmt-sdmmc.c
+++ b/drivers/mmc/host/wmt-sdmmc.c
@@ -759,6 +759,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
 	const struct wmt_mci_caps *wmt_caps;
 	int ret;
 	int regular_irq, dma_irq;
+	struct resource *res;
 
 	if (!of_id || !of_id->data) {
 		dev_err(&pdev->dev, "Controller capabilities data missing\n");
@@ -813,10 +814,11 @@ static int wmt_mci_probe(struct platform_device *pdev)
 	if (of_get_property(np, "cd-inverted", NULL))
 		priv->cd_inverted = 1;
 
-	priv->sdmmc_base = of_iomap(np, 0);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);
 	if (!priv->sdmmc_base) {
 		dev_err(&pdev->dev, "Failed to map IO space\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(priv->sdmmc_base);
 		goto fail2;
 	}
 
@@ -826,7 +828,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
 	ret = request_irq(regular_irq, wmt_mci_regular_isr, 0, "sdmmc", priv);
 	if (ret) {
 		dev_err(&pdev->dev, "Register regular IRQ fail\n");
-		goto fail3;
+		goto fail2;
 	}
 
 	ret = request_irq(dma_irq, wmt_mci_dma_isr, 0, "sdmmc", priv);
@@ -869,8 +871,6 @@ fail5:
 	free_irq(dma_irq, priv);
 fail4:
 	free_irq(regular_irq, priv);
-fail3:
-	iounmap(priv->sdmmc_base);
 fail2:
 	mmc_free_host(mmc);
 fail1:
@@ -881,7 +881,6 @@ static int wmt_mci_remove(struct platform_device *pdev)
 {
 	struct mmc_host *mmc;
 	struct wmt_mci_priv *priv;
-	struct resource *res;
 	u32 reg_tmp;
 
 	mmc = platform_get_drvdata(pdev);
@@ -904,14 +903,9 @@ static int wmt_mci_remove(struct platform_device *pdev)
 	free_irq(priv->irq_regular, priv);
 	free_irq(priv->irq_dma, priv);
 
-	iounmap(priv->sdmmc_base);
-
 	clk_disable_unprepare(priv->clk_sdmmc);
 	clk_put(priv->clk_sdmmc);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(res->start, resource_size(res));
-
 	mmc_free_host(mmc);
 
 	dev_info(&pdev->dev, "WMT MCI device removed\n");
-- 
1.7.10.4

Re: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[lautriv]

 From b0509e27e33326e6dccd67d8ebe67e2bdb0cfdde Mon Sep 17 00:00:00 2001
From: Helmut Stengele <lautriv@coldplug.net>
Date: Thu, 20 Nov 2014 15:27:40 +0100
Subject: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

Current code calls release_mem_region upon module unload
without requesting the region first. This patch fixes it
by moving to a devres-based ioremap implementation, which
handles requesting and releasing memory region internally.

This also makes the error/unload paths a bit simpler.

Signed-off-by: Helmut Stengele <lautriv@coldplug.net>
---

This is a corrected patch based on the one Alexey sent for me.
I sent it already but since i'm not used to send something upstream
it was an attached file and obviously rejected.
Patch is tested on bare-metal ( wm8850 ) and works like suspected.

  drivers/mmc/host/wmt-sdmmc.c | 14 +++++---------
  1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/wmt-sdmmc.c b/drivers/mmc/host/wmt-sdmmc.c
index 54181b4..bcaa0cc 100644
--- a/drivers/mmc/host/wmt-sdmmc.c
+++ b/drivers/mmc/host/wmt-sdmmc.c
@@ -759,6 +759,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
      const struct wmt_mci_caps *wmt_caps;
      int ret;
      int regular_irq, dma_irq;
+    struct resource *res;

      if (!of_id || !of_id->data) {
          dev_err(&pdev->dev, "Controller capabilities data missing\n");
@@ -813,10 +814,11 @@ static int wmt_mci_probe(struct platform_device *pdev)
      if (of_get_property(np, "cd-inverted", NULL))
          priv->cd_inverted = 1;

-    priv->sdmmc_base = of_iomap(np, 0);
+    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+    priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);
      if (!priv->sdmmc_base) {
          dev_err(&pdev->dev, "Failed to map IO space\n");
-        ret = -ENOMEM;
+        ret = PTR_ERR(priv->sdmmc_base);
          goto fail2;
      }

@@ -826,7 +828,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
      ret = request_irq(regular_irq, wmt_mci_regular_isr, 0, "sdmmc", priv);
      if (ret) {
          dev_err(&pdev->dev, "Register regular IRQ fail\n");
-        goto fail3;
+        goto fail2;
      }

      ret = request_irq(dma_irq, wmt_mci_dma_isr, 0, "sdmmc", priv);
@@ -869,8 +871,6 @@ fail5:
      free_irq(dma_irq, priv);
  fail4:
      free_irq(regular_irq, priv);
-fail3:
-    iounmap(priv->sdmmc_base);
  fail2:
      mmc_free_host(mmc);
  fail1:
@@ -881,7 +881,6 @@ static int wmt_mci_remove(struct platform_device *pdev)
  {
      struct mmc_host *mmc;
      struct wmt_mci_priv *priv;
-    struct resource *res;
      u32 reg_tmp;

      mmc = platform_get_drvdata(pdev);
@@ -909,9 +908,6 @@ static int wmt_mci_remove(struct platform_device *pdev)
      clk_disable_unprepare(priv->clk_sdmmc);
      clk_put(priv->clk_sdmmc);

-    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-    release_mem_region(res->start, resource_size(res));
-
      mmc_free_host(mmc);

      dev_info(&pdev->dev, "WMT MCI device removed\n");
-- 
2.1.3

Re: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[Ulf Hansson]

On 20 November 2014 at 15:45, lautriv <lautriv@coldplug.net> wrote:
> From b0509e27e33326e6dccd67d8ebe67e2bdb0cfdde Mon Sep 17 00:00:00 2001
> From: Helmut Stengele <lautriv@coldplug.net>
> Date: Thu, 20 Nov 2014 15:27:40 +0100
> Subject: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region
>
> Current code calls release_mem_region upon module unload
> without requesting the region first. This patch fixes it
> by moving to a devres-based ioremap implementation, which
> handles requesting and releasing memory region internally.
>
> This also makes the error/unload paths a bit simpler.
>
> Signed-off-by: Helmut Stengele <lautriv@coldplug.net>
> ---
>
> This is a corrected patch based on the one Alexey sent for me.
> I sent it already but since i'm not used to send something upstream
> it was an attached file and obviously rejected.
> Patch is tested on bare-metal ( wm8850 ) and works like suspected.
>
>  drivers/mmc/host/wmt-sdmmc.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/wmt-sdmmc.c b/drivers/mmc/host/wmt-sdmmc.c
> index 54181b4..bcaa0cc 100644
> --- a/drivers/mmc/host/wmt-sdmmc.c
> +++ b/drivers/mmc/host/wmt-sdmmc.c
> @@ -759,6 +759,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
>      const struct wmt_mci_caps *wmt_caps;
>      int ret;
>      int regular_irq, dma_irq;
> +    struct resource *res;
>
>      if (!of_id || !of_id->data) {
>          dev_err(&pdev->dev, "Controller capabilities data missing\n");
> @@ -813,10 +814,11 @@ static int wmt_mci_probe(struct platform_device *pdev)
>      if (of_get_property(np, "cd-inverted", NULL))
>          priv->cd_inverted = 1;
>
> -    priv->sdmmc_base = of_iomap(np, 0);
> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +    priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);

You should check the return value from devm_ioremap_resource() with IS_ERR().

>      if (!priv->sdmmc_base) {
>          dev_err(&pdev->dev, "Failed to map IO space\n");

This print can be removed, devm_ioremap_resource() handles that.

> -        ret = -ENOMEM;
> +        ret = PTR_ERR(priv->sdmmc_base);
>          goto fail2;
>      }
>
> @@ -826,7 +828,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
>      ret = request_irq(regular_irq, wmt_mci_regular_isr, 0, "sdmmc", priv);
>      if (ret) {
>          dev_err(&pdev->dev, "Register regular IRQ fail\n");
> -        goto fail3;
> +        goto fail2;
>      }
>
>      ret = request_irq(dma_irq, wmt_mci_dma_isr, 0, "sdmmc", priv);
> @@ -869,8 +871,6 @@ fail5:
>      free_irq(dma_irq, priv);
>  fail4:
>      free_irq(regular_irq, priv);
> -fail3:
> -    iounmap(priv->sdmmc_base);
>  fail2:
>      mmc_free_host(mmc);
>  fail1:
> @@ -881,7 +881,6 @@ static int wmt_mci_remove(struct platform_device *pdev)
>  {
>      struct mmc_host *mmc;
>      struct wmt_mci_priv *priv;
> -    struct resource *res;
>      u32 reg_tmp;
>
>      mmc = platform_get_drvdata(pdev);
> @@ -909,9 +908,6 @@ static int wmt_mci_remove(struct platform_device *pdev)
>      clk_disable_unprepare(priv->clk_sdmmc);
>      clk_put(priv->clk_sdmmc);
>
> -    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -    release_mem_region(res->start, resource_size(res));
> -
>      mmc_free_host(mmc);
>
>      dev_info(&pdev->dev, "WMT MCI device removed\n");
> --
> 2.1.3
>
>

Kind regards
Uffe

Re: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region

[lautriv]

On 11/24/2014 09:34 AM, Ulf Hansson wrote:
> On 20 November 2014 at 15:45, lautriv <lautriv@coldplug.net> wrote:
>> From b0509e27e33326e6dccd67d8ebe67e2bdb0cfdde Mon Sep 17 00:00:00 2001
>> From: Helmut Stengele <lautriv@coldplug.net>
>> Date: Thu, 20 Nov 2014 15:27:40 +0100
>> Subject: [PATCH] mmc: wmt-sdmmc: fix unmatched release_mem_region
>>
>> Current code calls release_mem_region upon module unload
>> without requesting the region first. This patch fixes it
>> by moving to a devres-based ioremap implementation, which
>> handles requesting and releasing memory region internally.
>>
>> This also makes the error/unload paths a bit simpler.
>>
>> Signed-off-by: Helmut Stengele <lautriv@coldplug.net>
>> ---
>>
>> This is a corrected patch based on the one Alexey sent for me.
>> I sent it already but since i'm not used to send something upstream
>> it was an attached file and obviously rejected.
>> Patch is tested on bare-metal ( wm8850 ) and works like suspected.
>>
>>  drivers/mmc/host/wmt-sdmmc.c | 14 +++++---------
>>  1 file changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mmc/host/wmt-sdmmc.c b/drivers/mmc/host/wmt-sdmmc.c
>> index 54181b4..bcaa0cc 100644
>> --- a/drivers/mmc/host/wmt-sdmmc.c
>> +++ b/drivers/mmc/host/wmt-sdmmc.c
>> @@ -759,6 +759,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
>>      const struct wmt_mci_caps *wmt_caps;
>>      int ret;
>>      int regular_irq, dma_irq;
>> +    struct resource *res;
>>
>>      if (!of_id || !of_id->data) {
>>          dev_err(&pdev->dev, "Controller capabilities data missing\n");
>> @@ -813,10 +814,11 @@ static int wmt_mci_probe(struct platform_device *pdev)
>>      if (of_get_property(np, "cd-inverted", NULL))
>>          priv->cd_inverted = 1;
>>
>> -    priv->sdmmc_base = of_iomap(np, 0);
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    priv->sdmmc_base = devm_ioremap_resource(&pdev->dev, res);
> You should check the return value from devm_ioremap_resource() with IS_ERR().
>
>>      if (!priv->sdmmc_base) {
>>          dev_err(&pdev->dev, "Failed to map IO space\n");
> This print can be removed, devm_ioremap_resource() handles that.
>
>> -        ret = -ENOMEM;
>> +        ret = PTR_ERR(priv->sdmmc_base);
>>          goto fail2;
>>      }
>>
>> @@ -826,7 +828,7 @@ static int wmt_mci_probe(struct platform_device *pdev)
>>      ret = request_irq(regular_irq, wmt_mci_regular_isr, 0, "sdmmc", priv);
>>      if (ret) {
>>          dev_err(&pdev->dev, "Register regular IRQ fail\n");
>> -        goto fail3;
>> +        goto fail2;
>>      }
>>
>>      ret = request_irq(dma_irq, wmt_mci_dma_isr, 0, "sdmmc", priv);
>> @@ -869,8 +871,6 @@ fail5:
>>      free_irq(dma_irq, priv);
>>  fail4:
>>      free_irq(regular_irq, priv);
>> -fail3:
>> -    iounmap(priv->sdmmc_base);
>>  fail2:
>>      mmc_free_host(mmc);
>>  fail1:
>> @@ -881,7 +881,6 @@ static int wmt_mci_remove(struct platform_device *pdev)
>>  {
>>      struct mmc_host *mmc;
>>      struct wmt_mci_priv *priv;
>> -    struct resource *res;
>>      u32 reg_tmp;
>>
>>      mmc = platform_get_drvdata(pdev);
>> @@ -909,9 +908,6 @@ static int wmt_mci_remove(struct platform_device *pdev)
>>      clk_disable_unprepare(priv->clk_sdmmc);
>>      clk_put(priv->clk_sdmmc);
>>
>> -    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -    release_mem_region(res->start, resource_size(res));
>> -
>>      mmc_free_host(mmc);
>>
>>      dev_info(&pdev->dev, "WMT MCI device removed\n");
>> --
>> 2.1.3
>>
>>
> Kind regards
> Uffe
>
Thanks for the hint, will do so. ( Sent the wrong patch by accident anyway )