On 18 October 2016 at 09:39, Haibo Chen <haibo.chen@xxxxxxx> wrote:
> When suspend usdhc, it will access usdhc register. So usdhc clock
> should be enabled, otherwise the access usdhc register will return
> error or cause system hung.
>
> Take this into consideration, if system enable a usdhc and do not
> connect any SD/SDIO/MMC card, after system boot up, this usdhc
> will do runtime suspend, and close all usdhc clock. At this time,
> if suspend the system, due to no card persent, usdhc runtime resume
> will not be called. So usdhc clock still closed, then in suspend,
> once access usdhc register, system hung or bus error return.
>
> This patch make sure usdhc clock always enabled while doing usdhc
> suspend.
Yes, and since the clocks are kept enabled during system suspend that
means wasting power, doesn't it!?
May I propose another solution. Currently you deal only with clock
gating/ungating during runtime suspend/resume. I am wondering whether
you could extend those operations to be similar to what is needed
during system suspend/resume?
If that is possible, you can instead deploy the runtime PM centric
approach and get system suspend/resume for free. All you would have to
do is to assign the system PM callbacks to
pm_runtime_force_suspend|resume(). In that way, the above problem
would be solved and you don't need to keep the clocks enabled during
system suspend/resume.
Kind regards
Uffe
>
> Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx>
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 7123ef9..1df3846 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1322,17 +1322,28 @@ static int sdhci_esdhc_suspend(struct device *dev)
> {
> struct sdhci_host *host = dev_get_drvdata(dev);
>
> +#ifdef CONFIG_PM
> + pm_runtime_get_sync(host->mmc->parent);
> +#endif
> +
> return sdhci_suspend_host(host);
> }
>
> static int sdhci_esdhc_resume(struct device *dev)
> {
> struct sdhci_host *host = dev_get_drvdata(dev);
> + int ret;
>
> /* re-initialize hw state in case it's lost in low power mode */
> sdhci_esdhc_imx_hwinit(host);
> + ret = sdhci_resume_host(host);
>
> - return sdhci_resume_host(host);
> +#ifdef CONFIG_PM
> + pm_runtime_mark_last_busy(host->mmc->parent);
> + pm_runtime_put_autosuspend(host->mmc->parent);
> +#endif
> +
> + return ret;
> }
> #endif
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
