[PATCH v2 0/7] mmc: sdhci: set .remove to sdhci_pltfm_unregister()

[PATCH v2 0/7] mmc: sdhci: set .remove to sdhci_pltfm_unregister()

[Kevin Hao]

Hi,

v2:
  - Update the sdhci-bcm-kona as suggested by Ray Jui.
  - Also kill the private get_max_clock function in sdhci-sirf.

v1:
In the current kernel, the specific .remove function in several sdhci platform
drivers seem pretty similar. Actually we can just set all of them to
sdhci_pltfm_unregister() with just a bit twist.

Only run test for sdhci-of-esdhc driver. All others just passed the build test.

Kevin Hao (7):
  mmc: sdhci-dove: remove the unneeded error check
  mmc: sdhci-dove: kill the driver specific private struct
  mmc: tegra: use devm help functions to get the clk and gpio
  mmc: sdhci-sirf: kill the "clk" member in driver private struct
  mmc: sdhci-bcm-kona: kill the "external_clk" member in driver private 
       struct
  mmc: sdhci: disable the clock in sdhci_pltfm_unregister()
  mmc: sdhci: set the .remove to sdhci_pltfm_unregister()

 drivers/mmc/host/sdhci-bcm-kona.c  | 61 ++++++++------------------------------
 drivers/mmc/host/sdhci-bcm2835.c   |  7 +----
 drivers/mmc/host/sdhci-cns3xxx.c   |  7 +----
 drivers/mmc/host/sdhci-dove.c      | 39 ++++--------------------
 drivers/mmc/host/sdhci-of-arasan.c |  1 -
 drivers/mmc/host/sdhci-of-esdhc.c  |  7 +----
 drivers/mmc/host/sdhci-of-hlwd.c   |  7 +----
 drivers/mmc/host/sdhci-pltfm.c     |  2 ++
 drivers/mmc/host/sdhci-sirf.c      | 36 +++++-----------------
 drivers/mmc/host/sdhci-st.c        | 12 +-------
 drivers/mmc/host/sdhci-tegra.c     | 30 +++----------------
 11 files changed, 36 insertions(+), 173 deletions(-)

-- 
1.9.3

[PATCH v2 1/7] mmc: sdhci-dove: remove the unneeded error check

[Kevin Hao]

The function clk_disable_unprepare() already take care of either error
or null cases.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
v2: No change.

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

diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index ca969d271a27..0a7aeb162497 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -117,8 +117,7 @@ static int sdhci_dove_probe(struct platform_device *pdev)
 	return 0;
 
 err_sdhci_add:
-	if (!IS_ERR(priv->clk))
-		clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk);
 	sdhci_pltfm_free(pdev);
 	return ret;
 }
@@ -131,8 +130,7 @@ static int sdhci_dove_remove(struct platform_device *pdev)
 
 	sdhci_pltfm_unregister(pdev);
 
-	if (!IS_ERR(priv->clk))
-		clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk);
 
 	return 0;
 }
-- 
1.9.3

[PATCH v2 2/7] mmc: sdhci-dove: kill the driver specific private struct

[Kevin Hao]

There is only one "clk" member in this driver specific private struct.
Actually we can use the "clk" member in the struct sdhci_pltfm_host,
and then kill this struct completely.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
v2: No change.

 drivers/mmc/host/sdhci-dove.c | 26 +++++---------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index 0a7aeb162497..f78440ef1ba4 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -28,10 +28,6 @@
 
 #include "sdhci-pltfm.h"
 
