This allows sdhci to detect its own activity and to autosuspend
when not used
inspired from mmci: handle clock frequency 0 properly
>From Linus Walleij <[email protected]>
author of mmc aggressive clock gating fw.
The idea of using mmc clock gating fw in order to power gate the
sdhci is simple (hope no too simplistic):
Whenever the mmc fw tells we dont need the MMC clock, we dont need
the sdhci power as well.
This does not mean that the child (card) is
suspended. In case of a Wifi SDIO card, the card will be suspended
and resumed according to the ifconfig up/down status.
Even if the Wifi interface is up, user might not use the network.
Sdhci can be powered off during those period. It is up to the HW
implementation to implement smart enough power gating to still
support enough always-on circuitry allowing to detect sdio
interrupts.
This patch goes on top of Yunpeng's patch available on patchwork:
378052 [v2,2/3] mmc: enable runtime PM support of sdhci host controller
CC: Chris Ball <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Alan Cox <[email protected]>
CC: Takashi Iwai <[email protected]>
CC: Maxim Levitsky <[email protected]>
CC: Linus Walleij <[email protected]>
CC: Ohad Ben-Cohen <[email protected]>
CC: Yunpeng Gao <[email protected]>
Signed-off-by: Pierre Tardy <[email protected]>
---
drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++
include/linux/mmc/sdhci.h | 1 +
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a298fb0..a648330 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1161,6 +1161,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct sdhci_host *host;
unsigned long flags;
+ unsigned int lastclock;
u8 ctrl;
host = mmc_priv(mmc);
@@ -1171,6 +1172,24 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
goto out;
/*
+ * get/put runtime_pm usage counter at ios->clock transitions
+ * We need to do it before any other chip access, as sdhci could
+ * be power gated
+ */
+ lastclock = host->iosclock;
+ host->iosclock = ios->clock;
+ if (lastclock == 0 && ios->clock != 0) {
+ spin_unlock_irqrestore(&host->lock, flags);
+ pm_runtime_get_sync(&host->parent.dev);
+ spin_lock_irqsave(&host->lock, flags);
+ } else if (lastclock != 0 && ios->clock == 0) {
+ spin_unlock_irqrestore(&host->lock, flags);
+ pm_runtime_put_autosuspend(&host->parent.dev);
+ spin_lock_irqsave(&host->lock, flags);
+ /* no need to configure the rest.. */
+ goto out;
+ }
+ /*
* Reset the chip on each power off.
* Should clear out any weird states.
*/
@@ -1779,6 +1798,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
host = mmc_priv(mmc);
host->mmc = mmc;
+ host->iosclock = 0;
return host;
}
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 0d953f5..78c1528 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -114,6 +114,7 @@ struct sdhci_host {
unsigned int timeout_clk; /* Timeout freq (KHz) */
unsigned int clock; /* Current clock (MHz) */
+ unsigned int iosclock; /* Last clock asked via set_ios */
u8 pwr; /* Current voltage */
struct mmc_request *mrq; /* Current request */
--
1.7.1
Just have a question why not let sdio card driver call pm_runtime_get/put instead of host
controller driver itself?
This patch may suspend host controller without cooperation of sdio card driver. But suspending
host controller will change controller's registers and then impact sdio card. I think it's
safer to suspend host controller according to sdio card driver's notification following runtime
PM framework.
Another question is why to call pm_runtime_get/put when ios-clock changes? Is it based on
Linus Walleij's aggressive clock gating framework patch? Linus' patch doesn't gate SDIO cards.
Thanks,
Henry
> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> On Behalf Of Pierre Tardy
> Sent: Wednesday, December 22, 2010 9:14 PM
> To: [email protected]; [email protected]
> Cc: Pierre Tardy; Chris Ball; Andrew Morton; Alan Cox; Takashi Iwai; Maxim Levitsky;
> Linus Walleij; Ohad Ben-Cohen; Gao, Yunpeng
> Subject: [RFC] sdhci: use ios->clock to know when sdhci is idle
>
> This allows sdhci to detect its own activity and to autosuspend
> when not used
>
> inspired from mmci: handle clock frequency 0 properly
> From Linus Walleij <[email protected]>
> author of mmc aggressive clock gating fw.
>
> The idea of using mmc clock gating fw in order to power gate the
> sdhci is simple (hope no too simplistic):
> Whenever the mmc fw tells we dont need the MMC clock, we dont need
> the sdhci power as well.
>
> This does not mean that the child (card) is
> suspended. In case of a Wifi SDIO card, the card will be suspended
> and resumed according to the ifconfig up/down status.
> Even if the Wifi interface is up, user might not use the network.
> Sdhci can be powered off during those period. It is up to the HW
> implementation to implement smart enough power gating to still
> support enough always-on circuitry allowing to detect sdio
> interrupts.
>
> This patch goes on top of Yunpeng's patch available on patchwork:
> 378052 [v2,2/3] mmc: enable runtime PM support of sdhci host controller
>
> CC: Chris Ball <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Alan Cox <[email protected]>
> CC: Takashi Iwai <[email protected]>
> CC: Maxim Levitsky <[email protected]>
> CC: Linus Walleij <[email protected]>
> CC: Ohad Ben-Cohen <[email protected]>
> CC: Yunpeng Gao <[email protected]>
>
> Signed-off-by: Pierre Tardy <[email protected]>
> ---
> drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++
> include/linux/mmc/sdhci.h | 1 +
> 2 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index a298fb0..a648330 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1161,6 +1161,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
> {
> struct sdhci_host *host;
> unsigned long flags;
> + unsigned int lastclock;
> u8 ctrl;
>
> host = mmc_priv(mmc);
> @@ -1171,6 +1172,24 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
> goto out;
>
> /*
> + * get/put runtime_pm usage counter at ios->clock transitions
> + * We need to do it before any other chip access, as sdhci could
> + * be power gated
> + */
> + lastclock = host->iosclock;
> + host->iosclock = ios->clock;
> + if (lastclock == 0 && ios->clock != 0) {
> + spin_unlock_irqrestore(&host->lock, flags);
> + pm_runtime_get_sync(&host->parent.dev);
> + spin_lock_irqsave(&host->lock, flags);
> + } else if (lastclock != 0 && ios->clock == 0) {
> + spin_unlock_irqrestore(&host->lock, flags);
> + pm_runtime_put_autosuspend(&host->parent.dev);
> + spin_lock_irqsave(&host->lock, flags);
> + /* no need to configure the rest.. */
> + goto out;
> + }
> + /*
> * Reset the chip on each power off.
> * Should clear out any weird states.
> */
> @@ -1779,6 +1798,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
>
> host = mmc_priv(mmc);
> host->mmc = mmc;
> + host->iosclock = 0;
>
> return host;
> }
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index 0d953f5..78c1528 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -114,6 +114,7 @@ struct sdhci_host {
> unsigned int timeout_clk; /* Timeout freq (KHz) */
>
> unsigned int clock; /* Current clock (MHz) */
> + unsigned int iosclock; /* Last clock asked via set_ios */
> u8 pwr; /* Current voltage */
>
> struct mmc_request *mrq; /* Current request */
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 23, 2010 at 8:02 AM, Yuan, Hang <[email protected]> wrote:
> Just have a question why not let sdio card driver call pm_runtime_get/put instead of host
> controller driver itself?
Because sdio card maintain its own power via runtime_pm, and sdhci
wants to manage its power more independantly, and go suspended more
often than the sdio card. A wifi sdio card would be active when
background scanning for network. During that time, the sdio bus is
completly inactive (for full-mac wifi device), and sdhci can go
suspended during that time.
Lowest SoC platform state can be achieved during that time.
> This patch may suspend host controller without cooperation of sdio card driver. But suspending
> host controller will change controller's registers and then impact sdio card. I think it's
> safer to suspend host controller according to sdio card driver's notification following runtime
> PM framework.
The sdhci must suspend itself without impacting the sdio card.
> Another question is why to call pm_runtime_get/put when ios-clock changes? Is it based on
> Linus Walleij's aggressive clock gating framework patch? Linus' patch doesn't gate SDIO cards.
runtime_suspend of sdhci should *not* gate the sdio card. It should
only gate the sdhci.
An sdio bus inactive does not mean that the sdio card is inactive.
(think wifi Idle, bluetooth idle, ethernet idle)
We need to suspend the sdhci as soon as the sdio bus is inactive,
which is what clock gating framework is trying to detect. It does not
do usage counting via runtime_pm because it is generic enough to allow
clock gating *and* power gating.
Regards,
Pierre
>> Another question is why to call pm_runtime_get/put when ios-clock changes?
>Is it based on
>> Linus Walleij's aggressive clock gating framework patch? Linus' patch doesn't
>gate SDIO cards.
>runtime_suspend of sdhci should *not* gate the sdio card. It should
>only gate the sdhci.
>An sdio bus inactive does not mean that the sdio card is inactive.
>(think wifi Idle, bluetooth idle, ethernet idle)
>
>We need to suspend the sdhci as soon as the sdio bus is inactive,
>which is what clock gating framework is trying to detect.
If the wifi sdio card enters into some low power state which will not use the sdio bus, then the wifi driver can notify the sdhci host controller driver to power down the host controller only by calling the pm_runtime_put() and pm_suspend_ignore_children(). That is, it can be handled well in the current runtime pm framework. So, why we have to move to the 'aggressive clock gating framework'?
Also, I noticed that in the current 'aggressive clock gating framework' patch, the clock gating of SDIO card has been disabled (please reference code and comments of function mmc_host_may_gate_card() in that patch). That means, if discard the runtime PM framework and move to the aggressive clock gating framework, then all the SDIO cards (including the wifi sdio card) will fail to notify its host controller to enter into any low power mode.
Thanks.
Regards,
Yunpeng
2010/12/29 Gao, Yunpeng <[email protected]>:
> So, why we have to move to the 'aggressive clock gating framework'?
The aggressive clock gating makes more sense since the different
drivers will know better how to handle the gating. ios with f=0 can
be interpreted differently. Else every driver has to register
runtime PM hooks for this, which is less elegant.
A better question is why the clock gating does not use runtime
PM abstractions.
The hysteresis (timeout after inactivity, in the MMC spec >= 8 MCLK
cycles) can possibly be handled by refactoring the runtime PM
framework, someone offered to look at this later actually.
> Also, I noticed that in the current 'aggressive clock gating
> framework' patch, the clock gating of SDIO card has been
> disabled (please reference code and comments of function
> mmc_host_may_gate_card() in that patch).
We discussed different approaches to this, from marking an
SDIO slot as suspendable by using platform data, or whitelisting
the SDIO cards that can handle clock gating.
It was decided to keep SDIO cards out of it until we have this
infrastructure in place. So now you will have the opportunity
to fix this!
Not all SDIO cards will work properly if you try to gate them
so we need a mechanism to selectively do this.
Any suggestions?
Yours,
Linus Walleij
>> So, why we have to move to the 'aggressive clock gating framework'?
>
>The aggressive clock gating makes more sense since the different
>drivers will know better how to handle the gating. ios with f=0 can
>be interpreted differently. Else every driver has to register
>runtime PM hooks for this, which is less elegant.
Thanks for the response. I just curious that is this the only reason to change the framework? To my understanding, seems it's not a very strong reason :-)
Take the example of sd/mmc card -
by using the aggressive clock gating framework, it means the host controller will gate (clock gating or power gating) itself if not receiving requests for 8 clocks even if the request queue of mmc block driver is not empty at that time. So the host controller has to be gated / ungated repeatedly until the current request queue of mmc block driver becomes empty. I don't think this is elegant since most of the gating / ungating operations are not necessary.
Instead, if we do it in the mmc block driver by just call the pm_runtime_put() once the current request queue is empty and call pm_runtime_get() once any new request comes, then the host controller can be gated/ungated appropriately.
Thanks.
Regards,
Yunpeng