diff mbox

[RFC,v2,1/4] mmc: dw_mmc: add runtime PM callback

Message ID 1475225317-2815-2-git-send-email-shawn.lin@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shawn Lin Sept. 30, 2016, 8:48 a.m. UTC 
This patch add dw_mci_runtime_suspend/resume interfaces
and expose it to dw_mci variant driver to support runtime
PM.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/mmc/host/dw_mmc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++--
 drivers/mmc/host/dw_mmc.h |  4 +++-
 2 files changed, 61 insertions(+), 3 deletions(-)

Comments

Jaehoon Chung Oct. 7, 2016, 4:12 a.m. UTC | #1 
Hi Shawn,

On 09/30/2016 05:48 PM, Shawn Lin wrote:
> This patch add dw_mci_runtime_suspend/resume interfaces
> and expose it to dw_mci variant driver to support runtime
> PM.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> ---
> 
>  drivers/mmc/host/dw_mmc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/mmc/host/dw_mmc.h |  4 +++-
>  2 files changed, 61 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4fcbc40..54b860e 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -3266,7 +3266,7 @@ EXPORT_SYMBOL(dw_mci_remove);
>  
>  
>  
> -#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_PM
>  /*
>   * TODO: we should probably disable the clock to the card in the suspend path.
>   */
> @@ -3324,7 +3324,63 @@ int dw_mci_resume(struct dw_mci *host)
>  	return 0;
>  }
>  EXPORT_SYMBOL(dw_mci_resume);
> -#endif /* CONFIG_PM_SLEEP */
> +
> +int dw_mci_runtime_suspend(struct dw_mci *host)
> +{
> +	printk("dw_mci_runtime_suspend\n");

Maybe I think you will remove this..if you send the patch, not RFC.

> +
> +	if (host->use_dma && host->dma_ops->exit)
> +		host->dma_ops->exit(host);

Can just call dw_mci_suspend()

> +
> +	clk_disable_unprepare(host->ciu_clk);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(dw_mci_runtime_suspend);
> +
> +int dw_mci_runtime_resume(struct dw_mci *host)
> +{
> +	int ret = 0;
> +	int i;

Can make one line.

> +
> +	printk("dw_mci_runtime_resume\n");
> +
> +	ret = clk_prepare_enable(host->ciu_clk);
> +	if (ret)
> +		return ret;
> +
> +	if (host->use_dma && host->dma_ops->init)
> +		host->dma_ops->init(host);
> +
> +	mci_writel(host, FIFOTH, host->fifoth_val);
> +	host->prev_blksz = 0;
> +
> +	mci_writel(host, TMOUT, 0xFFFFFFFF);
> +	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> +	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE |
> +				  SDMMC_INT_DATA_OVER |
> +				  SDMMC_INT_TXDR | SDMMC_INT_RXDR |
> +				  DW_MCI_ERROR_FLAGS);
> +	mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
> +
> +	for (i = 0; i < host->num_slots; i++) {
> +		struct dw_mci_slot *slot = host->slot[i];
> +
> +		if (!slot)
> +			continue;
> +
> +		if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
> +			dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
> +			dw_mci_setup_bus(slot, true);
> +		}
> +	}
> +
> +	dw_mci_enable_cd(host);

Some part of this function can be reused with codes in dw_mci_resume().

