On 12 December 2012 07:53, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > On Tue, Dec 11, 2012 at 3:52 PM, Russell King - ARM Linux > <linux@xxxxxxxxxxxxxxxx> wrote: >> On Tue, Dec 11, 2012 at 03:17:54PM +0100, Ulf Hansson wrote: >>> From: Ulf Hansson <ulf.hansson@xxxxxxxxxx> >>> >>> The amba bus is already performing same actions but for the apb_pclk. >>> So here we just make sure the clock to card is gated as well to save >>> more power. At runtime resume we will thus restore the clock again. >> >> And how exactly do you ensure the requirement that a certain number of >> clocks is supplied to the card after the last command before you cut >> the clock? So, I agree that this needs to be taken care of, but I think it already is, see more comments below. > > Oh, that's gonna be a problem. > > So I can see a quite straight-forward way to do this actually: > > 1) Turn on MMC_CLKGATE for this driver (select from Kconfig) > which means that the ios will be called with f=0 whenever > the card is unused, taking into account the required number > of clocks to the card. I think MMC_CLKGATE was a good initiative in the past. But that was before runtime pm was there to use. Runtime pm suits much better for handling clock gating and other runtime power save actions that could be possible for a host driver to do. I would even suggest the MMC_CLKGATE should be removed from the protocol layer, once we see that all host drivers that used it has switch to runtime pm. > > 2) Implement handling of f=0 from the ios in the ios callback. > Then clk_disable(host->clk). I did a hack at this some months > back but never got around to finish it, sorry for not doing > that even though this clock gating was invented for the MMCI > in the first place :-( > > 3) In the suspend() callback, sleep until the clock indicates > that the card is declocked with something like: > while (clk_is_enabled(host->clk)) { sleep(1) } > However the clk framework does not have clk_is_enabled() > so you'd either have to add that or use a local atomic > variable host_clk_enabled set in (2). > > Should work, I think? I think the solution is much easier. Please consider the below: 1. Some host drivers which implements runtime pm and corresponding runtime callbacks, primarily rely on the the autosuspend timeout. In mmci case, this timeout is set to 50ms, which will be more than enough for any slow frequency to accomplish the 8 clock cycles. I believe sdhci_s3c driver and omap_hsmmc.c does this as well. 2. If we think 1) is not enough for all mmc/sd/sdio operations you could implement ".enable|disable" host function pointers. In those you can do pm_runtime_get|put. Thus the responsibility will be moved to the protocol layer of handling the mmc_claim|release_host properly. It must then keep the host claimed if by some reason the host must be prevented from entering "runtime power save mode". Although, I have not yet found any mmc/sd/sdio operations that seems to require this to be done, you will have to convince me about what circumstances this is need, except for the 8 clock cycles of course. omap_hsmmc.c, does this, I doubt it is needed. > > Yours, > Linus Walleij Kind regards Ulf Hansson -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html