2011/1/25 Tardy, Pierre <pierre.tardy@xxxxxxxxx>: > 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. This is the case with host/mmci.c. It is taking a clock inside the driver (MCLK), but since it's an AMBA primecell, you can see that when it's probed it also requests and optional IP clock in drivers/amba/bus.c. The ARM reference platforms are wired with two different clocks connected to each of these. There are platforms, such as the U300 that just connect the two into one terminal though. To get a chance to handle the abstraction for both cases these two clocks refer to the same struct clk in the clock tree. >> 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. > */ Aha I get it. So it uses the freq shift from the existing clock gate code to hint rpm, that's actually how I envisioned that this would work for this case. Now I really started liking this patch. Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxxxxxx> > @@ -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. Time will tell I guess, the code in each driver will surely be similar to the above, but I already see some deviations, e.g. some HW will need to delay the actual action taken, some may want to manipulate a register or regulator etc, so I'd leave it open-ended like this. Yours, Linus Walleij -- 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