Skip to contentSkip to conversation

davinci_mmc device tree fixes

PATCH Superseded David Lechner 3 participants 28 messages Mar 9, 2016 – Mar 16, 2016

Subsystems (4)
Arm (32-bit) Arm sub-architectures MMC, SD and SDIO TI DaVinci machine support

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

David Lechner

author
#0 lore raw

cover davinci_mmc device tree fixes

I'm working on getting a davinci device working with device tree. I came across some issues in the davinci_mmc driver.

David Lechner (4):
mmc: davinci: remove matching string
mmc: davinci: fix unwinding in probe
mmc: davinci: prepare clock
mmc: davinci: suppress waring when using DT

 drivers/mmc/host/davinci_mmc.c | 86 ++++++++++++++++++++----------------------
 1 file changed, 41 insertions(+), 45 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/4 mmc: davinci: remove matching string

The string "MMCSDCLK" is not actually used for clock lookup, so can be removed.

Signed-off-by David Lechner <[email protected]>
---
 drivers/mmc/host/davinci_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/host/davinci_mmc.c +1 -1
index ea2a2eb..469aa8c 100644
@@ -1280,7 +1280,7 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 		goto out;
 
 	ret = -ENXIO;
-	host->clk = clk_get(&pdev->dev, "MMCSDCLK");
+	host->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->clk)) {
 		ret = PTR_ERR(host->clk);
 		goto out;
Signature · 1 line
1.9.1
author
#3 lore raw

PATCH 2/4 mmc: davinci: fix unwinding in probe

In reply to David Lechner

Unwiding from an error in davinci_mmcsd_probe was a mess. Some errors were not handled and not all paths unwound correctly.

Signed-off-by David Lechner <[email protected]>
---
 drivers/mmc/host/davinci_mmc.c | 62 +++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 34 deletions(-)
drivers/mmc/host/davinci_mmc.c +28 -34
index 469aa8c..f571549 100644
@@ -1232,7 +1232,7 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	struct mmc_davinci_host *host = NULL;
 	struct mmc_host *mmc = NULL;
 	struct resource *r, *mem = NULL;
-	int ret = 0, irq = 0;
+	int ret, irq;
 	size_t mem_size;
 	const struct platform_device_id *id_entry;
 

@@ -1242,22 +1242,21 @@ 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);
 	if (!mem)
-		goto out;
+		return -EBUSY;
 
-	ret = -ENOMEM;
 	mmc = mmc_alloc_host(sizeof(struct mmc_davinci_host), &pdev->dev);
-	if (!mmc)
-		goto out;
+	if (!mmc) {
+		ret = -ENOMEM;
+		goto mmc_alloc_host_fail;
+	}
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */

@@ -1277,15 +1276,17 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	host->mem_res = mem;
 	host->base = ioremap(mem->start, mem_size);
 	if (!host->base)
-		goto out;
+		goto ioremap_fail;
 
-	ret = -ENXIO;
 	host->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->clk)) {
 		ret = PTR_ERR(host->clk);
-		goto out;
+		goto clk_get_fail;
 	}
-	clk_enable(host->clk);
+	ret = clk_enable(host->clk);
+	if (ret)
+		goto clk_enable_fail;
+
 	host->mmc_input_clk = clk_get_rate(host->clk);
 
 	init_mmcsd_host(host);

@@ -1355,11 +1356,11 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 
 	ret = mmc_add_host(mmc);
 	if (ret < 0)
-		goto out;
+		goto mmc_add_host_fail;
 
 	ret = request_irq(irq, mmc_davinci_irq, 0, mmc_hostname(mmc), host);
 	if (ret)
-		goto out;
+		goto request_irq_fail;
 
 	if (host->sdio_irq >= 0) {
 		ret = request_irq(host->sdio_irq, mmc_davinci_sdio_irq, 0,

@@ -1376,28 +1377,21 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 
 	return 0;
 
-out:
+request_irq_fail:
+	mmc_remove_host(mmc);
+mmc_add_host_fail:
 	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);
-
-	dev_dbg(&pdev->dev, "probe err %d\n", ret);
+	davinci_release_dma_channels(host);
+	clk_disable(host->clk);
+clk_enable_fail:
+	clk_put(host->clk);
+clk_get_fail:
+	iounmap(host->base);
+ioremap_fail:
+	mmc_free_host(mmc);
+mmc_alloc_host_fail:
+	release_resource(mem);
 
 	return ret;
 }
