> -----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. > 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. My concern was more with respect to OFF mode in idle path since your patches removed context restore calls if I recall correctly. 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? Regards, Madhu > 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
