On Thu, 18 May 2023 at 10:17, <haibo.chen@xxxxxxx> wrote:
>
> From: Haibo Chen <haibo.chen@xxxxxxx>
>
> During suspend/resume, need to access usdhc register, so need
> to enable IPG clock to avoid bus error.
>
> Find this issue when a card slot do not insert a card, so when
> system suspend, sdhci_esdhc_runtime_resume() will not be called
> before sdhci_esdhc_suspend(), so will meet system hung or bus err
> once access usdhc register.
>
> Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx>
> ---
> The first thing I do is try to merge the sdhci_esdhc_suspend() into
> current sdhci_esdhc_runtime_suspend(), but find some obstacles:
> 1, sdhci_esdhc_imx_hwinit() is called in original sdhci_esdhc_resume(),
> it will clear the DLL config, which after remove to sdhci_esdhc_runtime_resume,
> will finally impact the DDR50 timing.
> 2, if merge, everytime after sdhci_esdhc_runtime_resume, will do re-tuning,
> this is unnecessary for the normal runtime PM case without power lost.
> 3, the CD gpio wakeup. Seems strange to enable the CD gpio wakeup in
> sdhci_esdhc_runtime_suspend().
>
> So, finally I drop this merge method, and directly add change in the original
> sdhci_esdhc_suspend()/sdhci_esdhc_resume().
Looks like you need some differentiation between runtime
suspend/resume and system suspend/resume. One way to deal with this is
to set some driver flags during system suspend and then monitor those
flags at runtime resume - and then act accordingly.
There are a couple of sdhci drivers that do this, like sdhci-xenon.c
and sdhci-of-at91.c. Those drivers don't seems to need the
sdhci_suspend|resume_host(). Maybe that pattern can help to fix this
too?
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index eebf94604a7f..4cf42a028bb9 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1836,6 +1836,8 @@ static int sdhci_esdhc_suspend(struct device *dev)
> struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> int ret;
>
> + pm_runtime_get_sync(dev);
> +
> if (host->mmc->caps2 & MMC_CAP2_CQE) {
> ret = cqhci_suspend(host->mmc);
> if (ret)
> @@ -1855,6 +1857,8 @@ static int sdhci_esdhc_suspend(struct device *dev)
> if (ret)
> return ret;
>
> + pm_runtime_force_suspend(dev);
> +
> ret = pinctrl_pm_select_sleep_state(dev);
> if (ret)
> return ret;
> @@ -1873,6 +1877,8 @@ static int sdhci_esdhc_resume(struct device *dev)
> if (ret)
> return ret;
>
> + pm_runtime_force_resume(dev);
> +
> /* re-initialize hw state in case it's lost in low power mode */
> sdhci_esdhc_imx_hwinit(host);
>
> @@ -1886,6 +1892,8 @@ static int sdhci_esdhc_resume(struct device *dev)
> if (!ret)
> ret = mmc_gpio_set_cd_wake(host->mmc, false);
>
> + pm_runtime_put_autosuspend(dev);
> +
> return ret;
> }
> #endif
> --
> 2.34.1
>
