> -----Original Message-----
> From: Linus Walleij [mailto:linus.ml.wall...@gmail.com]
> Sent: Monday, January 24, 2011 11:11 PM
> To: Chris Ball
> Cc: Tardy, Pierre; linux-...@vger.kernel.org; Ohad Ben-Cohen; linux-omap
> Mailing List
> Subject: Re: [PATCH v1 1/3]mmc: implemented runtime pm for mmc host
>
> 2011/1/20 Chris Ball <c...@laptop.org>:
> > On Wed, Jan 19, 2011 at 09:45:57AM +0000, Tardy, Pierre wrote:
> >> Chris,
> >> (Sorry for top posting)
> >> Seems we have a intel intern disagreement.
> >>
> >> Could we have maintainer opinion on this ?
> >
> > Linus W and Ohad, any input here? Thanks,
>
> What I see is orthogonal purposes altogether. Usually there is
> something like two clocks and two regulators or switches
> available in every sufficiently advanced system:
>
> - One regulator to power the card
> - One clock to clock the card (MCLK)
> - One regulator or switch to power the MMC IP block
> - One clock to clock the MMC IP block
>
> The MMC core regulator handling and the clock gating I
> implemented is about the two first things.
>
> I think this patch is about the two *other* things, if I read it
> right.
Actually in sdhci, you power off the MMC IP block, you power OFF the MCLK (at
least on our platform)
I don't know other platform where mmc clock is not controlled more or less
directlty by the sdhc IP.
> Further this patch cannot be applied as-is.
> It needs a clause where it will wait for the clock gating to
> be sync:ed *before* calling the suspend hook.
That's the purpose of those hunks:
iff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8c3769b..27931f6 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -20,6 +20,7 @@
#include <linux/slab.h>
#include <linux/scatterlist.h>
#include <linux/regulator/consumer.h>
+#include <linux/pm_runtime.h>
#include <linux/leds.h>
@@ -1221,6 +1222,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);
@@ -1231,6 +1233,27 @@ 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->mmc->parent);
+ spin_lock_irqsave(&host->lock, flags);
+ } else if (lastclock != 0 && ios->clock == 0) {
+ spin_unlock_irqrestore(&host->lock, flags);
+ pm_runtime_mark_last_busy(host->mmc->parent);
+ pm_runtime_put_autosuspend(host->mmc->parent);
+ spin_lock_irqsave(&host->lock, flags);
+ }
+ /* no need to configure the rest.. */
+ if (host->iosclock == 0)
+ goto out;
+
+ /*
* Reset the chip on each power off.
* Should clear out any weird states.
*/
@@ -1303,6 +1326,8 @@ static int sdhci_get_ro(struct mmc_host *mmc)
I'm wondering if this code could be generic to all drivers, and if clock gating
could not be taking/releasing reference counter on the mmc_bus whenever there
is a clock gating transition?
We could condition that on some MMC_CAP_POWERGATE_WILL_CLKGATE capability flag.
Regards,
Pierre
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
