Hi Ulf, On 2018-06-11 11:35, Ulf Hansson wrote: > On 11 June 2018 at 08:48, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: >> dw_mci_exynos_resume_noirq() performs DWMMC register access without >> ensuring that respective clocks are enabled. This might cause external >> abort on some systems (observed on Exynos5433 based boards). Fix this >> by adding needed prepare_enable/disable_unprepare calls. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >> --- >> drivers/mmc/host/dw_mmc-exynos.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c >> index 3164681108ae..6125b68726b0 100644 >> --- a/drivers/mmc/host/dw_mmc-exynos.c >> +++ b/drivers/mmc/host/dw_mmc-exynos.c >> @@ -193,6 +193,9 @@ static int dw_mci_exynos_resume_noirq(struct device *dev) >> struct dw_mci_exynos_priv_data *priv = host->priv; >> u32 clksel; >> >> + clk_prepare_enable(host->biu_clk); >> + clk_prepare_enable(host->ciu_clk); >> + >> if (priv->ctrl_type == DW_MCI_TYPE_EXYNOS7 || >> priv->ctrl_type == DW_MCI_TYPE_EXYNOS7_SMU) >> clksel = mci_readl(host, CLKSEL64); >> @@ -207,6 +210,9 @@ static int dw_mci_exynos_resume_noirq(struct device *dev) >> mci_writel(host, CLKSEL, clksel); >> } >> >> + clk_disable_unprepare(host->biu_clk); >> + clk_disable_unprepare(host->ciu_clk); >> + >> return 0; >> } >> #else > I looked a little closer and I am wondering if it wouldn't be possible > to use SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() instead of > SET_SYSTEM_SLEEP_PM_OPS()? > > Somelike this: > SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > dw_mci_exynos_resume_noirq) > > Then from dw_mci_exynos_resume_noirq() call pm_runtime_force_resume(). > > I think it would simplify the code a bit, as you can rely on the > runtime PM callbacks to deal with clk_prepare_enable() and > clk_disable_unprepare(), unless I am mistaken. This will not fix the problem, because mci_writel() calls in dw_mci_exynos_resume_noirq are done unconditionally, regardless of the controller's runtime pm state. Since commit 1d9174fbc55e after calling pm_runtime_force_resume() there is no guarantee that device is in runtime active state if it was runtime suspended state. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- 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
