linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] mmc: sdhci-esdhc-imx: add runtime pm support
@ 2013-10-30 14:12 Dong Aisheng
  2013-10-30 14:12 ` [PATCH 1/5] mmc: sdhci-pltfm: export pltfm suspend/resume api Dong Aisheng
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Dong Aisheng @ 2013-10-30 14:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-arm-kernel, cjb, shawn.guo, s.hauer, b29396, wsa

We disable root clock in runtime pm to save power.

Patch 2~4 are some fixes needed for enable runtime pm or it may
break the UHS cards(SD3.0/eMMC4.5).

Below are some tests result i ran which passed mmc_test tool after enable rpm:
SDHC UHS1 on imx6q sabreauto
eMMC 4.5 on imx6sl evk
SDXC UHS1 on imx6sl evk
eMMC DDR on imx6q sabresd
SDHC on imx6q sabresd

Dong Aisheng (5):
  mmc: sdhci-pltfm: export pltfm suspend/resume api
  mmc: sdhci-esdhc-imx: tuning bits should not be cleared during reset
  mmc: sdhci-esdhc-imx: clear SDHCI_CTRL_EXEC_TUNING should not affect
    other bits
  mmc: sdhci-esdhc-imx: fix runtime pm unblance issue
  mmc: sdhci-esdhc-imx: add runtime pm support

 drivers/mmc/host/sdhci-esdhc-imx.c |   85 ++++++++++++++++++++++++++++++------
 drivers/mmc/host/sdhci-pltfm.c     |    6 ++-
 drivers/mmc/host/sdhci-pltfm.h     |    2 +
 3 files changed, 77 insertions(+), 16 deletions(-)

-- 
1.7.2.rc3



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/5] mmc: sdhci-pltfm: export pltfm suspend/resume api
  2013-10-30 14:12 [PATCH 0/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
@ 2013-10-30 14:12 ` Dong Aisheng
  2013-10-30 14:12 ` [PATCH 2/5] mmc: sdhci-esdhc-imx: tuning bits should not be cleared during reset Dong Aisheng
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Dong Aisheng @ 2013-10-30 14:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-arm-kernel, cjb, shawn.guo, s.hauer, b29396, wsa

It is helpful for platforms code to use to elimiate duplicated code.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci-pltfm.c |    6 ++++--
 drivers/mmc/host/sdhci-pltfm.h |    2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index e2065a4..bef250e 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -237,19 +237,21 @@ int sdhci_pltfm_unregister(struct platform_device *pdev)
 EXPORT_SYMBOL_GPL(sdhci_pltfm_unregister);
 
 #ifdef CONFIG_PM
-static int sdhci_pltfm_suspend(struct device *dev)
+int sdhci_pltfm_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 
 	return sdhci_suspend_host(host);
 }
+EXPORT_SYMBOL_GPL(sdhci_pltfm_suspend);
 
-static int sdhci_pltfm_resume(struct device *dev)
+int sdhci_pltfm_resume(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 
 	return sdhci_resume_host(host);
 }
+EXPORT_SYMBOL_GPL(sdhci_pltfm_resume);
 
 const struct dev_pm_ops sdhci_pltfm_pmops = {
 	.suspend	= sdhci_pltfm_suspend,
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index e15ced79..04bc248 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -111,6 +111,8 @@ static inline void *sdhci_pltfm_priv(struct sdhci_pltfm_host *host)
 }
 
 #ifdef CONFIG_PM
+extern int sdhci_pltfm_suspend(struct device *dev);
+extern int sdhci_pltfm_resume(struct device *dev);
 extern const struct dev_pm_ops sdhci_pltfm_pmops;
 #define SDHCI_PLTFM_PMOPS (&sdhci_pltfm_pmops)
 #else
-- 
1.7.2.rc3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/5] mmc: sdhci-esdhc-imx: tuning bits should not be cleared during reset
  2013-10-30 14:12 [PATCH 0/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
  2013-10-30 14:12 ` [PATCH 1/5] mmc: sdhci-pltfm: export pltfm suspend/resume api Dong Aisheng