Signature · 1 line
1.9.1
#4 lore raw

PATCH 2/4 mmc: davinci: fix unwinding in probe

Hi David,

Quoted 4 lines · David Lechner

Unwiding from an error in davinci_mmcsd_probe was a mess. Some errors were not handled and not all paths unwound correctly.

Signed-off-by David Lechner <[email protected]>

Since you are touching this, can you also shift to using devm_*() variants where possible (like devm_ioremap()). That will simplify the error handling quite a bit.

Regards,
Sekhar

author
#5 lore raw

PATCH 3/4 mmc: davinci: prepare clock

In reply to David Lechner

When trying to use this driver with the common clock framework, enabling the clock fails because it was not prepared. This fixes the problem by calling clk_prepare and clk_enable in a single function.

Signed-off-by David Lechner <[email protected]>
---
 drivers/mmc/host/davinci_mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/mmc/host/davinci_mmc.c +3 -3
index f571549..b160feb 100644
@@ -1283,9 +1283,9 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 		ret = PTR_ERR(host->clk);
 		goto clk_get_fail;
 	}
-	ret = clk_enable(host->clk);
+	ret = clk_prepare_enable(host->clk);
 	if (ret)
-		goto clk_enable_fail;
+		goto clk_prepare_enable_fail;
 
 	host->mmc_input_clk = clk_get_rate(host->clk);
 

@@ -1384,7 +1384,7 @@ mmc_add_host_fail:
 cpu_freq_fail:
 	davinci_release_dma_channels(host);
 	clk_disable(host->clk);
-clk_enable_fail:
+clk_prepare_enable_fail:
 	clk_put(host->clk);
 clk_get_fail:
 	iounmap(host->base);
Signature · 1 line
1.9.1
#6 lore raw

PATCH 3/4 mmc: davinci: prepare clock

Quoted 5 lines · David Lechner

When trying to use this driver with the common clock framework, enabling the clock fails because it was not prepared. This fixes the problem by calling clk_prepare and clk_enable in a single function.

Signed-off-by David Lechner <[email protected]>

Patch looks good. But I wonder how you hit this issue since we dont use common clock framework on mach-davinci and this driver is not used anywhere else AFAIK.

Regards,
Sekhar

author
#7 lore raw

PATCH 3/4 mmc: davinci: prepare clock

Quoted 5 lines · Sekhar Nori
Quoted 4 lines · David Lechner

When trying to use this driver with the common clock framework, enabling the clock fails because it was not prepared. This fixes the problem by calling clk_prepare and clk_enable in a single function.

Quoted 8 lines
Quoted 1 line
Signed-off-by David Lechner <[email protected]>

Patch looks good. But I wonder how you hit this issue since we dont use common clock framework on mach-davinci and this driver is not used anywhere else AFAIK.

Regards,
Sekhar

Because I am working towards getting the common clock framework working on mach-davinci - at least with da8xx/device tree. :-)

author
#8 lore raw

PATCH 3/4 mmc: davinci: prepare clock

Quoted 11 lines · David Lechner
Quoted 10 lines · Sekhar Nori
Quoted 5 lines · David Lechner

When trying to use this driver with the common clock framework, enabling the clock fails because it was not prepared. This fixes the problem by calling clk_prepare and clk_enable in a single function.

Signed-off-by David Lechner <[email protected]>

Patch looks good. But I wonder how you hit this issue since we dont use common clock framework on mach-davinci and this driver is not used anywhere else AFAIK.

Quoted 5 lines
Quoted 3 lines

Regards,
Sekhar

Because I am working towards getting the common clock framework working on mach-davinci - at least with da8xx/device tree. :-)

I should also mention that I did test using the existing mach-davinci clocks as well.

#9 lore raw

PATCH 3/4 mmc: davinci: prepare clock

In reply to David Lechner
Quoted 11 lines · David Lechner
Quoted 10 lines · Sekhar Nori
Quoted 5 lines · David Lechner

When trying to use this driver with the common clock framework, enabling the clock fails because it was not prepared. This fixes the problem by calling clk_prepare and clk_enable in a single function.

Signed-off-by David Lechner <[email protected]>

Patch looks good. But I wonder how you hit this issue since we dont use common clock framework on mach-davinci and this driver is not used anywhere else AFAIK.

Quoted 5 lines
Quoted 3 lines

Regards,
Sekhar

Because I am working towards getting the common clock framework working on mach-davinci - at least with da8xx/device tree. :-)

