Kevin Hilman wrote:
"Madhusudhan" <madhu.cr@xxxxxx> writes:
-----Original Message-----
From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
owner@xxxxxxxxxxxxxxx] On Behalf Of Kevin Hilman
Sent: Tuesday, February 23, 2010 6:51 PM
To: linux-omap@xxxxxxxxxxxxxxx
Subject: Re: [PATCH/RFC 0/4] convert HS-MMC driver to hwmod + runtime PM
Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx> 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?
In the scenario where there is no activity on the bus the MMC clocks are
kept disabled. Now in the suspend path the MMC core will issue certain CMDs
like CMD7(to end the suspend path) to deselect the card(more of a protocol
stuff). Hence the host need to be in enabled state before letting the core
know that there is a suspend request.
I guess what I'm wondering is whether we can (should) have the exact
same path for runtime PM (idle) and static PM (suspend/resume.)
My current approach is that the suspend is a NOP if the device
is already runtime suspended.
One difference is that the card-detect irq is disabled during suspend.
That means that removeable cards must have their block devices removed
too (or not if "unsafe-resume" is configured). The advantage of OFF-mode
is that it will wake-up if someone opens the cover, whereas suspend
will not.
A similar issue may develop with SDIO which may want to wake-up the
device from OFF-mode (e.g. wireless card connected via SDIO)
So is the difference that OFF-mode has wakeups but suspend does not?
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.
This is exactly for the reason I mentioned above.
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.
I did some basic testing with your previously posted patches. But my testing
was incomplete because on Zoom2 because for some reason the OFF mode was not
working even without your patches.
OK, I will try on Zoom2. I've only tested this so far on OMAP3EVM.
My concern was more with respect to OFF mode in idle path since your patches
removed context restore calls if I recall correctly.
No, I didn't remove restore. I just moved it into the runtime PM
resume callback.
Are you able to hit CORE OFF and then come back and do the
read/write transfers in idle as well as suspend/resume path?
Yes.
Kevin
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@xxxxxxxxxxxxxxxxxxx>
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;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