@ 2013-10-30 14:12 ` Dong Aisheng
  2013-10-30 14:12 ` [PATCH 3/5] mmc: sdhci-esdhc-imx: clear SDHCI_CTRL_EXEC_TUNING should not affect other bits Dong Aisheng
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Dong Aisheng @ 2013-10-30 14:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-arm-kernel, cjb, shawn.guo, s.hauer, b29396, wsa

We should not clear tuning bits during reset or the SD3.0/eMMC4.5 card
working on UHS mode may not work after reset since the former tuning
settings was lost.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 5816585..f5e1dcf 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -45,6 +45,8 @@
 #define  ESDHC_MIX_CTRL_FBCLK_SEL	(1 << 25)
 /* Bits 3 and 6 are not SDHCI standard definitions */
 #define  ESDHC_MIX_CTRL_SDHCI_MASK	0xb7
+/* Tuning bits */
+#define  ESDHC_MIX_CTRL_TUNING_MASK	0x03c00000
 
 /* dll control register */
 #define ESDHC_DLL_CTRL			0x60
@@ -562,7 +564,10 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 		 * Do it manually here.
 		 */
 		if (esdhc_is_usdhc(imx_data)) {
-			writel(0, host->ioaddr + ESDHC_MIX_CTRL);
+			/* the tuning bits should be kept during reset */
+			new_val = readl(host->ioaddr + ESDHC_MIX_CTRL);
+			writel(new_val & ESDHC_MIX_CTRL_TUNING_MASK,
+					host->ioaddr + ESDHC_MIX_CTRL);
 			imx_data->is_ddr = 0;
 		}
 	}