Alright, glad to know someone is pursuing this.

Thanks,
Sekhar

author
#10 lore raw

PATCH 4/4 mmc: davinci: suppress waring when using DT

In reply to David Lechner

When using device tree dms resources are not obtained via IORESOURCE_DMA. As a result, we would get warning even though DMA was working.

This fixes the problem by checking for dev.of_node and skipping trying to get the dma platform resource if we are using device tree.

Signed-off-by David Lechner <[email protected]>
---
 drivers/mmc/host/davinci_mmc.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
drivers/mmc/host/davinci_mmc.c +12 -10
index b160feb..475704d 100644
@@ -1261,17 +1261,19 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (!r)
-		dev_warn(&pdev->dev, "RX DMA resource not specified\n");
-	else
-		host->rxdma = r->start;
+	if (!pdev->dev.of_node) {
+		r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+		if (!r)
+			dev_warn(&pdev->dev, "RX DMA resource not specified\n");
+		else
+			host->rxdma = r->start;
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-	if (!r)
-		dev_warn(&pdev->dev, "TX DMA resource not specified\n");
-	else
-		host->txdma = r->start;
+		r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
+		if (!r)
+			dev_warn(&pdev->dev, "TX DMA resource not specified\n");
+		else
+			host->txdma = r->start;
+	}
 
 	host->mem_res = mem;
 	host->base = ioremap(mem->start, mem_size);
Signature · 1 line
1.9.1
#11 lore raw

PATCH 4/4 mmc: davinci: suppress waring when using DT

Quoted 7 lines · David Lechner

When using device tree dms resources are not obtained via IORESOURCE_DMA. As a result, we would get warning even though DMA was working.

This fixes the problem by checking for dev.of_node and skipping trying to get the dma platform resource if we are using device tree.

Signed-off-by David Lechner <[email protected]>

Actually, with Peter's series adding dma_slave_map[] for all mach-davinci platforms, this code should not be needed at all. Can you try that dropping the platform_get_resource() calls for DMA works with linux-next?

Thanks,
Sekhar

author
#12 lore raw

v2 cover davinci_mmc fixes

I've updated the patche set based on feedback. This is tested on linux-next with LEGO MINDSTORMS EV3 (da850), both with and without device tree enabled.

David Lechner (5):
mmc: davinci: remove matching string
mmc: davinci: fix unwinding in probe
mmc: davinci: prepare clock
mmc: davinci: don't use dma platform resources
arm: davinci: remove mmc dma resources

 arch/arm/mach-davinci/devices-da8xx.c |  20 ------
 arch/arm/mach-davinci/devices.c       |  16 -----
 drivers/mmc/host/davinci_mmc.c        | 112 +++++++++++-----------------------
 3 files changed, 37 insertions(+), 111 deletions(-)
Signature · 1 line
1.9.1
author
#13 lore raw

PATCH v2 1/5 mmc: davinci: remove matching string

The string "MMCSDCLK" is not actually used for clock lookup, so can be removed.

Signed-off-by David Lechner <[email protected]>

---

v2 changes: none

 drivers/mmc/host/davinci_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/host/davinci_mmc.c +1 -1
index 693144e..a294d261 100644
@@ -1271,7 +1271,7 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 		goto out;
 
 	ret = -ENXIO;
-	host->clk = clk_get(&pdev->dev, "MMCSDCLK");
+	host->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->clk)) {
 		ret = PTR_ERR(host->clk);
 		goto out;
Signature · 1 line
1.9.1
Acked-by on v2 1/5
#14 lore raw

Re: [PATCH v2 1/5] mmc: davinci: remove matching string

Quoted 3 lines · David Lechner

The string "MMCSDCLK" is not actually used for clock lookup, so can be removed.

Signed-off-by David Lechner <[email protected]>

In general, you can carry forward acks received in the last version of the patch if the patch is unchanged.

Acked-by Sekhar Nori <[email protected]>

Thanks,
Sekhar

author
#15 lore raw

PATCH v2 2/5 mmc: davinci: fix unwinding in probe

In reply to David Lechner

Unwiding from an error in davinci_mmcsd_probe was a mess. Some errors were not handled and not all paths unwound correctly. Also using devm_ where possible to simplify things.

Signed-off-by David Lechner <[email protected]>

---

v2 changes: use devm_ where possible

 drivers/mmc/host/davinci_mmc.c | 93 ++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 58 deletions(-)
drivers/mmc/host/davinci_mmc.c +35 -58
index a294d261..6dd9562 100644
@@ -1223,7 +1223,7 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	struct mmc_davinci_host *host = NULL;
 	struct mmc_host *mmc = NULL;
 	struct resource *r, *mem = NULL;
-	int ret = 0, irq = 0;
+	int ret, irq;
 	size_t mem_size;
 	const struct platform_device_id *id_entry;
 

@@ -1233,22 +1233,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;
+		ret = -ENOMEM;
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */

@@ -1266,17 +1264,19 @@ 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);
+	host->base = devm_ioremap(&pdev->dev, mem->start, mem_size);
 	if (!host->base)
