On Mon, 22 Mar 2021 at 10:13, <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 both enable CONFIG_PM and CONFIG_PM_SLEEP, which
> means both support PM and Runtime PM. If the card slot do not insert
> a card, then after system boot up, will do sdhci_esdhc_runtime_suspend(),
> disable all clocks, include the ipg clock. In this case, when suspend the
> system, due to no card present, 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>
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 30 ++++++++++++++++++++++++------
> 1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 94327988da91..a48b977ca13f 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1695,10 +1695,14 @@ static int sdhci_esdhc_suspend(struct device *dev)
> struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> int ret;
>
> + ret = clk_prepare_enable(imx_data->clk_ipg);
> + if (ret)
> + return ret;
> +
This isn't sufficient.
If the device is attached to a PM domain, that needs to be powered on
too. In other words, you need to call pm_runtime_get_sync() instead of
clk_prepare_enable().
That said, it's kind of surprising to me, that the clocks may remain
ungated during system suspend, but are gated in runtime suspend.
Perhaps what you really should look into, is to re-organize the code
and then can call pm_runtime_force_suspend() from
sdhci_esdhc_suspend() (and pm_runtime_force_resume() from
sdhci_esdhc_suspend()). I think that will be both easier and likely
helps to avoid wasting energy as well.
> if (host->mmc->caps2 & MMC_CAP2_CQE) {
> ret = cqhci_suspend(host->mmc);
> if (ret)
> - return ret;
> + goto disable_ipg_clk;
> }
>
> if ((imx_data->socdata->flags & ESDHC_FLAG_STATE_LOST_IN_LPMODE) &&
> @@ -1712,38 +1716,52 @@ static int sdhci_esdhc_suspend(struct device *dev)
>
> ret = sdhci_suspend_host(host);
> if (ret)
> - return ret;
> + goto disable_ipg_clk;
>
> ret = pinctrl_pm_select_sleep_state(dev);
> if (ret)
> - return ret;
> + goto disable_ipg_clk;
>
> ret = mmc_gpio_set_cd_wake(host->mmc, true);
>
> +disable_ipg_clk:
> + clk_disable_unprepare(imx_data->clk_ipg);
> +
> return ret;
> }
[...]
Kind regards
Uffe
