Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1382997 > unrolled thread

[PATCH v2 0/7] mmc: sdhci-pltfm: fix and tidy up sdhci_pltfm_init()

Started byMasahiro Yamada <yamada.masahiro@socionext.com>
First post2016-04-20 04:20 +0200
Last post2016-04-22 15:00 +0200
Articles 10 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/7] mmc: sdhci-pltfm: fix and tidy up sdhci_pltfm_init() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-04-20 04:20 +0200
    [PATCH v2 2/7] mmc: sdhci-pltfm: check return value of platform_get_irq() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-04-20 04:20 +0200
    [PATCH v2 4/7] mmc: sdhci-pltfm: use devm_ioremap() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-04-20 04:20 +0200
    [PATCH v2 3/7] mmc: sdhci-pltfm: use devm_request_mem_region() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-04-20 04:20 +0200
    [PATCH v2 5/7] mmc: sdhci-pltfm: use devm_ioremap_resource() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-04-20 04:20 +0200
    [PATCH v2 1/7] mmc: sdhci-pltfm: drop error message for too small MMIO resource size Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-04-20 04:20 +0200
    [PATCH v2 7/7] mmc: sdhci-pltfm: call platform_get_irq() before sdhci_alloc_host() Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-04-20 04:20 +0200
    [PATCH v2 6/7] mmc: sdhci-pltfm: move devm_ioremap_resource() up Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-04-20 04:20 +0200
    Re: [PATCH v2 0/7] mmc: sdhci-pltfm: fix and tidy up  sdhci_pltfm_init() Adrian Hunter <adrian.hunter@intel.com> - 2016-04-20 08:20 +0200
    Re: [PATCH v2 0/7] mmc: sdhci-pltfm: fix and tidy up sdhci_pltfm_init() Ulf Hansson <ulf.hansson@linaro.org> - 2016-04-22 15:00 +0200

#1382997 — [PATCH v2 0/7] mmc: sdhci-pltfm: fix and tidy up sdhci_pltfm_init()

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-04-20 04:20 +0200
Subject[PATCH v2 0/7] mmc: sdhci-pltfm: fix and tidy up sdhci_pltfm_init()
Message-ID<rpPUe-oM-1@gated-at.bofh.it>

Changes in v2:
  - Remove resource size checking rather than fix it.
  - Add \n to the tail of the error message

Masahiro Yamada (7):
  mmc: sdhci-pltfm: drop error message for too small MMIO resource size
  mmc: sdhci-pltfm: check return value of platform_get_irq()
  mmc: sdhci-pltfm: use devm_request_mem_region()
  mmc: sdhci-pltfm: use devm_ioremap()
  mmc: sdhci-pltfm: use devm_ioremap_resource()
  mmc: sdhci-pltfm: move devm_ioremap_resource() up
  mmc: sdhci-pltfm: call platform_get_irq() before sdhci_alloc_host()

 drivers/mmc/host/sdhci-pltfm.c | 42 +++++++++++++-----------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1382998 — [PATCH v2 2/7] mmc: sdhci-pltfm: check return value of platform_get_irq()

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-04-20 04:20 +0200
Subject[PATCH v2 2/7] mmc: sdhci-pltfm: check return value of platform_get_irq()
Message-ID<rpPUe-oM-5@gated-at.bofh.it>
In reply to#1382997
The function platform_get_irq() can fail; it returns a negative error
code on failure.  A negative IRQ number will make sdhci_add_host() fail
to request IRQ anyway, but it makes sense to let it fail earlier here.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Add \n to the tail of the error message

 drivers/mmc/host/sdhci-pltfm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 03dbdeb..24377a3 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -146,6 +146,11 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 	}
 
 	host->irq = platform_get_irq(pdev, 0);