-		goto out;
+		goto ioremap_fail;
 
-	ret = -ENXIO;
-	host->clk = clk_get(&pdev->dev, NULL);
+	host->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->clk)) {
 		ret = PTR_ERR(host->clk);
-		goto out;
+		goto clk_get_fail;
 	}
-	clk_enable(host->clk);
+	ret = clk_enable(host->clk);
+	if (ret)
+		goto clk_enable_fail;
+
 	host->mmc_input_clk = clk_get_rate(host->clk);
 
 	init_mmcsd_host(host);

@@ -1346,15 +1346,17 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 
 	ret = mmc_add_host(mmc);
 	if (ret < 0)
-		goto out;
+		goto mmc_add_host_fail;
 
-	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 request_irq_fail;
 
 	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;
 	}

@@ -1367,28 +1369,17 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 
 	return 0;
 
-out:
+request_irq_fail:
+	mmc_remove_host(mmc);
+mmc_add_host_fail:
 	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);
-
-	dev_dbg(&pdev->dev, "probe err %d\n", ret);
+	davinci_release_dma_channels(host);
+	clk_disable(host->clk);
+clk_enable_fail:
+clk_get_fail:
+ioremap_fail:
+	mmc_free_host(mmc);
 
 	return ret;
 }

@@ -1397,25 +1388,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_remove_host(host->mmc);
+	mmc_davinci_cpufreq_deregister(host);
+	davinci_release_dma_channels(host);
+	clk_disable(host->clk);
+	mmc_free_host(host->mmc);
 
 	return 0;
 }
Signature · 1 line
1.9.1
#16 lore raw

Re: [PATCH v2 2/5] mmc: davinci: fix unwinding in probe

Quoted 40 lines · David Lechner

Unwiding from an error in davinci_mmcsd_probe was a mess. Some errors were not handled and not all paths unwound correctly. Also using devm_ where possible to simplify things.

Signed-off-by David Lechner <[email protected]>

---

v2 changes: use devm_ where possible

 drivers/mmc/host/davinci_mmc.c | 93 ++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 58 deletions(-)
drivers/mmc/host/davinci_mmc.c +3 -5
index a294d261..6dd9562 100644
@@ -1223,7 +1223,7 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	struct mmc_davinci_host *host = NULL;
 	struct mmc_host *mmc = NULL;
 	struct resource *r, *mem = NULL;
-	int ret = 0, irq = 0;
+	int ret, irq;
 	size_t mem_size;
 	const struct platform_device_id *id_entry;
 

@@ -1233,22 +1233,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,

drivers/mmc/host/davinci_mmc.c static int __init davinci_mmcsd_probe(struct platform_device *pdev) · line 1249

Quoted 10 lines
drivers/mmc/host/davinci_mmc.c +3 -3
+				      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;
+		ret = -ENOMEM;

This should be 'return -ENOMEM' you dont want to proceed further if this fails.

Other than that, looks fine to me.

Regards,
Sekhar

author
#17 lore raw

PATCH v2 3/5 mmc: davinci: prepare clock

In reply to David Lechner

When trying to use this driver with the common clock framework, enabling the clock fails because it was not prepared. This fixes the problem by calling clk_prepare and clk_enable in a single function. Ditto for clk_disable_unprepare.

Signed-off-by David Lechner <[email protected]>

---

v2 changes: use clk_disable_unprepare where applicable.

 drivers/mmc/host/davinci_mmc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/mmc/host/davinci_mmc.c +5 -5
index 6dd9562..8d10a92 100644
@@ -1273,9 +1273,9 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 		ret = PTR_ERR(host->clk);
 		goto clk_get_fail;
 	}
-	ret = clk_enable(host->clk);
+	ret = clk_prepare_enable(host->clk);
 	if (ret)
