Re: [PATCH] mmc: mmci: Improve runtime PM support

[Ulf Hansson <ulf.hansson@stericsson.com>]

Ulf Hansson wrote:
> Linus Walleij wrote:
>> On Mon, Oct 24, 2011 at 1:48 PM, Ulf Hansson <ulf.hansson@stericsson.com> wrote:
>>> [Russell]
>>>> The MCIPWR signal controls the external power switch.  This is the only
>>>> signal for it.  This is the only connection for it.  There is no other
>>>> control form for this power switch.
>>>>
>>> Then we are only left to use the variant struct I believe. In principle, a
>>> flag in the variant struct, could indicate whether it is OK to disable the
>>> vcore regulator and thus clear the MCIPWR when doing runtime_suspend.
>> Yep I think the best could be to add some variant named
>> bool external_card_power; and then document in the kerneldoc that this
>> means the driver can clear MMCIPWR without risk of cutting the power
>> to the card.
>>
>> This should be true for Ux500, U300 and Nomadik (just checked the designs -
>> they all have external regulators).
>>
> 
> Could we assume that all boards which utilizes the ARM PL180 are using 
> the MMCIPWR register to control power the card? Or should we add a new 
> amba mmci platform member so this is configurable for each board?
> 
> An option could also be if we might want to simplify code to just skip 
> the entire runtime_suspend|idle|resume function (ie stubb it or 
> something) for these kind of boards?
> 
> What do you prefer?

By the way, there is also another option. In the runtime_suspend 
function for ARM PL180 block we can use mmc_power_save_host (and then 
also disable vcore etc) and vice verse in runtime_resume with 
mmc_power_restore_host.

Of course, these kind of operations takes quite some time to execute and 
therefore we use a much bigger timeout than 50 ms (for example 10 s 
instead) for ARM PL180.

Just an idea...

> 
>> So the state save/restore and amba_vcore_disable(adev); should be done
>> only for those variants.
>>
>> However this:
>>
>> clk_disable(host->clk);
>>
>> We ought to be able to do for *all* variants, provided we can create
>> pm_runtime_get/put and delay properly to cover all bus traffic
>> (looks like the patch already does that), plus all the time the card is
>> signalling busy. The best I can think of is to just return -EBUSY to runtime
>> PM like this:
>>
>> if ((readl(base + MMCISTATUS) & (MCI_CMDACTIVE | MCI_TXACTIVE| MCI_RXACTIVE))
>>   return -EBUSY;
>>
>> Yours,
>> Linus Walleij
>>
> 
>