[RFC] sdhci: use ios->clock to know when sdhci is idle

[RFC] sdhci: use ios->clock to know when sdhci is idle

[Pierre Tardy]

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 <linus.walleij@stericsson.com>
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 <cjb@laptop.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: Takashi Iwai <tiwai@suse.de>
CC: Maxim Levitsky <maximlevitsky@gmail.com>
CC: Linus Walleij <linus.walleij@stericsson.com>
CC: Ohad Ben-Cohen <ohad@wizery.com>
CC: Yunpeng Gao <yunpeng.gao@intel.com>

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
---
 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

RE: [RFC] sdhci: use ios->clock to know when sdhci is idle

[Yuan, Hang]

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: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-owner@vger.kernel.org]
> On Behalf Of Pierre Tardy
> Sent: Wednesday, December 22, 2010 9:14 PM
> To: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
> 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 <linus.walleij@stericsson.com>
> 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 <cjb@laptop.org>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Alan Cox <alan@linux.intel.com>
> CC: Takashi Iwai <tiwai@suse.de>
> CC: Maxim Levitsky <maximlevitsky@gmail.com>
> CC: Linus Walleij <linus.walleij@stericsson.com>
> CC: Ohad Ben-Cohen <ohad@wizery.com>
> CC: Yunpeng Gao <yunpeng.gao@intel.com>
> 
> Signed-off-by: Pierre Tardy <tardyp@gmail.com>
> ---
>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] sdhci: use ios->clock to know when sdhci is idle

[Pierre Tardy]

On Thu, Dec 23, 2010 at 8:02 AM, Yuan, Hang <hang.yuan@intel.com> 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

RE: [RFC] sdhci: use ios->clock to know when sdhci is idle

[Gao, Yunpeng]

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

Re: [RFC] sdhci: use ios->clock to know when sdhci is idle

[Linus Walleij]

2010/12/29 Gao, Yunpeng <yunpeng.gao@intel.com>:

> 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

RE: [RFC] sdhci: use ios->clock to know when sdhci is idle

[Gao, Yunpeng]

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

Re: [RFC] sdhci: use ios->clock to know when sdhci is idle

[Pierre Tardy]

On Thu, Dec 30, 2010 at 11:37 AM, Gao, Yunpeng <yunpeng.gao@intel.com> wrote:
>>> 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.

I'm also concerned by thrashing. Please see my original patch. I am
using pm_runtime_put_autosuspend() so that suspend timeout can be
tweaked in user space.

Please also note that clock gating only platforms must also be
concerned by thrashing, as clock_disable() call might endup with PLL
power down that will have to be warmed up again later...

>
> 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.
My original patch does not include any ignore_child() call. So that
children can decide wether they want to prevent suspension of their
parent.That will be the case of a wifi card who uses runtime_pm to
manage its own power, and still wants its sdio bus to suspend
automatically when not used (wifi idle)


Regards,
Pierre

Re: [RFC] sdhci: use ios->clock to know when sdhci is idle

[Pierre Tardy]

> Not all SDIO cards will work properly if you try to gate them
> so we need a mechanism to selectively do this.
You are right..##??!! HW vendors that does not support standards...

AFAIK (was working on a sdio HW IP team before), the sdio clock is not
supposed to be taken as is for a system clock derivative especially
because it can stop between request.
SDIO IRQ are asynchronous when a command or data is not ongoing

> Any suggestions?
I would personally suggest white list from drivers. With a call to
mmc_host_allow_gate_card(struct mmc_card *card, bool allow)

see following patch..

Regards,
-- 
Pierre

[PATCH] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Pierre Tardy]

Some sdio card are not following sdio standard, and does not work
when the sdio bus's clock is gated