-struct sdhci_dove_priv {
-	struct clk *clk;
-};
-
 static u16 sdhci_dove_readw(struct sdhci_host *host, int reg)
 {
 	u16 ret;
@@ -84,27 +80,17 @@ static int sdhci_dove_probe(struct platform_device *pdev)
 {
 	struct sdhci_host *host;
 	struct sdhci_pltfm_host *pltfm_host;
-	struct sdhci_dove_priv *priv;
 	int ret;
 
-	priv = devm_kzalloc(&pdev->dev, sizeof(struct sdhci_dove_priv),
-			    GFP_KERNEL);
-	if (!priv) {
-		dev_err(&pdev->dev, "unable to allocate private data");
-		return -ENOMEM;
-	}
-
-	priv->clk = devm_clk_get(&pdev->dev, NULL);
-
 	host = sdhci_pltfm_init(pdev, &sdhci_dove_pdata, 0);
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
 	pltfm_host = sdhci_priv(host);
-	pltfm_host->priv = priv;
+	pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
 
-	if (!IS_ERR(priv->clk))
-		clk_prepare_enable(priv->clk);
+	if (!IS_ERR(pltfm_host->clk))
+		clk_prepare_enable(pltfm_host->clk);
 
 	ret = mmc_of_parse(host->mmc);
 	if (ret)
@@ -117,7 +103,7 @@ static int sdhci_dove_probe(struct platform_device *pdev)
 	return 0;
 
 err_sdhci_add:
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(pltfm_host->clk);
 	sdhci_pltfm_free(pdev);
 	return ret;
 }
@@ -126,12 +112,10 @@ static int sdhci_dove_remove(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_dove_priv *priv = pltfm_host->priv;
 
+	clk_disable_unprepare(pltfm_host->clk);
 	sdhci_pltfm_unregister(pdev);
 
-	clk_disable_unprepare(priv->clk);
-
 	return 0;
 }
 
-- 
1.9.3

[PATCH v2 3/7] mmc: tegra: use devm help functions to get the clk and gpio

[Kevin Hao]

Simplify the error and remove path.

Signed-off-by: Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
v2: No change.

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

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index f3778d58d1cd..0643f66b4e14 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -286,7 +286,8 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 		goto err_parse_dt;
 
 	if (gpio_is_valid(tegra_host->power_gpio)) {
-		rc = gpio_request(tegra_host->power_gpio, "sdhci_power");
+		rc = devm_gpio_request(&pdev->dev, tegra_host->power_gpio,
+				       "sdhci_power");
 		if (rc) {
 			dev_err(mmc_dev(host->mmc),
 				"failed to allocate power gpio\n");
@@ -295,7 +296,7 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 		gpio_direction_output(tegra_host->power_gpio, 1);
 	}
 
-	clk = clk_get(mmc_dev(host->mmc), NULL);
+	clk = devm_clk_get(mmc_dev(host->mmc), NULL);
 	if (IS_ERR(clk)) {
 		dev_err(mmc_dev(host->mmc), "clk err\n");
 		rc = PTR_ERR(clk);
@@ -312,10 +313,7 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 
 err_add_host:
 	clk_disable_unprepare(pltfm_host->clk);
-	clk_put(pltfm_host->clk);
 err_clk_get:
-	if (gpio_is_valid(tegra_host->power_gpio))
-		gpio_free(tegra_host->power_gpio);
 err_power_req:
 err_parse_dt:
 err_alloc_tegra_host:
@@ -327,16 +325,11 @@ static int sdhci_tegra_remove(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_tegra *tegra_host = pltfm_host->priv;
 	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
 
 	sdhci_remove_host(host, dead);
 
-	if (gpio_is_valid(tegra_host->power_gpio))
-		gpio_free(tegra_host->power_gpio);
-
 	clk_disable_unprepare(pltfm_host->clk);
-	clk_put(pltfm_host->clk);
 
 	sdhci_pltfm_free(pdev);
 
-- 
1.9.3

Re: [PATCH v2 3/7] mmc: tegra: use devm help functions to get the clk and gpio

[Alexandre Courbot]

On Fri, Feb 27, 2015 at 4:47 PM, Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Simplify the error and remove path.
>
> Signed-off-by: Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> v2: No change.
>
>  drivers/mmc/host/sdhci-tegra.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)

Acked-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[PATCH v2 4/7] mmc: sdhci-sirf: kill the "clk" member in driver private struct

[Kevin Hao]

Actually we can use the "clk" in the struct sdhci_pltfm_host.
With this change we can also kill the private function for get
max clock in this driver.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
v2: Also kill the private sdhci_sirf_get_max_clk().

 drivers/mmc/host/sdhci-sirf.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
index f6f82ec3618d..c30d6f3dd9aa 100644
--- a/drivers/mmc/host/sdhci-sirf.c
+++ b/drivers/mmc/host/sdhci-sirf.c
@@ -20,17 +20,9 @@
 #define SIRF_TUNING_COUNT 128
 
 struct sdhci_sirf_priv {
-	struct clk *clk;
 	int gpio_cd;
 };
 
-static unsigned int sdhci_sirf_get_max_clk(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_sirf_priv *priv = sdhci_pltfm_priv(pltfm_host);
-	return clk_get_rate(priv->clk);
-}
-
 static void sdhci_sirf_set_bus_width(struct sdhci_host *host, int width)
 {
 	u8 ctrl;
@@ -122,7 +114,7 @@ retry:
 static struct sdhci_ops sdhci_sirf_ops = {
 	.platform_execute_tuning = sdhci_sirf_execute_tuning,
 	.set_clock = sdhci_set_clock,
-	.get_max_clock	= sdhci_sirf_get_max_clk,
+	.get_max_clock	= sdhci_pltfm_clk_get_max_clock,
 	.set_bus_width = sdhci_sirf_set_bus_width,
 	.reset = sdhci_reset,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
@@ -162,13 +154,13 @@ static int sdhci_sirf_probe(struct platform_device *pdev)
 		return PTR_ERR(host);
 
 	pltfm_host = sdhci_priv(host);
+	pltfm_host->clk = clk;
 	priv = sdhci_pltfm_priv(pltfm_host);
-	priv->clk = clk;
 	priv->gpio_cd = gpio_cd;
 
 	sdhci_get_of_property(pdev);
 
-	ret = clk_prepare_enable(priv->clk);
+	ret = clk_prepare_enable(pltfm_host->clk);
 	if (ret)
 		goto err_clk_prepare;
 
@@ -195,7 +187,7 @@ static int sdhci_sirf_probe(struct platform_device *pdev)
 err_request_cd:
 	sdhci_remove_host(host, 0);
 err_sdhci_add:
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(pltfm_host->clk);
 err_clk_prepare:
 	sdhci_pltfm_free(pdev);
 	return ret;
@@ -205,11 +197,9 @@ static int sdhci_sirf_remove(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_sirf_priv *priv = sdhci_pltfm_priv(pltfm_host);
 
+	clk_disable_unprepare(pltfm_host->clk);
 	sdhci_pltfm_unregister(pdev);
-
-	clk_disable_unprepare(priv->clk);
 	return 0;
 }
 
@@ -218,14 +208,13 @@ static int sdhci_sirf_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_sirf_priv *priv = sdhci_pltfm_priv(pltfm_host);
 	int ret;
 
 	ret = sdhci_suspend_host(host);
 	if (ret)
 		return ret;
 
-	clk_disable(priv->clk);
+	clk_disable(pltfm_host->clk);
 
 	return 0;
 }
@@ -234,10 +223,9 @@ static int sdhci_sirf_resume(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_sirf_priv *priv = sdhci_pltfm_priv(pltfm_host);
 	int ret;
 
-	ret = clk_enable(priv->clk);
+	ret = clk_enable(pltfm_host->clk);
 	if (ret) {
 		dev_dbg(dev, "Resume: Error enabling clock\n");
 		return ret;
-- 
1.9.3

[PATCH v2 5/7] mmc: sdhci-bcm-kona: kill the "external_clk" member in driver private struct

[Kevin Hao]

Actually we can use the "clk" in the struct sdhci_pltfm_host. Also
change the "external clock" to "core clock" and kill two redundant
private functions in this driver as suggested by Ray Jui.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
v2:
  - s/external clock/core clock/
  - kill the private functions sdhci_bcm_kona_get_max_clk() and sdhci_bcm_kona_get_timeout_clock()

 drivers/mmc/host/sdhci-bcm-kona.c | 46 +++++++++++----------------------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
index 34bb8f92586e..5bc263b447b2 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -54,7 +54,6 @@
 
 struct sdhci_bcm_kona_dev {
 	struct mutex	write_lock; /* protect back to back writes */
-	struct clk	*external_clk;
 };
 
 
@@ -175,24 +174,6 @@ static void sdhci_bcm_kona_card_event(struct sdhci_host *host)
 	}
 }
 
-/*
- * Get the base clock. Use central clock source for now. Not sure if different
- * clock speed to each dev is allowed
- */
-static unsigned int sdhci_bcm_kona_get_max_clk(struct sdhci_host *host)
-{
-	struct sdhci_bcm_kona_dev *kona_dev;
-	struct sdhci_pltfm_host *pltfm_priv = sdhci_priv(host);
-	kona_dev = sdhci_pltfm_priv(pltfm_priv);
-
-	return host->mmc->f_max;
-}
-
-static unsigned int sdhci_bcm_kona_get_timeout_clock(struct sdhci_host *host)
-{
-	return sdhci_bcm_kona_get_max_clk(host);
-}
-
 static void sdhci_bcm_kona_init_74_clocks(struct sdhci_host *host,
 				u8 power_mode)
 {
@@ -207,8 +188,8 @@ static void sdhci_bcm_kona_init_74_clocks(struct sdhci_host *host,
 
 static struct sdhci_ops sdhci_bcm_kona_ops = {
 	.set_clock = sdhci_set_clock,
-	.get_max_clock = sdhci_bcm_kona_get_max_clk,
-	.get_timeout_clock = sdhci_bcm_kona_get_timeout_clock,
+	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
+	.get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
 	.platform_send_init_74_clocks = sdhci_bcm_kona_init_74_clocks,
 	.set_bus_width = sdhci_set_bus_width,
 	.reset = sdhci_reset,
@@ -264,21 +245,21 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
 		goto err_pltfm_free;
 	}
 
-	/* Get and enable the external clock */
-	kona_dev->external_clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(kona_dev->external_clk)) {
-		dev_err(dev, "Failed to get external clock\n");
-		ret = PTR_ERR(kona_dev->external_clk);
+	/* Get and enable the core clock */
+	pltfm_priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(pltfm_priv->clk)) {
+		dev_err(dev, "Failed to get core clock\n");
+		ret = PTR_ERR(pltfm_priv->clk);
 		goto err_pltfm_free;
 	}
 
-	if (clk_set_rate(kona_dev->external_clk, host->mmc->f_max) != 0) {
-		dev_err(dev, "Failed to set rate external clock\n");
+	if (clk_set_rate(pltfm_priv->clk, host->mmc->f_max) != 0) {
+		dev_err(dev, "Failed to set rate core clock\n");
 		goto err_pltfm_free;
 	}
 
-	if (clk_prepare_enable(kona_dev->external_clk) != 0) {
-		dev_err(dev, "Failed to enable external clock\n");
+	if (clk_prepare_enable(pltfm_priv->clk) != 0) {
+		dev_err(dev, "Failed to enable core clock\n");
 		goto err_pltfm_free;
 	}
 
@@ -333,7 +314,7 @@ err_reset:
 	sdhci_bcm_kona_sd_reset(host);
 
 err_clk_disable:
-	clk_disable_unprepare(kona_dev->external_clk);
+	clk_disable_unprepare(pltfm_priv->clk);
 
 err_pltfm_free:
 	sdhci_pltfm_free(pdev);
@@ -346,12 +327,11 @@ static int sdhci_bcm_kona_remove(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_priv = sdhci_priv(host);
-	struct sdhci_bcm_kona_dev *kona_dev = sdhci_pltfm_priv(pltfm_priv);
 	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
 
 	sdhci_remove_host(host, dead);
 
-	clk_disable_unprepare(kona_dev->external_clk);
+	clk_disable_unprepare(pltfm_priv->clk);
 
 	sdhci_pltfm_free(pdev);
 
-- 
1.9.3

Re: [PATCH v2 5/7] mmc: sdhci-bcm-kona: kill the "external_clk" member in driver private struct

[Ray Jui]

Hi Kevin,

On 2/26/2015 11:47 PM, Kevin Hao wrote:
> Actually we can use the "clk" in the struct sdhci_pltfm_host. Also
> change the "external clock" to "core clock" and kill two redundant
> private functions in this driver as suggested by Ray Jui.
> 
> Signed-off-by: Kevin Hao <haokexin@gmail.com>
> ---
> v2:
>   - s/external clock/core clock/
>   - kill the private functions sdhci_bcm_kona_get_max_clk() and sdhci_bcm_kona_get_timeout_clock()
> 
>  drivers/mmc/host/sdhci-bcm-kona.c | 46 +++++++++++----------------------------
>  1 file changed, 13 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
> index 34bb8f92586e..5bc263b447b2 100644
> --- a/drivers/mmc/host/sdhci-bcm-kona.c
> +++ b/drivers/mmc/host/sdhci-bcm-kona.c
> @@ -54,7 +54,6 @@
>  
>  struct sdhci_bcm_kona_dev {
>  	struct mutex	write_lock; /* protect back to back writes */
> -	struct clk	*external_clk;
>  };
>  
>  
> @@ -175,24 +174,6 @@ static void sdhci_bcm_kona_card_event(struct sdhci_host *host)
>  	}
>  }
>  
> -/*
> - * Get the base clock. Use central clock source for now. Not sure if different
> - * clock speed to each dev is allowed
> - */
> -static unsigned int sdhci_bcm_kona_get_max_clk(struct sdhci_host *host)
> -{
> -	struct sdhci_bcm_kona_dev *kona_dev;
> -	struct sdhci_pltfm_host *pltfm_priv = sdhci_priv(host);
> -	kona_dev = sdhci_pltfm_priv(pltfm_priv);
> -
> -	return host->mmc->f_max;
> -}
> -
> -static unsigned int sdhci_bcm_kona_get_timeout_clock(struct sdhci_host *host)
> -{
> -	return sdhci_bcm_kona_get_max_clk(host);
> -}
> -
>  static void sdhci_bcm_kona_init_74_clocks(struct sdhci_host *host,
>  				u8 power_mode)
>  {
> @@ -207,8 +188,8 @@ static void sdhci_bcm_kona_init_74_clocks(struct sdhci_host *host,
>  
>  static struct sdhci_ops sdhci_bcm_kona_ops = {
>  	.set_clock = sdhci_set_clock,
> -	.get_max_clock = sdhci_bcm_kona_get_max_clk,
> -	.get_timeout_clock = sdhci_bcm_kona_get_timeout_clock,
> +	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +	.get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
>  	.platform_send_init_74_clocks = sdhci_bcm_kona_init_74_clocks,
>  	.set_bus_width = sdhci_set_bus_width,
>  	.reset = sdhci_reset,
> @@ -264,21 +245,21 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
>  		goto err_pltfm_free;
>  	}
>  
> -	/* Get and enable the external clock */
> -	kona_dev->external_clk = devm_clk_get(dev, NULL);
> -	if (IS_ERR(kona_dev->external_clk)) {
> -		dev_err(dev, "Failed to get external clock\n");
> -		ret = PTR_ERR(kona_dev->external_clk);
> +	/* Get and enable the core clock */
> +	pltfm_priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(pltfm_priv->clk)) {
> +		dev_err(dev, "Failed to get core clock\n");
> +		ret = PTR_ERR(pltfm_priv->clk);
>  		goto err_pltfm_free;
>  	}
>  
> -	if (clk_set_rate(kona_dev->external_clk, host->mmc->f_max) != 0) {
> -		dev_err(dev, "Failed to set rate external clock\n");
> +	if (clk_set_rate(pltfm_priv->clk, host->mmc->f_max) != 0) {
> +		dev_err(dev, "Failed to set rate core clock\n");
>  		goto err_pltfm_free;
>  	}
>  
> -	if (clk_prepare_enable(kona_dev->external_clk) != 0) {
> -		dev_err(dev, "Failed to enable external clock\n");
> +	if (clk_prepare_enable(pltfm_priv->clk) != 0) {
> +		dev_err(dev, "Failed to enable core clock\n");
>  		goto err_pltfm_free;
>  	}
>  
> @@ -333,7 +314,7 @@ err_reset:
>  	sdhci_bcm_kona_sd_reset(host);
>  
>  err_clk_disable:
> -	clk_disable_unprepare(kona_dev->external_clk);
> +	clk_disable_unprepare(pltfm_priv->clk);
>  
>  err_pltfm_free:
>  	sdhci_pltfm_free(pdev);
> @@ -346,12 +327,11 @@ static int sdhci_bcm_kona_remove(struct platform_device *pdev)
>  {
>  	struct sdhci_host *host = platform_get_drvdata(pdev);
>  	struct sdhci_pltfm_host *pltfm_priv = sdhci_priv(host);
> -	struct sdhci_bcm_kona_dev *kona_dev = sdhci_pltfm_priv(pltfm_priv);
>  	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
>  
>  	sdhci_remove_host(host, dead);
>  
> -	clk_disable_unprepare(kona_dev->external_clk);
> +	clk_disable_unprepare(pltfm_priv->clk);
>  
>  	sdhci_pltfm_free(pdev);
>  
> 

The change looks good to me. Thanks!

Reviewed-by: Ray Jui <rjui@broadcom.com>

[PATCH v2 6/7] mmc: sdhci: disable the clock in sdhci_pltfm_unregister()

[Kevin Hao]

So we can avoid to sprinkle the clk_disable_unprepare() in many
drivers.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
v2: No change.

 drivers/mmc/host/sdhci-bcm-kona.c  | 12 +-----------
 drivers/mmc/host/sdhci-dove.c      |  8 +-------
 drivers/mmc/host/sdhci-of-arasan.c |  1 -
 drivers/mmc/host/sdhci-pltfm.c     |  2 ++
 drivers/mmc/host/sdhci-sirf.c      |  5 +----
 drivers/mmc/host/sdhci-st.c        |  5 -----
 drivers/mmc/host/sdhci-tegra.c     | 12 +-----------
 7 files changed, 6 insertions(+), 39 deletions(-)

diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
index 5bc263b447b2..91cba26f75af 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -325,17 +325,7 @@ err_pltfm_free:
 
 static int sdhci_bcm_kona_remove(struct platform_device *pdev)
 {
-	struct sdhci_host *host = platform_get_drvdata(pdev);
-	struct sdhci_pltfm_host *pltfm_priv = sdhci_priv(host);
-	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
-
-	sdhci_remove_host(host, dead);
-
-	clk_disable_unprepare(pltfm_priv->clk);
-
-	sdhci_pltfm_free(pdev);
-
-	return 0;
+	return sdhci_pltfm_unregister(pdev);
 }
 
 static struct platform_driver sdhci_bcm_kona_driver = {
diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index f78440ef1ba4..2314ff88d942 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -110,13 +110,7 @@ err_sdhci_add:
 
 static int sdhci_dove_remove(struct platform_device *pdev)
 {
-	struct sdhci_host *host = platform_get_drvdata(pdev);
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-
-	clk_disable_unprepare(pltfm_host->clk);
-	sdhci_pltfm_unregister(pdev);
-
-	return 0;
+	return sdhci_pltfm_unregister(pdev);
 }
 
 static const struct of_device_id sdhci_dove_of_match_table[] = {
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index bcb51e9dfdcd..129079fb53bf 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -195,7 +195,6 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_arasan_data *sdhci_arasan = pltfm_host->priv;
 
-	clk_disable_unprepare(pltfm_host->clk);
 	clk_disable_unprepare(sdhci_arasan->clk_ahb);
 
 	return sdhci_pltfm_unregister(pdev);
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index c5b01d6bb85d..74c58d9a4fc8 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -225,9 +225,11 @@ EXPORT_SYMBOL_GPL(sdhci_pltfm_register);
 int sdhci_pltfm_unregister(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
 
 	sdhci_remove_host(host, dead);
+	clk_disable_unprepare(pltfm_host->clk);
 	sdhci_pltfm_free(pdev);
 
 	return 0;
diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
index c30d6f3dd9aa..f7dd68e55b49 100644
--- a/drivers/mmc/host/sdhci-sirf.c
+++ b/drivers/mmc/host/sdhci-sirf.c
@@ -196,11 +196,8 @@ err_clk_prepare:
 static int sdhci_sirf_remove(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 
-	clk_disable_unprepare(pltfm_host->clk);
-	sdhci_pltfm_unregister(pdev);
-	return 0;
+	return sdhci_pltfm_unregister(pdev);
 }
 
 #ifdef CONFIG_PM_SLEEP
diff --git a/drivers/mmc/host/sdhci-st.c b/drivers/mmc/host/sdhci-st.c
index 882b07e9667e..0019df452522 100644
--- a/drivers/mmc/host/sdhci-st.c
+++ b/drivers/mmc/host/sdhci-st.c
@@ -115,11 +115,6 @@ err_of:
 
 static int sdhci_st_remove(struct platform_device *pdev)
 {
-	struct sdhci_host *host = platform_get_drvdata(pdev);
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-
-	clk_disable_unprepare(pltfm_host->clk);
-
 	return sdhci_pltfm_unregister(pdev);
 }
 
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 0643f66b4e14..12881e05ff30 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -323,17 +323,7 @@ err_alloc_tegra_host:
 
 static int sdhci_tegra_remove(struct platform_device *pdev)
 {
-	struct sdhci_host *host = platform_get_drvdata(pdev);
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
-
-	sdhci_remove_host(host, dead);
-
-	clk_disable_unprepare(pltfm_host->clk);
-
-	sdhci_pltfm_free(pdev);
-
-	return 0;
+	return sdhci_pltfm_unregister(pdev);
 }
 
 static struct platform_driver sdhci_tegra_driver = {
-- 
1.9.3

[PATCH v2 7/7] mmc: sdhci: set the .remove to sdhci_pltfm_unregister()

[Kevin Hao]

In these drivers, the driver specific .remove function just a simple
wrapper of function sdhci_pltfm_unregister(). So remove these wrappers
and just set .remove to sdhci_pltfm_unregister().

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
v2: No change.

 drivers/mmc/host/sdhci-bcm-kona.c | 7 +------
 drivers/mmc/host/sdhci-bcm2835.c  | 7 +------
 drivers/mmc/host/sdhci-cns3xxx.c  | 7 +------
 drivers/mmc/host/sdhci-dove.c     | 7 +------
 drivers/mmc/host/sdhci-of-esdhc.c | 7 +------
 drivers/mmc/host/sdhci-of-hlwd.c  | 7 +------
 drivers/mmc/host/sdhci-sirf.c     | 9 +--------
 drivers/mmc/host/sdhci-st.c       | 7 +------
 drivers/mmc/host/sdhci-tegra.c    | 7 +------
 9 files changed, 9 insertions(+), 56 deletions(-)

diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
index 91cba26f75af..2bd90fb35c75 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -323,11 +323,6 @@ err_pltfm_free:
 	return ret;
 }
 
-static int sdhci_bcm_kona_remove(struct platform_device *pdev)
-{
-	return sdhci_pltfm_unregister(pdev);
-}
-
 static struct platform_driver sdhci_bcm_kona_driver = {
 	.driver		= {
 		.name	= "sdhci-kona",
@@ -335,7 +330,7 @@ static struct platform_driver sdhci_bcm_kona_driver = {
 		.of_match_table = sdhci_bcm_kona_of_match,
 	},
 	.probe		= sdhci_bcm_kona_probe,
-	.remove		= sdhci_bcm_kona_remove,
+	.remove		= sdhci_pltfm_unregister,
 };
 module_platform_driver(sdhci_bcm_kona_driver);
 
diff --git a/drivers/mmc/host/sdhci-bcm2835.c b/drivers/mmc/host/sdhci-bcm2835.c
index 439d259fdf1d..0ef0343c603a 100644
--- a/drivers/mmc/host/sdhci-bcm2835.c
+++ b/drivers/mmc/host/sdhci-bcm2835.c
@@ -180,11 +180,6 @@ err:
 	return ret;
 }
 
-static int bcm2835_sdhci_remove(struct platform_device *pdev)
-{
-	return sdhci_pltfm_unregister(pdev);
-}
-
 static const struct of_device_id bcm2835_sdhci_of_match[] = {
 	{ .compatible = "brcm,bcm2835-sdhci" },
 	{ }
@@ -198,7 +193,7 @@ static struct platform_driver bcm2835_sdhci_driver = {
 		.pm = SDHCI_PLTFM_PMOPS,
 	},
 	.probe = bcm2835_sdhci_probe,
-	.remove = bcm2835_sdhci_remove,
+	.remove = sdhci_pltfm_unregister,
 };
 module_platform_driver(bcm2835_sdhci_driver);
 
diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
index a7935a8d0922..59f2923f8054 100644
--- a/drivers/mmc/host/sdhci-cns3xxx.c
+++ b/drivers/mmc/host/sdhci-cns3xxx.c
@@ -98,18 +98,13 @@ static int sdhci_cns3xxx_probe(struct platform_device *pdev)
 	return sdhci_pltfm_register(pdev, &sdhci_cns3xxx_pdata, 0);
 }
 
-static int sdhci_cns3xxx_remove(struct platform_device *pdev)
-{
-	return sdhci_pltfm_unregister(pdev);
-}
-
 static struct platform_driver sdhci_cns3xxx_driver = {
 	.driver		= {
 		.name	= "sdhci-cns3xxx",
 		.pm	= SDHCI_PLTFM_PMOPS,
 	},
 	.probe		= sdhci_cns3xxx_probe,
-	.remove		= sdhci_cns3xxx_remove,
+	.remove		= sdhci_pltfm_unregister,
 };
 
 module_platform_driver(sdhci_cns3xxx_driver);
diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index 2314ff88d942..407c21f152b2 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -108,11 +108,6 @@ err_sdhci_add:
 	return ret;
 }
 
-static int sdhci_dove_remove(struct platform_device *pdev)
-{
-	return sdhci_pltfm_unregister(pdev);
-}
-
 static const struct of_device_id sdhci_dove_of_match_table[] = {
 	{ .compatible = "marvell,dove-sdhci", },
 	{}
@@ -126,7 +121,7 @@ static struct platform_driver sdhci_dove_driver = {
 		.of_match_table = sdhci_dove_of_match_table,
 	},
 	.probe		= sdhci_dove_probe,
-	.remove		= sdhci_dove_remove,
+	.remove		= sdhci_pltfm_unregister,
 };
 
 module_platform_driver(sdhci_dove_driver);
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 17fe02ed6672..22e9111b11ff 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -386,11 +386,6 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int sdhci_esdhc_remove(struct platform_device *pdev)
-{
-	return sdhci_pltfm_unregister(pdev);
-}
-
 static const struct of_device_id sdhci_esdhc_of_match[] = {
 	{ .compatible = "fsl,mpc8379-esdhc" },
 	{ .compatible = "fsl,mpc8536-esdhc" },
@@ -406,7 +401,7 @@ static struct platform_driver sdhci_esdhc_driver = {
 		.pm = ESDHC_PMOPS,
 	},
 	.probe = sdhci_esdhc_probe,
-	.remove = sdhci_esdhc_remove,
+	.remove = sdhci_pltfm_unregister,
 };
 
 module_platform_driver(sdhci_esdhc_driver);
diff --git a/drivers/mmc/host/sdhci-of-hlwd.c b/drivers/mmc/host/sdhci-of-hlwd.c
index be479279a1d5..4079a96ad37e 100644
--- a/drivers/mmc/host/sdhci-of-hlwd.c
+++ b/drivers/mmc/host/sdhci-of-hlwd.c
@@ -75,11 +75,6 @@ static int sdhci_hlwd_probe(struct platform_device *pdev)
 	return sdhci_pltfm_register(pdev, &sdhci_hlwd_pdata, 0);
 }
 
-static int sdhci_hlwd_remove(struct platform_device *pdev)
-{
-	return sdhci_pltfm_unregister(pdev);
-}
-
 static const struct of_device_id sdhci_hlwd_of_match[] = {
 	{ .compatible = "nintendo,hollywood-sdhci" },
 	{ }
@@ -93,7 +88,7 @@ static struct platform_driver sdhci_hlwd_driver = {
 		.pm = SDHCI_PLTFM_PMOPS,
 	},
 	.probe = sdhci_hlwd_probe,
-	.remove = sdhci_hlwd_remove,
+	.remove = sdhci_pltfm_unregister,
 };
 
 module_platform_driver(sdhci_hlwd_driver);
diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
index f7dd68e55b49..606af4b5c312 100644
--- a/drivers/mmc/host/sdhci-sirf.c
+++ b/drivers/mmc/host/sdhci-sirf.c
@@ -193,13 +193,6 @@ err_clk_prepare:
 	return ret;
 }
 
-static int sdhci_sirf_remove(struct platform_device *pdev)
-{
-	struct sdhci_host *host = platform_get_drvdata(pdev);
-
-	return sdhci_pltfm_unregister(pdev);
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int sdhci_sirf_suspend(struct device *dev)
 {
@@ -249,7 +242,7 @@ static struct platform_driver sdhci_sirf_driver = {
 #endif
 	},
 	.probe		= sdhci_sirf_probe,
-	.remove		= sdhci_sirf_remove,
+	.remove		= sdhci_pltfm_unregister,
 };
 
 module_platform_driver(sdhci_sirf_driver);
diff --git a/drivers/mmc/host/sdhci-st.c b/drivers/mmc/host/sdhci-st.c
index 0019df452522..594a4ef3285b 100644
--- a/drivers/mmc/host/sdhci-st.c
+++ b/drivers/mmc/host/sdhci-st.c
@@ -113,11 +113,6 @@ err_of:
 	return ret;
 }
 
-static int sdhci_st_remove(struct platform_device *pdev)
-{
-	return sdhci_pltfm_unregister(pdev);
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int sdhci_st_suspend(struct device *dev)
 {
@@ -155,7 +150,7 @@ MODULE_DEVICE_TABLE(of, st_sdhci_match);
 
 static struct platform_driver sdhci_st_driver = {
 	.probe = sdhci_st_probe,
-	.remove = sdhci_st_remove,
+	.remove = sdhci_pltfm_unregister,
 	.driver = {
 		   .name = "sdhci-st",
 		   .pm = &sdhci_st_pmops,
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 12881e05ff30..2489bb753708 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -321,11 +321,6 @@ err_alloc_tegra_host:
 	return rc;
 }
 
-static int sdhci_tegra_remove(struct platform_device *pdev)
-{
-	return sdhci_pltfm_unregister(pdev);
-}
-
 static struct platform_driver sdhci_tegra_driver = {
 	.driver		= {
 		.name	= "sdhci-tegra",
@@ -333,7 +328,7 @@ static struct platform_driver sdhci_tegra_driver = {
 		.pm	= SDHCI_PLTFM_PMOPS,
 	},
 	.probe		= sdhci_tegra_probe,
-	.remove		= sdhci_tegra_remove,
+	.remove		= sdhci_pltfm_unregister,
 };
 
 module_platform_driver(sdhci_tegra_driver);
-- 
1.9.3

Re: [PATCH v2 0/7] mmc: sdhci: set .remove to sdhci_pltfm_unregister()

[Ulf Hansson]

On 27 February 2015 at 08:47, Kevin Hao <haokexin@gmail.com> wrote:
> Hi,
>
> v2:
>   - Update the sdhci-bcm-kona as suggested by Ray Jui.
>   - Also kill the private get_max_clock function in sdhci-sirf.
>
> v1:
> In the current kernel, the specific .remove function in several sdhci platform
> drivers seem pretty similar. Actually we can just set all of them to
> sdhci_pltfm_unregister() with just a bit twist.
>
> Only run test for sdhci-of-esdhc driver. All others just passed the build test.
>
> Kevin Hao (7):
>   mmc: sdhci-dove: remove the unneeded error check
>   mmc: sdhci-dove: kill the driver specific private struct
>   mmc: tegra: use devm help functions to get the clk and gpio
>   mmc: sdhci-sirf: kill the "clk" member in driver private struct
>   mmc: sdhci-bcm-kona: kill the "external_clk" member in driver private
>        struct
>   mmc: sdhci: disable the clock in sdhci_pltfm_unregister()
>   mmc: sdhci: set the .remove to sdhci_pltfm_unregister()
>
>  drivers/mmc/host/sdhci-bcm-kona.c  | 61 ++++++++------------------------------
>  drivers/mmc/host/sdhci-bcm2835.c   |  7 +----
>  drivers/mmc/host/sdhci-cns3xxx.c   |  7 +----
>  drivers/mmc/host/sdhci-dove.c      | 39 ++++--------------------
>  drivers/mmc/host/sdhci-of-arasan.c |  1 -
>  drivers/mmc/host/sdhci-of-esdhc.c  |  7 +----
>  drivers/mmc/host/sdhci-of-hlwd.c   |  7 +----
>  drivers/mmc/host/sdhci-pltfm.c     |  2 ++
>  drivers/mmc/host/sdhci-sirf.c      | 36 +++++-----------------
>  drivers/mmc/host/sdhci-st.c        | 12 +-------
>  drivers/mmc/host/sdhci-tegra.c     | 30 +++----------------
>  11 files changed, 36 insertions(+), 173 deletions(-)
>
> --
> 1.9.3
>

Applied, thanks!

Kind regards
Uffe