+	if (host->irq < 0) {
+		dev_err(&pdev->dev, "failed to get IRQ number\n");
+		ret = host->irq;
+		goto err_request;
+	}
 
 	if (!request_mem_region(iomem->start, resource_size(iomem),
 		mmc_hostname(host->mmc))) {
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1382999 — [PATCH v2 4/7] mmc: sdhci-pltfm: use devm_ioremap()

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-04-20 04:20 +0200
Subject[PATCH v2 4/7] mmc: sdhci-pltfm: use devm_ioremap()
Message-ID<rpPUe-oM-3@gated-at.bofh.it>
In reply to#1382997
Use the managed variant of ioremap().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 drivers/mmc/host/sdhci-pltfm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 8527a7c..5213287 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -160,7 +160,8 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		goto err_request;
 	}
 
-	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
+	host->ioaddr = devm_ioremap(&pdev->dev, iomem->start,
+				    resource_size(iomem));
 	if (!host->ioaddr) {
 		dev_err(&pdev->dev, "failed to remap registers\n");
 		ret = -ENOMEM;
@@ -190,7 +191,6 @@ void sdhci_pltfm_free(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 
-	iounmap(host->ioaddr);
 	sdhci_free_host(host);
 }
 EXPORT_SYMBOL_GPL(sdhci_pltfm_free);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383000 — [PATCH v2 3/7] mmc: sdhci-pltfm: use devm_request_mem_region()

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-04-20 04:20 +0200
Subject[PATCH v2 3/7] mmc: sdhci-pltfm: use devm_request_mem_region()
Message-ID<rpPUe-oM-7@gated-at.bofh.it>
In reply to#1382997
Use the managed variant of request_mem_region().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 drivers/mmc/host/sdhci-pltfm.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 24377a3..8527a7c 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -152,8 +152,9 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		goto err_request;
 	}
 