-		goto clk_enable_fail;
+		goto clk_prepare_enable_fail;
 
 	host->mmc_input_clk = clk_get_rate(host->clk);
 

@@ -1375,8 +1375,8 @@ mmc_add_host_fail:
 	mmc_davinci_cpufreq_deregister(host);
 cpu_freq_fail:
 	davinci_release_dma_channels(host);
-	clk_disable(host->clk);
-clk_enable_fail:
+	clk_disable_unprepare(host->clk);
+clk_prepare_enable_fail:
 clk_get_fail:
 ioremap_fail:
 	mmc_free_host(mmc);

@@ -1391,7 +1391,7 @@ static int __exit davinci_mmcsd_remove(struct platform_device *pdev)
 	mmc_remove_host(host->mmc);
 	mmc_davinci_cpufreq_deregister(host);
 	davinci_release_dma_channels(host);
-	clk_disable(host->clk);
+	clk_disable_unprepare(host->clk);
 	mmc_free_host(host->mmc);
 
 	return 0;
Signature · 1 line
1.9.1
Acked-by on v2 3/5
#18 lore raw

Re: [PATCH v2 3/5] mmc: davinci: prepare clock

Quoted 6 lines · David Lechner

When trying to use this driver with the common clock framework, enabling the clock fails because it was not prepared. This fixes the problem by calling clk_prepare and clk_enable in a single function. Ditto for clk_disable_unprepare.

Signed-off-by David Lechner <[email protected]>
Acked-by Sekhar Nori <[email protected]>

Regards,
Sekhar

author
#19 lore raw

PATCH v2 4/5 mmc: davinci: don't use dma platform resources

In reply to David Lechner

The davinci arch now has dma_slave_map tables for dma resources, so it is no longer necessary to pass dma resources through the platform device.

Signed-off-by David Lechner <[email protected]>

---

v2 changes: Remove platform_get_resource completly instead of just ignoring it.

 drivers/mmc/host/davinci_mmc.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)
