On Thu, 19 Mar 2020 at 18:42, Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > Quoting Ulf Hansson (2020-03-06 02:07:41) > > 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> > > > > > --- > [...] > > > > > > > > > > > > > > + > > > > > + 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? > > > > I don't know if system wakeup is needed for an eMMC card. Probably only > if you plug in a card and some daemon wants to wake up and probe the > card for auto-play or something like that? Seems possible so might as > well expose the CD gpio as a wakeup in that case and let userspace > decide if it wants to do that. Right, card detect IRQs could be useful for system wakeups. I assume you are using a GPIO IRQ for that, which is easily managed, as the runtime PM status of the mmc controller is irrelevant when configuring the GPIO IRQ as wakeup. We even have a helper for doing this, mmc_gpio_set_cd_wake(). > > Is runtime suspended state the same as system suspended state here > though? The commit text seems to imply that only clks are disabled when > it's desirable to disable the entire controller. I'm still fuzzy on how > runtime PM and system PM interact because it seems to have changed since > I looked last a few years ago. If the driver can stay in a runtime > suspended state across system suspend then I'm all for it. That would > save time for system PM transitions. In most cases this should be possible. And so far, for this case, I haven't found a good reason to why it shouldn't work. Although, perhaps we need to improve some of the sdhci's library functions for PM, to better support this. Kind regards Uffe
