On Wed, 4 Mar 2020 at 17:46, Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > Quoting Ulf Hansson (2020-03-04 07:34:29) > > On Thu, 20 Feb 2020 at 07:45, Shaik Sajida Bhanu <sbhanu@xxxxxxxxxxxxxx> wrote: > > > > > > The existing suspend/resume callbacks of sdhci-msm driver are just > > > gating/un-gating the clocks. During suspend cycle more can be done > > > like disabling controller, disabling card detection, enabling wake-up events. > > > > > > So updating the system pm callbacks for performing these extra > > > actions besides controlling the clocks. > > > > > > Signed-off-by: Shaik Sajida Bhanu <sbhanu@xxxxxxxxxxxxxx> > > > Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx> > > > --- > > > Changes since V3: > > > Invoking sdhci & cqhci resume if sdhci_host_suspend fails. > > > Removed condition check before invoking cqhci_resume since its a dummy function. > > > > > > Changes since V2: > > > Removed disabling/enabling pwr-irq from system pm ops. > > > > > > Changes since V1: > > > Invoking pm_runtime_force_suspend/resume instead of > > > sdhci_msm_runtime_suepend/resume. > > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > > > index 3955fa5d..3559b50 100644 > > > --- a/drivers/mmc/host/sdhci-msm.c > > > +++ b/drivers/mmc/host/sdhci-msm.c > > > @@ -2159,9 +2159,52 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) > [...] > > > + > > > + ret = sdhci_suspend_host(host); > > > + if (ret) > > > + goto resume_cqhci; > > > > sdhci_suspend_host() can't be called on a device that has been runtime > > suspended, as that would lead to accessing device registers when > > clocks/PM domains are gated. > > > > Depending on how the corresponding cqhci device is managed from a > > runtime PM point of view, it could also be problematic to call > > cqhci_suspend(). > > There seems to be another patch floating around here[1] that is an > attempt at a fix to this patch. They should probably be combined so that > it's not confusing what's going on. Yeah, it would be easier if these are discussed together. > > > > > > + > > > + ret = pm_runtime_force_suspend(dev); > > > > It looks to me that perhaps you could make use of solely > > pm_runtime_force_suspend(), then just skip calling > > sdhci_suspend|resume_host() altogether. Do you think that could work? > > Does that do all the things the commit text mentions is desired for > system suspend? No. :-) But why is system wakeup needed for an eMMC card? > > > > like disabling controller, disabling card detection, enabling wake-up events. > > [1] https://lore.kernel.org/linux-arm-msm/1583322863-21790-1-git-send-email-vbadigan@xxxxxxxxxxxxxx/ Kind regards Uffe
