Re: [PATCH/RFC 0/4] convert HS-MMC driver to hwmod + runtime PM

Kevin Hilman
2010-02-24T00:51:15Z

Kevin Hilman <khilman@deeprootsystems.com> writes:

> This series converts the OMAP HS-MMC driver to use omap_hwmod +
> runtime PM API.
>
> Depends on MMC hwmods available in 'pm-wip/hwmods' branch of
> my git tree[1] as well as previously posted runtime PM series:
>
> [PATCH/RFC 0/2] initial runtime PM layer for OMAP
>
> The easies way to experiment/test is to use my 'pm-wip/mmc' branch
> which has all the dependencies, and is based on omap/for-next'.
> It has been tested by merging with current PM branch.
>
> [1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git
>

A question for those of you who actually understand the MMC driver...
I'm having problems getting my head around the current PM stuff in the
MMC driver. My primary question is:

Why does the suspend hook need to re-enable the device before
suspending?

When using runtime PM, the MMC device is disabled including
clocks off and regulator off (if power_savings == true) when there
is no activity.

Then, in the static suspend hook, it's re-enabled (including taking it out of
off, re-enabling regulators etc) only to be quickly disabled again.
This seems horribly inefficient.

I admit to not understanding the MMC layer terribly well, so can someone
enlighten me as to what is going on here?

What I am testing here is a patch on top of this series (below) that
adds a check to the static suspend hook. If the device is already
runtime suspended, then the suspend and resume hooks should be noop.

This appears to work just fine while testing on omap3evm just doing
simple read/write tests before an after suspend resume.

Note that if you want to test this patch, it also depends on this
patch to runtime PM from the linux-pm list:
https://lists.linux-foundation.org/pipermail/linux-pm/2010-February/024275.html

These are all included in an updated version of my pm-wip/mmc branch
for ease of testing. Merge it with the current PM branch, enable
CONFIG_PM_RUNTIME and test away.

Kevin

commit 166cba7679fa267ee6e6eb39fd1e871ede5ded16
Author: Kevin Hilman <khilman@deeprootsystems.com>
Date: Tue Feb 23 16:21:56 2010 -0800

MMC: omap_hsmmc: check for runtime-suspend in static suspend

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 16d66b9..dd027bb 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2222,6 +2222,9 @@ static int omap_hsmmc_suspend(struct device *dev)
if (host && host->suspended)
return 0;

+ if (pm_is_runtime_suspended(host->dev))
+ return 0;
+
if (host) {
host->suspended = 1;
if (host->pdata->suspend) {
@@ -2260,12 +2263,6 @@ static int omap_hsmmc_suspend(struct device *dev)
}
mmc_host_disable(host->mmc);
}
-
- /*
- * HACK: "extra" put to compensate for DPM core keeping
- * runtime PM disabled. -- khilman
- */
- pm_runtime_put_sync(host->dev);
}
return ret;
}
@@ -2280,13 +2277,10 @@ static int omap_hsmmc_resume(struct device *dev)
if (host && !host->suspended)
return 0;

- if (host) {
- /*
- * HACK: "extra" get to compensate for DPM core keeping
- * runtime PM disabled. -- khilman
- */
- pm_runtime_get_sync(host->dev);
+ if (pm_is_runtime_suspended(host->dev))
+ return 0;

+ if (host) {
if (mmc_host_enable(host->mmc) != 0) {
pm_runtime_suspend(host->dev);
goto clk_en_err;