Re: [PATCH 0/2] mmc: core: Remove all code related to MMC_CLKGATE

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

 



>> > Looking into your two examples, sh_mmcif and sh_mobile_sdhci (using
>> > tmio), I realize that they use runtime PM in an awkward way.
>
> I'm not sure what exactly you find awkward there. I know that a lot of
> work has been invested in runtime PM and clock gating on those two
> drivers, and the results were rather good: we were able to runtime suspend
> and wake up controllers according to .power_mode as provided to drivers'
> .set_ios() methods. We were able to use power domains and show, that even
> when the whole domain is powered up and down as a result of our runtime
> PM, we still can communicate with MMC, SD, and SDIO cards in slots. We
> added support for regulators. We also added support for aggressive MMC
> clock gating, which allowed us to save power by gating the clock _when_ it
> was safe, as decided by higher level protocols.

By comparing with other mmc host drivers you see that the runtime PM
implementation differs significantly for sh_mmcif and sh_mobile_sdhci.
I am not saying it is wrong, just question it to try to understand
why. And maybe we can improve something as well.

>
> E.g. look at this comment in tmio_mmc_pio.c:
>
> /*
>  * We differentiate between the following 3 power states:
>  * 1. card slot powered off, controller stopped. This is used, when either there
>  *    is no card in the slot, or the card really has to be powered down.

I guess the "card slot" be interpreted as the core power to the card, VCC/VDD?

>  * 2. card slot powered on, controller stopped. This is used, when a card is in
>  *    the slot, but no activity is currently taking place. This is a power-
>  *    saving mode with card-state preserved. This state can be entered, e.g.
>  *    when MMC clock-gating is used.
>  * 3. card slot powered on, controller running. This is the actual active state.
>  */

Some questions/statements, please shoot them down if possible. :-)

1.
Is the controller involved in providing the actual power to the card
(VCC/VDD)? Looking at the code in .set_ios for sh_mmcif, it seems like
the external regulator (mmc->supply.vmmc) is the one that handles this
on it's own. Especially since power state 3) exists as well.

If my assumption is correct, my best guess is that we shall be able to
rework the runtime PM support as my patchset for sh_mmcif tried to do.
Unless I missed something and there are some other obstacles to
consider from a power domain point of view.

2.
Regarding clock handling. Are there any specific reason to why you
prevent the power domain (though runtime PM) from being dropped while
the clock is enabled (through clk_enable)? That also seems like
unnecessary.

> enum tmio_mmc_power {
>         TMIO_MMC_OFF_STOP,      /* card power off, controller stopped */
>         TMIO_MMC_ON_STOP,       /* card power on, controller stopped */
>         TMIO_MMC_ON_RUN,        /* card power on, controller running */
> };
>
> Now, if you remove clock gating, would you still be able to really support
> all these modes? IIUC, the clock gating is there for higher level MMC/SD
> protocols (MMC, SD, SDHC, SDXC, SDIO,...) to inform the mmc core and
> controller drivers, that according to that protocol the clock can be
> switched off now. It seems to me, that you're just completely removing
> that functionality. Neither the mmc core nor low level controller drivers
> can know when the clock can be switched off. So, I don't see how runtime
> PM by itself can solve the problem. You can choose a different approach to
> signal idle / power-saving states from protocol drivers, but you anyway
> need to actually do that. In your patch "mmc: core: Remove MMC_CLKGATE"
> however, you just remove those calls from sd.c and mmc.c, so, I don't see
> how that functionality can be implemented after your proposed patches.

Simply by using runtime PM autosuspend feature. We use a time-out
value greater then we ever will need, to complete a request according
to the MMC/SD/SDIO spec. 50 ms seems to be an accepted value here.

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