To keep functionnality for all legacy driver, we turn this quirk on
for every sdio card.
Drivers needs to disable the quirk manually when someone verified that their
supported card works with clock gating.

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
---
 drivers/mmc/core/host.c  |    5 +----
 drivers/mmc/core/sdio.c  |    6 ++++++
 include/linux/mmc/card.h |    1 +
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 92e3370..54cc461 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -160,10 +160,7 @@ static bool mmc_host_may_gate_card(struct mmc_card *card)
 	 * gate the clock, because there is somebody out there that may still
 	 * be using it.
 	 */
-	if (mmc_card_sdio(card))
-		return false;
-
-	return true;
+	return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);
 }
 
 /**
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 82f4b90..6df1ead 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -785,6 +785,12 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 
 	mmc_release_host(host);
 
+        /*
+         * see comments in mmc_host_may_gate_card()
+         * this can be overidden by function drivers if they know that
+         * their sdio card works with clock gating
+         */
+        card->quirks |= MMC_QUIRK_BROKEN_CLK_GATING;
 	/*
 	 * First add the card to the driver model...
 	 */
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 8ce0827..5071eb1 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -121,6 +121,7 @@ struct mmc_card {
 						/* for byte mode */
 #define MMC_QUIRK_NONSTD_SDIO	(1<<2)		/* non-standard SDIO card attached */
 						/* (missing CIA registers) */
+#define MMC_QUIRK_BROKEN_CLK_GATING (1<<3)	/* clock gating the sdio bus will make card fail */
 
 	unsigned int		erase_size;	/* erase size in sectors */
  	unsigned int		erase_shift;	/* if erase unit is power 2 */
-- 
1.7.1

Re: [PATCH] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Philip Rakity]

why not just not compile the the code without clock gating defined ?

and if code cannot run with it clock gating defined do a 
#error ?

Also from the diff I think you patch changes the behavior of SD/eMMC cards which may not be what you want .


On Jan 2, 2011, at 1:08 PM, Pierre Tardy wrote:

> Some sdio card are not following sdio standard, and does not work
> when the sdio bus's clock is gated
> 
> To keep functionnality for all legacy driver, we turn this quirk on
> for every sdio card.
> Drivers needs to disable the quirk manually when someone verified that their
> supported card works with clock gating.
> 
> Signed-off-by: Pierre Tardy <tardyp@gmail.com>
> ---
> drivers/mmc/core/host.c  |    5 +----
> drivers/mmc/core/sdio.c  |    6 ++++++
> include/linux/mmc/card.h |    1 +
> 3 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 92e3370..54cc461 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -160,10 +160,7 @@ static bool mmc_host_may_gate_card(struct mmc_card *card)
> 	 * gate the clock, because there is somebody out there that may still
> 	 * be using it.
> 	 */
> -	if (mmc_card_sdio(card))
> -		return false;
> -
> -	return true;
> +	return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);
> }
> 
> /**
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 82f4b90..6df1ead 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -785,6 +785,12 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
> 
> 	mmc_release_host(host);
> 
> +        /*
> +         * see comments in mmc_host_may_gate_card()
> +         * this can be overidden by function drivers if they know that
> +         * their sdio card works with clock gating
> +         */
> +        card->quirks |= MMC_QUIRK_BROKEN_CLK_GATING;
> 	/*
> 	 * First add the card to the driver model...
> 	 */
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 8ce0827..5071eb1 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -121,6 +121,7 @@ struct mmc_card {
> 						/* for byte mode */
> #define MMC_QUIRK_NONSTD_SDIO	(1<<2)		/* non-standard SDIO card attached */
> 						/* (missing CIA registers) */
> +#define MMC_QUIRK_BROKEN_CLK_GATING (1<<3)	/* clock gating the sdio bus will make card fail */
> 
> 	unsigned int		erase_size;	/* erase size in sectors */
>  	unsigned int		erase_shift;	/* if erase unit is power 2 */
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Pierre Tardy]

On Sun, Jan 2, 2011 at 11:23 PM, Philip Rakity <prakity@marvell.com> wrote:
>
> why not just not compile the the code without clock gating defined ?
>
> and if code cannot run with it clock gating defined do a
> #error ?