Best Regards,
Jaehoon Chung

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(dw_mci_runtime_resume);
> +#endif /* CONFIG_PM */
>  
>  static int __init dw_mci_init(void)
>  {
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index e8cd2de..baa7261 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -234,9 +234,11 @@
>  
>  extern int dw_mci_probe(struct dw_mci *host);
>  extern void dw_mci_remove(struct dw_mci *host);
> -#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_PM
>  extern int dw_mci_suspend(struct dw_mci *host);
>  extern int dw_mci_resume(struct dw_mci *host);
> +extern int dw_mci_runtime_suspend(struct dw_mci *host);
> +extern int dw_mci_runtime_resume(struct dw_mci *host);
>  #endif
>  
>  /**
> 

--
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
Shawn Lin Oct. 7, 2016, 9:19 a.m. UTC | #2 
在 2016/10/7 12:12, Jaehoon Chung 写道:
> Hi Shawn,
>
> On 09/30/2016 05:48 PM, Shawn Lin wrote:
>> This patch add dw_mci_runtime_suspend/resume interfaces
>> and expose it to dw_mci variant driver to support runtime
>> PM.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>>  drivers/mmc/host/dw_mmc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++--
>>  drivers/mmc/host/dw_mmc.h |  4 +++-
>>  2 files changed, 61 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 4fcbc40..54b860e 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -3266,7 +3266,7 @@ EXPORT_SYMBOL(dw_mci_remove);
>>
>>
>>
>> -#ifdef CONFIG_PM_SLEEP
>> +#ifdef CONFIG_PM
>>  /*
>>   * TODO: we should probably disable the clock to the card in the suspend path.
>>   */
>> @@ -3324,7 +3324,63 @@ int dw_mci_resume(struct dw_mci *host)
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL(dw_mci_resume);
>> -#endif /* CONFIG_PM_SLEEP */
>> +
>> +int dw_mci_runtime_suspend(struct dw_mci *host)
>> +{
>> +	printk("dw_mci_runtime_suspend\n");
>
> Maybe I think you will remove this..if you send the patch, not RFC.

yep, it just for me to print info for testing the rpm governer.
I will remove this when sending non-RFC one.

>
>> +
>> +	if (host->use_dma && host->dma_ops->exit)
>> +		host->dma_ops->exit(host);
>
> Can just call dw_mci_suspend()

If we deploy rpm properly, we don't need system PM at all..

To be frank, I was wanna remove dw_mci_suspend/resume pairs,
and only leave the runtime pairs here. As you could see that
runtime PM and system PM does the same thing, so personally
I realize we should help dw_mmc variant drivers migrate to use
runtime PM. That is what I did for dw_mmc-rockchip. If we are
not sure should we enable rpm for i.e., exynos, or k3, we don't
add pm_runtime_* for them for the first step and leave it to
the owners/users to make the decision. The following code should
be enough to help we achieve the first step.

static const struct dev_pm_ops dw_mci_XXXXXX_dev_pm_ops = {
	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
				pm_runtime_force_resume)
	SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
			   dw_mci_runtime_resume,
			   NULL)
};

If there are some special ops for system PM like exynos, we could wrap
new dw_mci_exynos_runtime_resume there..

The same for dw_mci_resume.

What's your opinion?  Seems Ulf was too busy to make comments
here. But I believe it is in the right direction.


