Re: [PATCH 0/3] mmc: Use runtime pm for blkdevice

Ulf Hansson
2013-03-20T21:58:26Z

On 20 March 2013 16:44, Sujit Reddy Thumma <sthumma@codeaurora.org> wrote:
>>>> According the eMMC spec I can't see any requirement that the bus clock
>>>> needs to be on while a BKOPS is running. Moreover it is clearly stated
>>>> it is allowed to gate the bus clock for a device which indicates busy.
>>>> So, I can't see that this is needed.
>>>>
>>>
>>> What if host is aggressive and wants to keep eMMC in sleep-mode and turn
>>> off
>>> VCC regulator during runtime power management? I believe that eMMC card
>>> needs VCC supply as well in addition to VCCQ to carry out BKOPS. Do you
>>> think that the block device also needs to take a reference for VCC
>>> regulator
>>> while BKOPS is started in runtime suspend of block device?
>>
>>
>> What you are thinking of would be exactly the same scenario as doing
>> "mmc_suspend_host" from a host runtime suspend callback, which have
>> been discussed here earlier. Right now, no up-streamed host driver is
>> doing this and I would guess it would never happen either. Anyway,
>> still worth to consider somehow.
>
>
> If any driver wants to implement this then the runtime PM code would be
> refactored again. So I guess we might want to think about this now itself?

Refactored, no.

It is just a new feature that needs to be added, should be a rather
simple patch. Since this kind of code has not been upstreamed for your
host driver I did not consider it in this initial step. Do you want me
to create an additional patch on top of this patchset? I can help out
if you like.

>
> What about SD cards? For SD cards the runtime PM is not doing any advantage
> but instead waste cpu cycles with a timer interrupt and running noop runtime
> PM callbacks? I guess allowing to power off cards in such cases would have
> decent power savings.

We will waste some cpu cycles, true.

Do you think that will have bad impact on performance? In that case
why do we even bother doing runtime PM in host drivers and in many
other places in the kernel? Of course we could optmize the code and
only enable runtime PM if there are a corresponding runtime pm
callbacks implemented in the bus_ops, but is it needed?

>
>
>>
>> Please have a look at below thread to find the answers to your questions:
>> http://thread.gmane.org/gmane.linux.kernel.mmc/19444/focus=19443
>>
>
> Thanks a lot. I have missed this discussion :(
> I have some comments on the possible solutions:
>
> "In mmc bus_ops runtime callback, do a pm_runtime_get_sync("host plf
> device"), and vice verse in the runtime resume callback. This will
> prevent the host driver from entering runtime power save sate while
> for example doing BKOPS, thus preventing your host driver from doing
> mmc_suspend_host while BKOPS is running"
>
> [Sujit] In addition, probably we can allow host to turn off the clocks while
> carrying out BKOPS. But, how can we know whether card is done with BKOPS and
> is idle so that we can call mmc_suspend_host()?

We are then going into details about how to implement IDLE BKOPS,
which is a bit out of scope for this patch, but let me try to comment
anyway.

The initial patch for BKOPS could skip your consideration, and just
check for BKOPS complete once runtime suspend callback gets called.
This will then be rather simple to implement and work for all cases
but yours. I realize that a new blk request will be needed to move out
from BKOPS state then.

The next step could be to schedule a timer/work when issuing BKOPS,
that polls for completion. I belive it should be rather simple to
extend the runtime pm callbacks with this support, right?

>
> "Move mmc_suspend|resume_host from your host runtime callbacks, into
> the bus_ops runtime callbacks. Typically, when no BKOPS is needed
> mmc_suspend_host can be done."
>
> [Sujit] Doesn't it look like we are establishing parent child relationship
> here? If the card has nothing to do, suspend the host?

Well, the naming of these functions are not correct. It is not the
host that actually gets suspended, it is the card.

Right now these functions happens to be called when a host enters
suspend though, which indeed is also kind of strange. It would make
more sense to handle card suspend from the mmc/sdio bus instead; but
let's leave that for a separate discussion. :-)

I also assume that if your host driver runtime pm callbacks calls
mmc_suspend|resume_host, your host driver system suspend|resume
callbacks must not - otherwise you will have races! Instead upper
layers like a power domain, must force your device into runtime
suspend when entering system suspend and vice verse when doing system
resume. These issues exists with or without my patches.

>
>
> --
> Regards,
> Sujit

Thanks a lot for comment, Sujit!

Kind regards
Ulf Hansson