mmc: wmt-sdmmc: fix unmatched release_mem_region

PATCH In review Alexey Charkov 4 participants 8 messages Nov 9, 2014 – Nov 25, 2014

Subsystems (2)

MMC, SD and SDIO Arm/VT8500 Arm architecture

lore

Share & tools

lore

Alexey Charkov

author maintainer

2014-11-09 21:12 UTC #0 lore raw

PATCH 1/1 mmc: wmt-sdmmc: fix unmatched release_mem_region

From: Helmut Stengele <[email protected]>

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 <[email protected]>

(edited commit message)

Signed-off-by Alexey Charkov <[email protected]>

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

drivers/mmc/host/wmt-sdmmc.c +8 ~~-10~~

index 54181b4..02b5e6b 100644
@@ -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");

Signature · 1 line

2.1.0

Fabio Estevam

2014-11-09 21:55 UTC #1 lore raw

drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 788

Quoted 4 lines · Alexey Charkov

drivers/mmc/host/wmt-sdmmc.c +4 -0

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

You could save this NULL check...

drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 816

Quoted 9 lines

drivers/mmc/host/wmt-sdmmc.c +1 -1

+       }
        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.

Quoted 4 lines

drivers/mmc/host/wmt-sdmmc.c +1 -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;

lautriv

2014-11-09 22:18 UTC #2 lore raw

Quoted 16 lines · Fabio Estevam

drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 788

Quoted 4 lines · Alexey Charkov

drivers/mmc/host/wmt-sdmmc.c +4 -0

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

You could save this NULL check...

Quoted 8 lines

drivers/mmc/host/wmt-sdmmc.c +1 -0

+       }
         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;

Quoted 8 lines

drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 816
Quoted 1 line
drivers/mmc/host/wmt-sdmmc.c +0 -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.
Quoted 4 lines
drivers/mmc/host/wmt-sdmmc.c +1 -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;

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.

Alexey Charkov

author maintainer

2014-11-10 05:15 UTC #3 lore raw

2014-11-10 1:18 GMT+03:00 lautriv <[email protected]>:

Quoted 25 lines · Fabio Estevam

Quoted 24 lines · Alexey Charkov
drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 788
Quoted 5 lines
drivers/mmc/host/wmt-sdmmc.c +4 -0
+       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (!res) {
+               ret = -ENODEV;
+               goto fail1;
You could save this NULL check...
drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device · line 817
Quoted 10 lines
drivers/mmc/host/wmt-sdmmc.c +1 -1
+       }
         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.

Quoted 8 lines

Quoted 4 lines
Quoted 4 lines
drivers/mmc/host/wmt-sdmmc.c +1 -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;
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

lautriv

2014-11-10 20:16 UTC #4 lore raw

Quoted 35 lines · Alexey Charkov

2014-11-10 1:18 GMT+03:00 lautriv <[email protected]>:
Quoted 22 lines · Fabio Estevam
Quoted 21 lines · Alexey Charkov
Review
Quoted 5 lines
(quoted diff) +4 -0
+       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (!res) {
+               ret = -ENODEV;
+               goto fail1;
You could save this NULL check...
Review static int wmt_mci_probe(struct platform_device · line 817
Quoted 10 lines
(quoted diff) +1 -1
+       }
          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.
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
(quoted diff) +1 -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;
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.

Quoted 8 lines

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.

Quoted 1 line

From 7c8ebb55fde8786cd16d95659ff73675bedc287c Mon Sep 17 00:00:00 2001

From: Helmut Stengele <[email protected]>
Date: Mon, 10 Nov 2014 11:26:51 +0100
Subject: [PATCH] mmc: vt8500: change resource-mapping to devm_*

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

Signed-off-by Helmut Stengele <[email protected]>

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

drivers/mmc/host/wmt-sdmmc.c +5 ~~-11~~

index 54181b4..f874d36 100644
@@ -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");

Signature · 1 line

1.7.10.4

lautriv

2014-11-20 14:45 UTC #5 lore raw

In reply to Alexey Charkov

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

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

Signed-off-by Helmut Stengele <[email protected]>

---

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

drivers/mmc/host/wmt-sdmmc.c +5 -9

index 54181b4..bcaa0cc 100644
@@ -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");

Signature · 1 line

2.1.3

Ulf Hansson

2014-11-24 08:34 UTC #6 lore raw

Quoted 36 lines · lautriv

From b0509e27e33326e6dccd67d8ebe67e2bdb0cfdde Mon Sep 17 00:00:00 2001
From: Helmut Stengele <[email protected]>
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 <[email protected]>
---
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(-)
drivers/mmc/host/wmt-sdmmc.c +1 -0
index 54181b4..bcaa0cc 100644
@@ -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");

drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 818

Quoted 6 lines

drivers/mmc/host/wmt-sdmmc.c +2 -1

@@ -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().

drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 820

Quoted 2 lines

drivers/mmc/host/wmt-sdmmc.c +0 -0

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

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

Quoted 42 lines

drivers/mmc/host/wmt-sdmmc.c +2 -9

-        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");
--

Quoted 3 lines

2.1.3

Kind regards
Uffe

lautriv

2014-11-25 22:34 UTC #7 lore raw

Quoted 89 lines · Ulf Hansson

Quoted 36 lines · lautriv
From b0509e27e33326e6dccd67d8ebe67e2bdb0cfdde Mon Sep 17 00:00:00 2001
From: Helmut Stengele <[email protected]>
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 <[email protected]>
---
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(-)
drivers/mmc/host/wmt-sdmmc.c +1 -0
index 54181b4..bcaa0cc 100644
@@ -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");
drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 818
Quoted 6 lines
drivers/mmc/host/wmt-sdmmc.c +2 -1
@@ -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().
drivers/mmc/host/wmt-sdmmc.c static int wmt_mci_probe(struct platform_device *pdev) · line 820
Quoted 2 lines
drivers/mmc/host/wmt-sdmmc.c +0 -0
     if (!priv->sdmmc_base) {
         dev_err(&pdev->dev, "Failed to map IO space\n");
This print can be removed, devm_ioremap_resource() handles that.
Quoted 40 lines
drivers/mmc/host/wmt-sdmmc.c +2 -8
-        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);
 

Quoted 8 lines

Quoted 5 lines
drivers/mmc/host/wmt-sdmmc.c +0 -1
     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 )