Re: [PATCH V4] mmc: sdhci-msm: Update system suspend/resume callbacks of sdhci-msm platform driver

On Thu, 19 Mar 2020 at 18:42, Stephen Boyd [off-list ref] wrote:

Quoting Ulf Hansson (2020-03-06 02:07:41)

quoted

On Wed, 4 Mar 2020 at 17:46, Stephen Boyd [off-list ref] wrote:

quoted

Quoting Ulf Hansson (2020-03-04 07:34:29)

quoted

On Thu, 20 Feb 2020 at 07:45, Shaik Sajida Bhanu [off-list ref] wrote:

quoted

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 <redacted>
Reviewed-by: Stephen Boyd <redacted>
---

[...]

quoted

quoted

quoted

quoted

+
+       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