-	if (!request_mem_region(iomem->start, resource_size(iomem),
-		mmc_hostname(host->mmc))) {
+	if (!devm_request_mem_region(&pdev->dev, iomem->start,
+				     resource_size(iomem),
+				     mmc_hostname(host->mmc))) {
 		dev_err(&pdev->dev, "cannot request region\n");
 		ret = -EBUSY;
 		goto err_request;
@@ -163,7 +164,7 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 	if (!host->ioaddr) {
 		dev_err(&pdev->dev, "failed to remap registers\n");
 		ret = -ENOMEM;
-		goto err_remap;
+		goto err_request;
 	}
 
 	/*
@@ -177,8 +178,6 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 
 	return host;
 
-err_remap:
-	release_mem_region(iomem->start, resource_size(iomem));
 err_request:
 	sdhci_free_host(host);
 err:
@@ -190,10 +189,8 @@ EXPORT_SYMBOL_GPL(sdhci_pltfm_init);
 void sdhci_pltfm_free(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
-	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	iounmap(host->ioaddr);
-	release_mem_region(iomem->start, resource_size(iomem));
 	sdhci_free_host(host);
 }
 EXPORT_SYMBOL_GPL(sdhci_pltfm_free);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383001 — [PATCH v2 5/7] mmc: sdhci-pltfm: use devm_ioremap_resource()

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-04-20 04:20 +0200
Subject[PATCH v2 5/7] mmc: sdhci-pltfm: use devm_ioremap_resource()
Message-ID<rpPUe-oM-11@gated-at.bofh.it>
In reply to#1382997
The chain of devm_request_mem_region() and devm_ioremap() can be
replaced with devm_ioremap_resource().  Also, we can drop the error
messages because devm_ioremap_resource() displays similar messages
on error.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 drivers/mmc/host/sdhci-pltfm.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 5213287..caa05d7 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -152,19 +152,9 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		goto err_request;
 	}
 
-	if (!devm_request_mem_region(&pdev->dev, iomem->start,
-				     resource_size(iomem),
-				     mmc_hostname(host->mmc))) {
-		dev_err(&pdev->dev, "cannot request region\n");
-		ret = -EBUSY;
-		goto err_request;
-	}
-
-	host->ioaddr = devm_ioremap(&pdev->dev, iomem->start,
-				    resource_size(iomem));
-	if (!host->ioaddr) {
-		dev_err(&pdev->dev, "failed to remap registers\n");
-		ret = -ENOMEM;
+	host->ioaddr = devm_ioremap_resource(&pdev->dev, iomem);
+	if (IS_ERR(host->ioaddr)) {
+		ret = PTR_ERR(host->ioaddr);
 		goto err_request;
 	}
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383002 — [PATCH v2 1/7] mmc: sdhci-pltfm: drop error message for too small MMIO resource size

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-04-20 04:20 +0200
Subject[PATCH v2 1/7] mmc: sdhci-pltfm: drop error message for too small MMIO resource size
Message-ID<rpPUe-oM-15@gated-at.bofh.it>
In reply to#1382997
The requirement resource_size >= 0x100 may not necessarily be
reasonable; for example, sdhci-dove appears to sidestep some
registers in sdhci_dove_readw().

Moreover, current code displays an error message for too small
resource size, but still moves forward.

Every DT should be responsible for describing its properties
correctly, so lets's remove this error message from the common
framework.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Remove resource size checking rather than fix it.

 drivers/mmc/host/sdhci-pltfm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 072bb27..03dbdeb 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -127,9 +127,6 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		goto err;
 	}
 
-	if (resource_size(iomem) < 0x100)
-		dev_err(&pdev->dev, "Invalid iomem size!\n");
-
 	host = sdhci_alloc_host(&pdev->dev,
 		sizeof(struct sdhci_pltfm_host) + priv_size);
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383003 — [PATCH v2 7/7] mmc: sdhci-pltfm: call platform_get_irq() before sdhci_alloc_host()

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-04-20 04:20 +0200
Subject[PATCH v2 7/7] mmc: sdhci-pltfm: call platform_get_irq() before sdhci_alloc_host()
Message-ID<rpPUe-oM-9@gated-at.bofh.it>
In reply to#1382997
Swap the call order of sdhci_alloc_host() and platform_get_irq().
It makes sdhci_alloc_host() the last function that can fail in the
sdhci_pltfm_init().  So, we can drop the sdhci_free_host() call from
the failure path.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 drivers/mmc/host/sdhci-pltfm.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 1d74db8..64f287a 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -120,7 +120,7 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 	struct sdhci_host *host;
 	struct resource *iomem;
 	void __iomem *ioaddr;
-	int ret;
+	int irq, ret;
 
 	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ioaddr = devm_ioremap_resource(&pdev->dev, iomem);
@@ -129,6 +129,13 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		goto err;
 	}
 
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get IRQ number\n");
+		ret = irq;
+		goto err;
+	}
+
 	host = sdhci_alloc_host(&pdev->dev,
 		sizeof(struct sdhci_pltfm_host) + priv_size);
 
@@ -138,6 +145,7 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 	}
 
 	host->ioaddr = ioaddr;
+	host->irq = irq;
 	host->hw_name = dev_name(&pdev->dev);
 	if (pdata && pdata->ops)
 		host->ops = pdata->ops;
@@ -148,13 +156,6 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		host->quirks2 = pdata->quirks2;
 	}
 
-	host->irq = platform_get_irq(pdev, 0);
-	if (host->irq < 0) {
-		dev_err(&pdev->dev, "failed to get IRQ number\n");
-		ret = host->irq;
-		goto err_request;
-	}
-
 	/*
 	 * Some platforms need to probe the controller to be able to
 	 * determine which caps should be used.
@@ -165,9 +166,6 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 	platform_set_drvdata(pdev, host);
 
 	return host;
-
-err_request:
-	sdhci_free_host(host);
 err:
 	dev_err(&pdev->dev, "%s failed %d\n", __func__, ret);
 	return ERR_PTR(ret);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383004 — [PATCH v2 6/7] mmc: sdhci-pltfm: move devm_ioremap_resource() up

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2016-04-20 04:20 +0200
Subject[PATCH v2 6/7] mmc: sdhci-pltfm: move devm_ioremap_resource() up
Message-ID<rpPUe-oM-13@gated-at.bofh.it>
In reply to#1382997
Call devm_ioremap_resource() right after platform_get_resource().
This saves the error check of platform_get_resource() because
devm_ioremap_resource() checks if the given resource is NULL.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 drivers/mmc/host/sdhci-pltfm.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index caa05d7..1d74db8 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -119,11 +119,13 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 {
 	struct sdhci_host *host;
 	struct resource *iomem;
+	void __iomem *ioaddr;
 	int ret;
 
 	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!iomem) {
-		ret = -ENOMEM;
+	ioaddr = devm_ioremap_resource(&pdev->dev, iomem);
+	if (IS_ERR(ioaddr)) {
+		ret = PTR_ERR(ioaddr);
 		goto err;
 	}
 
@@ -135,6 +137,7 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		goto err;
 	}
 
+	host->ioaddr = ioaddr;
 	host->hw_name = dev_name(&pdev->dev);
 	if (pdata && pdata->ops)
 		host->ops = pdata->ops;
@@ -152,12 +155,6 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		goto err_request;
 	}
 
-	host->ioaddr = devm_ioremap_resource(&pdev->dev, iomem);
-	if (IS_ERR(host->ioaddr)) {
-		ret = PTR_ERR(host->ioaddr);
-		goto err_request;
-	}
-
 	/*
 	 * Some platforms need to probe the controller to be able to
 	 * determine which caps should be used.
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383068 — Re: [PATCH v2 0/7] mmc: sdhci-pltfm: fix and tidy up sdhci_pltfm_init()

FromAdrian Hunter <adrian.hunter@intel.com>
Date2016-04-20 08:20 +0200
SubjectRe: [PATCH v2 0/7] mmc: sdhci-pltfm: fix and tidy up sdhci_pltfm_init()
Message-ID<rpTEu-3fC-11@gated-at.bofh.it>
In reply to#1382997
On 20/04/16 05:16, Masahiro Yamada wrote:
> 
> 
> Changes in v2:
>   - Remove resource size checking rather than fix it.
>   - Add \n to the tail of the error message
> 
> Masahiro Yamada (7):
>   mmc: sdhci-pltfm: drop error message for too small MMIO resource size
>   mmc: sdhci-pltfm: check return value of platform_get_irq()
>   mmc: sdhci-pltfm: use devm_request_mem_region()
>   mmc: sdhci-pltfm: use devm_ioremap()
>   mmc: sdhci-pltfm: use devm_ioremap_resource()
>   mmc: sdhci-pltfm: move devm_ioremap_resource() up
>   mmc: sdhci-pltfm: call platform_get_irq() before sdhci_alloc_host()
> 
>  drivers/mmc/host/sdhci-pltfm.c | 42 +++++++++++++-----------------------------
>  1 file changed, 13 insertions(+), 29 deletions(-)
> 

Thank you!

For all 7 patches:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

[toc] | [prev] | [next] | [standalone]

#1385142

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-04-22 15:00 +0200
Message-ID<rqIQH-234-31@gated-at.bofh.it>
In reply to#1382997
On 20 April 2016 at 04:16, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
>
> Changes in v2:
>   - Remove resource size checking rather than fix it.
>   - Add \n to the tail of the error message
>
> Masahiro Yamada (7):
>   mmc: sdhci-pltfm: drop error message for too small MMIO resource size
>   mmc: sdhci-pltfm: check return value of platform_get_irq()
>   mmc: sdhci-pltfm: use devm_request_mem_region()
>   mmc: sdhci-pltfm: use devm_ioremap()
>   mmc: sdhci-pltfm: use devm_ioremap_resource()
>   mmc: sdhci-pltfm: move devm_ioremap_resource() up
>   mmc: sdhci-pltfm: call platform_get_irq() before sdhci_alloc_host()
>
>  drivers/mmc/host/sdhci-pltfm.c | 42 +++++++++++++-----------------------------
>  1 file changed, 13 insertions(+), 29 deletions(-)
>
> --
> 1.9.1
>

Thanks, applied for next!

Kind regards
Uffe

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web