drivers/mmc/host/davinci_mmc.c +2 -17
index 8d10a92..dc96401 100644
@@ -202,7 +202,6 @@ struct mmc_davinci_host {
 	u32 buffer_bytes_left;
 	u32 bytes_left;
 
-	u32 rxdma, txdma;
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
 	bool use_dma;

@@ -520,16 +519,14 @@ static int __init davinci_acquire_dma_channels(struct mmc_davinci_host *host)
 	dma_cap_set(DMA_SLAVE, mask);
 
 	host->dma_tx =
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->txdma, mmc_dev(host->mmc), "tx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "tx");
 	if (!host->dma_tx) {
 		dev_err(mmc_dev(host->mmc), "Can't get dma_tx channel\n");
 		return -ENODEV;
 	}
 
 	host->dma_rx =
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->rxdma, mmc_dev(host->mmc), "rx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "rx");
 	if (!host->dma_rx) {
 		dev_err(mmc_dev(host->mmc), "Can't get dma_rx channel\n");
 		r = -ENODEV;

@@ -1251,18 +1248,6 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (!r)
-		dev_warn(&pdev->dev, "RX DMA resource not specified\n");
-	else
-		host->rxdma = r->start;
-
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-	if (!r)
-		dev_warn(&pdev->dev, "TX DMA resource not specified\n");
-	else
-		host->txdma = r->start;
-
 	host->mem_res = mem;
 	host->base = devm_ioremap(&pdev->dev, mem->start, mem_size);
 	if (!host->base)
Signature · 1 line
1.9.1
Acked-by on v2 4/5
#20 lore raw

Re: [PATCH v2 4/5] mmc: davinci: don't use dma platform resources

+ Peter

Quoted 4 lines · David Lechner

The davinci arch now has dma_slave_map tables for dma resources, so it is no longer necessary to pass dma resources through the platform device.

Signed-off-by David Lechner <[email protected]>

Looks good to me.

Acked-by Sekhar Nori <[email protected]>

Thanks,
Sekhar

Quoted 59 lines

---

v2 changes: Remove platform_get_resource completly instead of just ignoring it.

 drivers/mmc/host/davinci_mmc.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)
drivers/mmc/host/davinci_mmc.c +2 -17
index 8d10a92..dc96401 100644
@@ -202,7 +202,6 @@ struct mmc_davinci_host {
 	u32 buffer_bytes_left;
 	u32 bytes_left;
 
-	u32 rxdma, txdma;
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
 	bool use_dma;

@@ -520,16 +519,14 @@ static int __init davinci_acquire_dma_channels(struct mmc_davinci_host *host)
 	dma_cap_set(DMA_SLAVE, mask);
 
 	host->dma_tx =
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->txdma, mmc_dev(host->mmc), "tx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "tx");
 	if (!host->dma_tx) {
 		dev_err(mmc_dev(host->mmc), "Can't get dma_tx channel\n");
 		return -ENODEV;
 	}
 
 	host->dma_rx =
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->rxdma, mmc_dev(host->mmc), "rx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "rx");
 	if (!host->dma_rx) {
 		dev_err(mmc_dev(host->mmc), "Can't get dma_rx channel\n");
 		r = -ENODEV;

@@ -1251,18 +1248,6 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (!r)
-		dev_warn(&pdev->dev, "RX DMA resource not specified\n");
-	else
-		host->rxdma = r->start;
-
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-	if (!r)
-		dev_warn(&pdev->dev, "TX DMA resource not specified\n");
-	else
-		host->txdma = r->start;
-
 	host->mem_res = mem;
 	host->base = devm_ioremap(&pdev->dev, mem->start, mem_size);
 	if (!host->base)
#21 lore raw

Re: [PATCH v2 4/5] mmc: davinci: don't use dma platform resources

In reply to David Lechner
Quoted 25 lines · David Lechner

The davinci arch now has dma_slave_map tables for dma resources, so it is no longer necessary to pass dma resources through the platform device.

Signed-off-by David Lechner <[email protected]>

---

v2 changes: Remove platform_get_resource completly instead of just ignoring it.

 drivers/mmc/host/davinci_mmc.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)
drivers/mmc/host/davinci_mmc.c +0 -1
index 8d10a92..dc96401 100644
@@ -202,7 +202,6 @@ struct mmc_davinci_host {
 	u32 buffer_bytes_left;
 	u32 bytes_left;
 
-	u32 rxdma, txdma;
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
 	bool use_dma;

drivers/mmc/host/davinci_mmc.c static int __init davinci_acquire_dma_channels(struct mmc_davinci_host *host) · line 522

Quoted 6 lines
drivers/mmc/host/davinci_mmc.c +1 -2
@@ -520,16 +519,14 @@ static int __init davinci_acquire_dma_channels(struct mmc_davinci_host *host)
 	dma_cap_set(DMA_SLAVE, mask);
 
 	host->dma_tx =
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->txdma, mmc_dev(host->mmc), "tx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "tx");

you would need to use dma_request_chan() to be able to rely on the legacy channel mapping.
I have staged commits for converting all daVinci and OMAP drivers, I'll attach the patch I have for davinci-mmc for reference.
When we convert to use the dma_request_chan() we can handle deferred probing also...

Quoted 32 lines
drivers/mmc/host/davinci_mmc.c +1 -14
 	if (!host->dma_tx) {
 		dev_err(mmc_dev(host->mmc), "Can't get dma_tx channel\n");
 		return -ENODEV;
 	}
 
 	host->dma_rx =
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->rxdma, mmc_dev(host->mmc), "rx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "rx");
 	if (!host->dma_rx) {
 		dev_err(mmc_dev(host->mmc), "Can't get dma_rx channel\n");
 		r = -ENODEV;

@@ -1251,18 +1248,6 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (!r)
-		dev_warn(&pdev->dev, "RX DMA resource not specified\n");
-	else
-		host->rxdma = r->start;
-
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-	if (!r)
-		dev_warn(&pdev->dev, "TX DMA resource not specified\n");
-	else
-		host->txdma = r->start;
-
 	host->mem_res = mem;
 	host->base = devm_ioremap(&pdev->dev, mem->start, mem_size);
 	if (!host->base)
Signature · 8 lines
P?ter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mmc-davinci_mmc-Use-dma_request_chan-to-requesting-D.patch
Type: text/x-patch
Size: 3424 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160315/91afbacb/attachment-0001.bin>
#22 lore raw

Re: [PATCH v2 4/5] mmc: davinci: don't use dma platform resources

Quoted 28 lines · Peter Ujfalusi
Quoted 27 lines · David Lechner

The davinci arch now has dma_slave_map tables for dma resources, so it is no longer necessary to pass dma resources through the platform device.

Signed-off-by David Lechner <[email protected]>

---

v2 changes: Remove platform_get_resource completly instead of just ignoring it.

 drivers/mmc/host/davinci_mmc.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)
drivers/mmc/host/davinci_mmc.c +0 -1
index 8d10a92..dc96401 100644
@@ -202,7 +202,6 @@ struct mmc_davinci_host {
 	u32 buffer_bytes_left;
 	u32 bytes_left;
 
-	u32 rxdma, txdma;
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
 	bool use_dma;

@@ -520,16 +519,14 @@ static int __init davinci_acquire_dma_channels(struct mmc_davinci_host *host)
 	dma_cap_set(DMA_SLAVE, mask);
Quoted 7 lines

drivers/mmc/host/davinci_mmc.c static int __init davinci_acquire_dma_channels(struct mmc_davinci_host *host) · line 522

Quoted 4 lines
drivers/mmc/host/davinci_mmc.c +1 -2
 	host->dma_tx =
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->txdma, mmc_dev(host->mmc), "tx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "tx");

you would need to use dma_request_chan() to be able to rely on the legacy channel mapping.

In other words: w/o dma_request_chan() legacy boot will not work as you will not get the DMA channel.

Quoted 41 lines

I have staged commits for converting all daVinci and OMAP drivers, I'll attach the patch I have for davinci-mmc for reference.
When we convert to use the dma_request_chan() we can handle deferred probing also...

Quoted 32 lines
drivers/mmc/host/davinci_mmc.c +1 -14
 	if (!host->dma_tx) {
 		dev_err(mmc_dev(host->mmc), "Can't get dma_tx channel\n");
 		return -ENODEV;
 	}
 
 	host->dma_rx =
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->rxdma, mmc_dev(host->mmc), "rx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "rx");
 	if (!host->dma_rx) {
 		dev_err(mmc_dev(host->mmc), "Can't get dma_rx channel\n");
 		r = -ENODEV;

@@ -1251,18 +1248,6 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
 	host = mmc_priv(mmc);
 	host->mmc = mmc;	/* Important */
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (!r)
-		dev_warn(&pdev->dev, "RX DMA resource not specified\n");
-	else
-		host->rxdma = r->start;
-
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-	if (!r)
-		dev_warn(&pdev->dev, "TX DMA resource not specified\n");
-	else
-		host->txdma = r->start;
-
 	host->mem_res = mem;
 	host->base = devm_ioremap(&pdev->dev, mem->start, mem_size);
 	if (!host->base)
Signature · 1 line
P?ter
author
#23 lore raw

Re: [PATCH v2 4/5] mmc: davinci: don't use dma platform resources

In reply to Peter Ujfalusi
Quoted 14 lines · Peter Ujfalusi

Review

Quoted 5 lines · David Lechner

host->dma_tx =

(quoted diff) +1 -2
-		dma_request_slave_channel_compat(mask, edma_filter_fn,
-				&host->txdma, mmc_dev(host->mmc), "tx");
+		dma_request_slave_channel(mmc_dev(host->mmc), "tx");

you would need to use dma_request_chan() to be able to rely on the legacy channel mapping.
I have staged commits for converting all daVinci and OMAP drivers, I'll attach the patch I have for davinci-mmc for reference.
When we convert to use the dma_request_chan() we can handle deferred probing also...

OK. So I should redo this patch on top of the patch you attached?

#24 lore raw

Re: [PATCH v2 4/5] mmc: davinci: don't use dma platform resources

Quoted 16 lines · David Lechner
Quoted 7 lines · Peter Ujfalusi

Review

Quoted 5 lines · David Lechner
      host->dma_tx =
(quoted diff) +1 -2
-        dma_request_slave_channel_compat(mask, edma_filter_fn,
-                &host->txdma, mmc_dev(host->mmc), "tx");
+        dma_request_slave_channel(mmc_dev(host->mmc), "tx");
Quoted 7 lines

you would need to use dma_request_chan() to be able to rely on the legacy channel mapping.
I have staged commits for converting all daVinci and OMAP drivers, I'll attach the patch I have for davinci-mmc for reference.
When we convert to use the dma_request_chan() we can handle deferred probing also...

Quoted 1 line

OK. So I should redo this patch on top of the patch you attached?

Or replace your patch with mine in your series. In my patch I forgot to remove the 'u32 rxdma, txdma;' from struct mmc_davinci_host...

Signature · 1 line
P?ter
author
#25 lore raw

PATCH v2 5/5 arm: davinci: remove mmc dma resources

In reply to David Lechner

The davinci_mmc driver no longer uses platform resources for getting dma channels. Instead lookup is now done using dma_slave_map.

Signed-off-by David Lechner <[email protected]>

---

v2 changes: This is a new patch in v2.

 arch/arm/mach-davinci/devices-da8xx.c | 20 --------------------
 arch/arm/mach-davinci/devices.c       | 16 ----------------
 2 files changed, 36 deletions(-)
arch/arm/mach-davinci/devices-da8xx.c +0 -20
index 725e693..add3771 100644
@@ -751,16 +751,6 @@ static struct resource da8xx_mmcsd0_resources[] = {
 		.end	= IRQ_DA8XX_MMCSDINT0,
 		.flags	= IORESOURCE_IRQ,
 	},
-	{		/* DMA RX */
-		.start	= DA8XX_DMA_MMCSD0_RX,
-		.end	= DA8XX_DMA_MMCSD0_RX,
-		.flags	= IORESOURCE_DMA,
-	},
-	{		/* DMA TX */
-		.start	= DA8XX_DMA_MMCSD0_TX,
-		.end	= DA8XX_DMA_MMCSD0_TX,
-		.flags	= IORESOURCE_DMA,
-	},
 };
 
 static struct platform_device da8xx_mmcsd0_device = {

@@ -788,16 +778,6 @@ static struct resource da850_mmcsd1_resources[] = {
 		.end	= IRQ_DA850_MMCSDINT0_1,
 		.flags	= IORESOURCE_IRQ,
 	},
-	{		/* DMA RX */
-		.start	= DA850_DMA_MMCSD1_RX,
-		.end	= DA850_DMA_MMCSD1_RX,
-		.flags	= IORESOURCE_DMA,
-	},
-	{		/* DMA TX */
-		.start	= DA850_DMA_MMCSD1_TX,
-		.end	= DA850_DMA_MMCSD1_TX,
-		.flags	= IORESOURCE_DMA,
-	},
 };
 
 static struct platform_device da850_mmcsd1_device = {
arch/arm/mach-davinci/devices.c +0 -16
index 6257aa4..67d26c5 100644
@@ -144,14 +144,6 @@ static struct resource mmcsd0_resources[] = {
 		.start = IRQ_SDIOINT,
 		.flags = IORESOURCE_IRQ,
 	},
-	/* DMA channels: RX, then TX */
-	{
-		.start = EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCRXEVT),
-		.flags = IORESOURCE_DMA,
-	}, {
-		.start = EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCTXEVT),
-		.flags = IORESOURCE_DMA,
-	},
 };
 
 static struct platform_device davinci_mmcsd0_device = {

@@ -181,14 +173,6 @@ static struct resource mmcsd1_resources[] = {
 		.start = IRQ_DM355_SDIOINT1,
 		.flags = IORESOURCE_IRQ,
 	},
-	/* DMA channels: RX, then TX */
-	{
-		.start = EDMA_CTLR_CHAN(0, 30),	/* rx */
-		.flags = IORESOURCE_DMA,
-	}, {
-		.start = EDMA_CTLR_CHAN(0, 31),	/* tx */
-		.flags = IORESOURCE_DMA,
-	},
 };
 
 static struct platform_device davinci_mmcsd1_device = {
Signature · 1 line
1.9.1
Acked-by on v2 5/5
#26 lore raw

Re: [PATCH v2 5/5] arm: davinci: remove mmc dma resources

Quoted 4 lines · David Lechner

The davinci_mmc driver no longer uses platform resources for getting dma channels. Instead lookup is now done using dma_slave_map.

Signed-off-by David Lechner <[email protected]>

I guess this will now be targeted for v4.7. I can apply the patch if I get an immutable branch from Ulf post v4.6-rc1 or if Ulf decides to apply this, fine with me.

Acked-by Sekhar Nori <[email protected]>

BTW, we prefer a captialized "ARM:" in subject prefix. Whoever is applying can probably fix it though.

Thanks,
Sekhar

#27 lore raw

Re: [PATCH v2 0/5] davinci_mmc fixes

In reply to David Lechner

Hi David,

Quoted 2 lines · David Lechner

I've updated the patche set based on feedback. This is tested on linux-next with LEGO MINDSTORMS EV3 (da850), both with and without device tree enabled.

When posting an updated series, please post as a fresh thread. Not as reply to the original posting. With the later, there are more chances of series being lost.

Thanks,
Sekhar