>
>> +
>> +	clk_disable_unprepare(host->ciu_clk);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(dw_mci_runtime_suspend);
>> +
>> +int dw_mci_runtime_resume(struct dw_mci *host)
>> +{
>> +	int ret = 0;
>> +	int i;
>
> Can make one line.
>
>> +
>> +	printk("dw_mci_runtime_resume\n");
>> +
>> +	ret = clk_prepare_enable(host->ciu_clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (host->use_dma && host->dma_ops->init)
>> +		host->dma_ops->init(host);
>> +
>> +	mci_writel(host, FIFOTH, host->fifoth_val);
>> +	host->prev_blksz = 0;
>> +
>> +	mci_writel(host, TMOUT, 0xFFFFFFFF);
>> +	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>> +	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE |
>> +				  SDMMC_INT_DATA_OVER |
>> +				  SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>> +				  DW_MCI_ERROR_FLAGS);
>> +	mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
>> +
>> +	for (i = 0; i < host->num_slots; i++) {
>> +		struct dw_mci_slot *slot = host->slot[i];
>> +
>> +		if (!slot)
>> +			continue;
>> +
>> +		if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
>> +			dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
>> +			dw_mci_setup_bus(slot, true);
>> +		}
>> +	}
>> +
>> +	dw_mci_enable_cd(host);
>
> Some part of this function can be reused with codes in dw_mci_resume().
>
> Best Regards,
> Jaehoon Chung
>
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(dw_mci_runtime_resume);
>> +#endif /* CONFIG_PM */
>>
>>  static int __init dw_mci_init(void)
>>  {
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index e8cd2de..baa7261 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -234,9 +234,11 @@
>>
>>  extern int dw_mci_probe(struct dw_mci *host);
>>  extern void dw_mci_remove(struct dw_mci *host);
>> -#ifdef CONFIG_PM_SLEEP
>> +#ifdef CONFIG_PM
>>  extern int dw_mci_suspend(struct dw_mci *host);
>>  extern int dw_mci_resume(struct dw_mci *host);
>> +extern int dw_mci_runtime_suspend(struct dw_mci *host);
>> +extern int dw_mci_runtime_resume(struct dw_mci *host);
>>  #endif
>>
>>  /**
>>
>
> --
> 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
>
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4fcbc40..54b860e 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3266,7 +3266,7 @@  EXPORT_SYMBOL(dw_mci_remove);
 
 
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
 /*
  * TODO: we should probably disable the clock to the card in the suspend path.
  */
@@ -3324,7 +3324,63 @@  int dw_mci_resume(struct dw_mci *host)
 	return 0;
 }
 EXPORT_SYMBOL(dw_mci_resume);
-#endif /* CONFIG_PM_SLEEP */
+
+int dw_mci_runtime_suspend(struct dw_mci *host)
+{
+	printk("dw_mci_runtime_suspend\n");
+
+	if (host->use_dma && host->dma_ops->exit)
+		host->dma_ops->exit(host);
+
+	clk_disable_unprepare(host->ciu_clk);
+
+	return 0;
+}
+EXPORT_SYMBOL(dw_mci_runtime_suspend);
+
+int dw_mci_runtime_resume(struct dw_mci *host)
+{
+	int ret = 0;
+	int i;
+
+	printk("dw_mci_runtime_resume\n");
+
+	ret = clk_prepare_enable(host->ciu_clk);
+	if (ret)
+		return ret;
+
+	if (host->use_dma && host->dma_ops->init)
+		host->dma_ops->init(host);
+
+	mci_writel(host, FIFOTH, host->fifoth_val);
+	host->prev_blksz = 0;
+
+	mci_writel(host, TMOUT, 0xFFFFFFFF);
+	mci_writel(host, RINTSTS, 0xFFFFFFFF);
+	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE |
+				  SDMMC_INT_DATA_OVER |
+				  SDMMC_INT_TXDR | SDMMC_INT_RXDR |
+				  DW_MCI_ERROR_FLAGS);
+	mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
+
+	for (i = 0; i < host->num_slots; i++) {
+		struct dw_mci_slot *slot = host->slot[i];
+
+		if (!slot)
+			continue;
+
+		if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
+			dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
+			dw_mci_setup_bus(slot, true);
+		}
+	}
+
+	dw_mci_enable_cd(host);
+
+	return ret;
+}
+EXPORT_SYMBOL(dw_mci_runtime_resume);
+#endif /* CONFIG_PM */
 
 static int __init dw_mci_init(void)
 {
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index e8cd2de..baa7261 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -234,9 +234,11 @@ 
 
 extern int dw_mci_probe(struct dw_mci *host);
 extern void dw_mci_remove(struct dw_mci *host);
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
 extern int dw_mci_suspend(struct dw_mci *host);
 extern int dw_mci_resume(struct dw_mci *host);
+extern int dw_mci_runtime_suspend(struct dw_mci *host);
+extern int dw_mci_runtime_resume(struct dw_mci *host);
 #endif
 
 /**