Re: [PATCH] mmc: mmci: Gate the clock in runtime suspend to save power

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux