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

[Shawn Lin]

在 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 at 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 at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Best Regards
Shawn Lin