-- 
1.7.2.rc3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/5] mmc: sdhci-esdhc-imx: clear SDHCI_CTRL_EXEC_TUNING should not affect other bits
  2013-10-30 14:12 [PATCH 0/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
  2013-10-30 14:12 ` [PATCH 1/5] mmc: sdhci-pltfm: export pltfm suspend/resume api Dong Aisheng
  2013-10-30 14:12 ` [PATCH 2/5] mmc: sdhci-esdhc-imx: tuning bits should not be cleared during reset Dong Aisheng
@ 2013-10-30 14:12 ` Dong Aisheng
  2013-10-30 14:12 ` [PATCH 4/5] mmc: sdhci-esdhc-imx: fix runtime pm unblance issue Dong Aisheng
  2013-10-30 14:12 ` [PATCH 5/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
  4 siblings, 0 replies; 8+ messages in thread
From: Dong Aisheng @ 2013-10-30 14:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-arm-kernel, cjb, shawn.guo, s.hauer, b29396, wsa

Current code will clear all turning related bits like ESDHC_STD_TUNING_EN
and ESDHC_MIX_CTRL_FBCLK_SEL when clear SDHCI_CTRL_EXEC_TUNING.
This may cause the card which has already passed the turning to become
unwork since the turning status lost.
We observed this failure when enable runtime pm.

BTW, imx needs to enable ESDHC_MIX_CTRL_FBCLK_SEL bit for turned clock.
The FBCLK_SEL will be cleared when SDHCI_CTRL_TUNED_CLK is cleared
and SDHCI_CTRL_EXEC_TUNING is not set.
This is used in case we change to another normal card from a UHS card
in the same slot. FBCLK_SEL is not needed for normal card.

After that, SDHCI_CTRL_EXEC_TUNING will only affect ESDHC_MIX_CTRL_EXE_TUNE.
Clearing it does not affect the turned card to remain working on UHS mode.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index f5e1dcf..0bd99eb 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -439,24 +439,20 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 		} else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
 			u32 v = readl(host->ioaddr + SDHCI_ACMD12_ERR);
 			u32 m = readl(host->ioaddr + ESDHC_MIX_CTRL);
-			new_val = readl(host->ioaddr + ESDHC_TUNING_CTRL);
+			if (val & SDHCI_CTRL_TUNED_CLK) {
+				v |= ESDHC_MIX_CTRL_SMPCLK_SEL;
+			} else {
+				v &= ~ESDHC_MIX_CTRL_SMPCLK_SEL;
+				m &= ~ESDHC_MIX_CTRL_FBCLK_SEL;
+			}
+
 			if (val & SDHCI_CTRL_EXEC_TUNING) {
-				new_val |= ESDHC_STD_TUNING_EN |
-						ESDHC_TUNING_START_TAP;
 				v |= ESDHC_MIX_CTRL_EXE_TUNE;
 				m |= ESDHC_MIX_CTRL_FBCLK_SEL;
 			} else {
-				new_val &= ~ESDHC_STD_TUNING_EN;
 				v &= ~ESDHC_MIX_CTRL_EXE_TUNE;
-				m &= ~ESDHC_MIX_CTRL_FBCLK_SEL;
 			}
 
-			if (val & SDHCI_CTRL_TUNED_CLK)
-				v |= ESDHC_MIX_CTRL_SMPCLK_SEL;
-			else
-				v &= ~ESDHC_MIX_CTRL_SMPCLK_SEL;
-
-			writel(new_val, host->ioaddr + ESDHC_TUNING_CTRL);
 			writel(v, host->ioaddr + SDHCI_ACMD12_ERR);
 			writel(m, host->ioaddr + ESDHC_MIX_CTRL);
 		}
@@ -1038,6 +1034,12 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING)
 		sdhci_esdhc_ops.platform_execute_tuning =
 					esdhc_executing_tuning;
+
+	if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING)
+		writel(readl(host->ioaddr + ESDHC_TUNING_CTRL) |
+			ESDHC_STD_TUNING_EN | ESDHC_TUNING_START_TAP,
+			host->ioaddr + ESDHC_TUNING_CTRL);
+
 	boarddata = &imx_data->boarddata;
 	if (sdhci_esdhc_imx_probe_dt(pdev, boarddata) < 0) {
 		if (!host->mmc->parent->platform_data) {
-- 
1.7.2.rc3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/5] mmc: sdhci-esdhc-imx: fix runtime pm unblance issue
  2013-10-30 14:12 [PATCH 0/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
                   ` (2 preceding siblings ...)
  2013-10-30 14:12 ` [PATCH 3/5] mmc: sdhci-esdhc-imx: clear SDHCI_CTRL_EXEC_TUNING should not affect other bits Dong Aisheng
@ 2013-10-30 14:12 ` Dong Aisheng
  2013-10-30 14:12 ` [PATCH 5/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
  4 siblings, 0 replies; 8+ messages in thread
From: Dong Aisheng @ 2013-10-30 14:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-arm-kernel, cjb, shawn.guo, s.hauer, b29396, wsa

Since we're using common esdhc_send_command for tuning commands and
the core code will call pm_runtime_put after command is finished.
So we add a pm_runtime_get_sync here to get the blance.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 0bd99eb..da8026b 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -698,6 +698,7 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
 	/* FIXME: delay a bit for card to be ready for next tuning due to errors */
 	mdelay(1);
 
+	pm_runtime_get_sync(host->mmc->parent);
 	reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
 	reg |= ESDHC_MIX_CTRL_EXE_TUNE | ESDHC_MIX_CTRL_SMPCLK_SEL |
 			ESDHC_MIX_CTRL_FBCLK_SEL;
-- 
1.7.2.rc3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/5] mmc: sdhci-esdhc-imx: add runtime pm support
  2013-10-30 14:12 [PATCH 0/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
                   ` (3 preceding siblings ...)
  2013-10-30 14:12 ` [PATCH 4/5] mmc: sdhci-esdhc-imx: fix runtime pm unblance issue Dong Aisheng
@ 2013-10-30 14:12 ` Dong Aisheng
  2013-10-30 22:49   ` Ulf Hansson
  4 siblings, 1 reply; 8+ messages in thread
From: Dong Aisheng @ 2013-10-30 14:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-arm-kernel, cjb, shawn.guo, s.hauer, b29396, wsa

The root clock will be disabled in runtime pm which can be used to save power.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   53 ++++++++++++++++++++++++++++++++++-
 1 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index da8026b..60201fd 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -27,6 +27,7 @@
 #include <linux/of_gpio.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_data/mmc-esdhc-imx.h>
+#include <linux/pm_runtime.h>
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
 
@@ -1117,9 +1118,17 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
 	}
 
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_suspend_ignore_children(&pdev->dev, 1);
+
 	err = sdhci_add_host(host);
-	if (err)
+	if (err) {
+		pm_runtime_forbid(&pdev->dev);
+		pm_runtime_get_noresume(&pdev->dev);
 		goto disable_clk;
+	}
 
 	return 0;
 
@@ -1141,6 +1150,9 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
 
 	sdhci_remove_host(host, dead);
 
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
 	clk_disable_unprepare(imx_data->clk_per);
 	clk_disable_unprepare(imx_data->clk_ipg);
 	clk_disable_unprepare(imx_data->clk_ahb);
@@ -1150,12 +1162,49 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_RUNTIME
+static int sdhci_esdhc_runtime_suspend(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+	int ret;
+
+	ret = sdhci_runtime_suspend_host(host);
+
+	clk_disable_unprepare(imx_data->clk_per);
+	clk_disable_unprepare(imx_data->clk_ipg);
+	clk_disable_unprepare(imx_data->clk_ahb);
+
+	return ret;
+}
+
+static int sdhci_esdhc_runtime_resume(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+
+	clk_prepare_enable(imx_data->clk_per);
+	clk_prepare_enable(imx_data->clk_ipg);
+	clk_prepare_enable(imx_data->clk_ahb);
+
+	return sdhci_runtime_resume_host(host);
+}
+#endif
+
+static const struct dev_pm_ops sdhci_esdhc_pmops = {
+	SET_SYSTEM_SLEEP_PM_OPS(sdhci_pltfm_suspend, sdhci_pltfm_resume)
+	SET_RUNTIME_PM_OPS(sdhci_esdhc_runtime_suspend,
+				sdhci_esdhc_runtime_resume, NULL)
+};
+
 static struct platform_driver sdhci_esdhc_imx_driver = {
 	.driver		= {
 		.name	= "sdhci-esdhc-imx",
 		.owner	= THIS_MODULE,
 		.of_match_table = imx_esdhc_dt_ids,
-		.pm	= SDHCI_PLTFM_PMOPS,
+		.pm	= &sdhci_esdhc_pmops,
 	},
 	.id_table	= imx_esdhc_devtype,
 	.probe		= sdhci_esdhc_imx_probe,
-- 
1.7.2.rc3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] mmc: sdhci-esdhc-imx: add runtime pm support
  2013-10-30 14:12 ` [PATCH 5/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
@ 2013-10-30 22:49   ` Ulf Hansson
  2013-11-01 12:31     ` Dong Aisheng
  0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2013-10-30 22:49 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-mmc, linux-arm-kernel@lists.infradead.org, Chris Ball,
	Shawn Guo, Sascha Hauer, wsa

On 30 October 2013 15:12, Dong Aisheng <b29396@freescale.com> wrote:
> The root clock will be disabled in runtime pm which can be used to save power.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |   53 ++++++++++++++++++++++++++++++++++-
>  1 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index da8026b..60201fd 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -27,6 +27,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_data/mmc-esdhc-imx.h>
> +#include <linux/pm_runtime.h>
>  #include "sdhci-pltfm.h"
>  #include "sdhci-esdhc.h"
>
> @@ -1117,9 +1118,17 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>                 host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>         }
>

Add:
pm_runtime_set_active

> +       pm_runtime_enable(&pdev->dev);
> +       pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> +       pm_runtime_use_autosuspend(&pdev->dev);
> +       pm_suspend_ignore_children(&pdev->dev, 1);
> +
>         err = sdhci_add_host(host);
> -       if (err)
> +       if (err) {
> +               pm_runtime_forbid(&pdev->dev);
> +               pm_runtime_get_noresume(&pdev->dev);
>                 goto disable_clk;
> +       }
>
>         return 0;
>
> @@ -1141,6 +1150,9 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
>
>         sdhci_remove_host(host, dead);
>
> +       pm_runtime_dont_use_autosuspend(&pdev->dev);
> +       pm_runtime_disable(&pdev->dev);
> +
>         clk_disable_unprepare(imx_data->clk_per);
>         clk_disable_unprepare(imx_data->clk_ipg);
>         clk_disable_unprepare(imx_data->clk_ahb);
> @@ -1150,12 +1162,49 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +#ifdef CONFIG_PM_RUNTIME
> +static int sdhci_esdhc_runtime_suspend(struct device *dev)
> +{
> +       struct sdhci_host *host = dev_get_drvdata(dev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct pltfm_imx_data *imx_data = pltfm_host->priv;
> +       int ret;
> +
> +       ret = sdhci_runtime_suspend_host(host);
> +
> +       clk_disable_unprepare(imx_data->clk_per);
> +       clk_disable_unprepare(imx_data->clk_ipg);
> +       clk_disable_unprepare(imx_data->clk_ahb);
> +
> +       return ret;
> +}
> +
> +static int sdhci_esdhc_runtime_resume(struct device *dev)
> +{
> +       struct sdhci_host *host = dev_get_drvdata(dev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct pltfm_imx_data *imx_data = pltfm_host->priv;
> +
> +       clk_prepare_enable(imx_data->clk_per);
> +       clk_prepare_enable(imx_data->clk_ipg);
> +       clk_prepare_enable(imx_data->clk_ahb);
> +
> +       return sdhci_runtime_resume_host(host);
> +}
> +#endif
> +
> +static const struct dev_pm_ops sdhci_esdhc_pmops = {
> +       SET_SYSTEM_SLEEP_PM_OPS(sdhci_pltfm_suspend, sdhci_pltfm_resume)
> +       SET_RUNTIME_PM_OPS(sdhci_esdhc_runtime_suspend,
> +                               sdhci_esdhc_runtime_resume, NULL)
> +};
> +
>  static struct platform_driver sdhci_esdhc_imx_driver = {
>         .driver         = {
>                 .name   = "sdhci-esdhc-imx",
>                 .owner  = THIS_MODULE,
>                 .of_match_table = imx_esdhc_dt_ids,
> -               .pm     = SDHCI_PLTFM_PMOPS,
> +               .pm     = &sdhci_esdhc_pmops,
>         },
>         .id_table       = imx_esdhc_devtype,
>         .probe          = sdhci_esdhc_imx_probe,
> --
> 1.7.2.rc3

Apart from the minor issue in the probe function, looks good to me.

Kind regards
Ulf Hansson

>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] mmc: sdhci-esdhc-imx: add runtime pm support
  2013-10-30 22:49   ` Ulf Hansson
@ 2013-11-01 12:31     ` Dong Aisheng
  0 siblings, 0 replies; 8+ messages in thread
From: Dong Aisheng @ 2013-11-01 12:31 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-mmc, linux-arm-kernel@lists.infradead.org, Chris Ball,
	Shawn Guo, Sascha Hauer, wsa

On Wed, Oct 30, 2013 at 11:49:19PM +0100, Ulf Hansson wrote:
> On 30 October 2013 15:12, Dong Aisheng <b29396@freescale.com> wrote:
> > The root clock will be disabled in runtime pm which can be used to save power.
> >
> > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c |   53 ++++++++++++++++++++++++++++++++++-
> >  1 files changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index da8026b..60201fd 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/of_gpio.h>
> >  #include <linux/pinctrl/consumer.h>
> >  #include <linux/platform_data/mmc-esdhc-imx.h>
> > +#include <linux/pm_runtime.h>
> >  #include "sdhci-pltfm.h"
> >  #include "sdhci-esdhc.h"
> >
> > @@ -1117,9 +1118,17 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
> >                 host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> >         }
> >
> 
> Add:
> pm_runtime_set_active
> 

Right.
Thanks for reminder.

BTW, it seems with adding pm_runtime_set_active, i need
put pm_runtime_* related functions right behind sdhci_add_host
since i observed that pm_runtime_set_autosuspend_delay would trigger a suspend
automatically if the device initial state is ACTIVE and during this time,
the host structure is still not initialized which will lead to a crash
when access it in sdhci_runtime_suspend_host.

> > +       pm_runtime_enable(&pdev->dev);
> > +       pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> > +       pm_runtime_use_autosuspend(&pdev->dev);
> > +       pm_suspend_ignore_children(&pdev->dev, 1);
> > +
> >         err = sdhci_add_host(host);
> > -       if (err)
> > +       if (err) {
> > +               pm_runtime_forbid(&pdev->dev);
> > +               pm_runtime_get_noresume(&pdev->dev);
> >                 goto disable_clk;
> > +       }
> >
> >         return 0;
> >
> > @@ -1141,6 +1150,9 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
> >
> >         sdhci_remove_host(host, dead);
> >
> > +       pm_runtime_dont_use_autosuspend(&pdev->dev);
> > +       pm_runtime_disable(&pdev->dev);
> > +
> >         clk_disable_unprepare(imx_data->clk_per);
> >         clk_disable_unprepare(imx_data->clk_ipg);
> >         clk_disable_unprepare(imx_data->clk_ahb);
> > @@ -1150,12 +1162,49 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
> >         return 0;
> >  }
> >
> > +#ifdef CONFIG_PM_RUNTIME
> > +static int sdhci_esdhc_runtime_suspend(struct device *dev)
> > +{
> > +       struct sdhci_host *host = dev_get_drvdata(dev);
> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > +       struct pltfm_imx_data *imx_data = pltfm_host->priv;
> > +       int ret;
> > +
> > +       ret = sdhci_runtime_suspend_host(host);
> > +
> > +       clk_disable_unprepare(imx_data->clk_per);
> > +       clk_disable_unprepare(imx_data->clk_ipg);
> > +       clk_disable_unprepare(imx_data->clk_ahb);
> > +
> > +       return ret;
> > +}
> > +
> > +static int sdhci_esdhc_runtime_resume(struct device *dev)
> > +{
> > +       struct sdhci_host *host = dev_get_drvdata(dev);
> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > +       struct pltfm_imx_data *imx_data = pltfm_host->priv;
> > +
> > +       clk_prepare_enable(imx_data->clk_per);
> > +       clk_prepare_enable(imx_data->clk_ipg);
> > +       clk_prepare_enable(imx_data->clk_ahb);
> > +
> > +       return sdhci_runtime_resume_host(host);
> > +}
> > +#endif
> > +
> > +static const struct dev_pm_ops sdhci_esdhc_pmops = {
> > +       SET_SYSTEM_SLEEP_PM_OPS(sdhci_pltfm_suspend, sdhci_pltfm_resume)
> > +       SET_RUNTIME_PM_OPS(sdhci_esdhc_runtime_suspend,
> > +                               sdhci_esdhc_runtime_resume, NULL)
> > +};
> > +
> >  static struct platform_driver sdhci_esdhc_imx_driver = {
> >         .driver         = {
> >                 .name   = "sdhci-esdhc-imx",
> >                 .owner  = THIS_MODULE,
> >                 .of_match_table = imx_esdhc_dt_ids,
> > -               .pm     = SDHCI_PLTFM_PMOPS,
> > +               .pm     = &sdhci_esdhc_pmops,
> >         },
> >         .id_table       = imx_esdhc_devtype,
> >         .probe          = sdhci_esdhc_imx_probe,
> > --
> > 1.7.2.rc3
> 
> Apart from the minor issue in the probe function, looks good to me.
> 
> Kind regards
> Ulf Hansson
> 

Thanks for the review.
Will update it in v2.

Regards
Dong Aisheng
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-11-01 12:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-30 14:12 [PATCH 0/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
2013-10-30 14:12 ` [PATCH 1/5] mmc: sdhci-pltfm: export pltfm suspend/resume api Dong Aisheng
2013-10-30 14:12 ` [PATCH 2/5] mmc: sdhci-esdhc-imx: tuning bits should not be cleared during reset Dong Aisheng
2013-10-30 14:12 ` [PATCH 3/5] mmc: sdhci-esdhc-imx: clear SDHCI_CTRL_EXEC_TUNING should not affect other bits Dong Aisheng
2013-10-30 14:12 ` [PATCH 4/5] mmc: sdhci-esdhc-imx: fix runtime pm unblance issue Dong Aisheng
2013-10-30 14:12 ` [PATCH 5/5] mmc: sdhci-esdhc-imx: add runtime pm support Dong Aisheng
2013-10-30 22:49   ` Ulf Hansson
2013-11-01 12:31     ` Dong Aisheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).