The problem here is not related to the drivers that wont compile with
clock gating.
sdio clock gating, is that you gate the sdio bus clock when you dont need it.

Because some vendors sdio card IP is bad, some card are not working
properly when you do this (Their interrupt logic might need a clock to
trigger interrupt, or they are derivating their own internal clock
from the sdio bus clock). Those card are wrong thus I'm using a quirk
to deal with them.

The problem is that we still dont know how many card are wrong and
what are they. So that's why I'm making it default for all sdio card
at the moment.
When we'll have more experience on this, we can switch the behaviour,
and make it a default off.


> Also from the diff I think you patch changes the behavior of SD/eMMC cards which may not be what you want .
The quirk is only set for sdio cards, which is, I think, equivalent to
the previous implementation.

-       if (mmc_card_sdio(card))
-               return false;
-
-       return true;
+       return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);

@@ -785,6 +785,12 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
+        card->quirks |= MMC_QUIRK_BROKEN_CLK_GATING;


This code is made to propose a solution for allowing drivers owner
that *knows* that their card work with CG, to disable the quirk in
their probe function.

Pierre

RE: [RFC] sdhci: use ios->clock to know when sdhci is idle

[Dong, Chuanxiao]

Hi Pierre,
I have some concerns about this RFC.
I agree with you that even if the Wifi interface is up, user might not use the network and host controller should be power gated. That also is the purport of runtime PM framework! But I think Wifi card driver can know whether user is using the network after ifconfig up, right? So that Wifi driver can tell host controller when should be power gated as well. That means you can use runtime PM framework to control Wifi card's parent to do power gating. Then why not use the it?

Another thing is, does the clock gating framework work well when use power gating? Let's consider the scenario there are two slots in one host controller. Clock gating framework can only effect the slot it wants to gate since it only set the register of this slot, not touch the other one. That is reasonable. But power gating is not just effect the slot it wants. It will influence both of the slots since it will set the host controller's power. Host controller never knows how many children are alive since his children never use runtime_put/get to notify their parent. I think this is not reasonable.


> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org
> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Pierre Tardy
> Sent: Wednesday, December 22, 2010 9:14 PM
> To: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
> 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 <linus.walleij@stericsson.com>
> 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 <cjb@laptop.org>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Alan Cox <alan@linux.intel.com>
> CC: Takashi Iwai <tiwai@suse.de>
> CC: Maxim Levitsky <maximlevitsky@gmail.com>
> CC: Linus Walleij <linus.walleij@stericsson.com>
> CC: Ohad Ben-Cohen <ohad@wizery.com>
> CC: Yunpeng Gao <yunpeng.gao@intel.com>
> 
> Signed-off-by: Pierre Tardy <tardyp@gmail.com>
> ---
>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Linus Walleij]

2011/1/2 Pierre Tardy <tardyp@gmail.com>:

> Some sdio card are not following sdio standard, and does not work
> when the sdio bus's clock is gated

Seem to me like it's pretty straight-forward and will work...
Acked-by: Linus Walleij <linus.walleij@stericsson.com>

> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 82f4b90..6df1ead 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -785,6 +785,12 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
>
>        mmc_release_host(host);
>
> +        /*
> +         * see comments in mmc_host_may_gate_card()
> +         * this can be overidden by function drivers if they know that
> +         * their sdio card works with clock gating
> +         */
> +        card->quirks |= MMC_QUIRK_BROKEN_CLK_GATING;

...so the function driver will do something like this:

/* May gate clock */
card->quirks &= ~MMC_QUIRK_BROKEN_CLK_GATING;

Do you have a candidate function driver to do this ...?

Yours,
Linus Walleij

Re: [PATCH] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Ohad Ben-Cohen]

On Wed, Jan 5, 2011 at 1:52 AM, Linus Walleij
<linus.walleij@stericsson.com> wrote:
> Do you have a candidate function driver to do this ...?

wl1271.

We don't use SDIO interrupts at all (the wl1271 has an external irq
line), so we will surely use this.