Re: [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice

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

 



On 11/04/13 13:14, Ulf Hansson wrote:
> On 11 April 2013 11:58, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>> On 11/04/13 12:40, Ulf Hansson wrote:
>>> On 11 April 2013 10:31, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>>> On 08/04/13 14:44, Ulf Hansson wrote:
>>>>> From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>>>>>
>>>>> Once the mmc blkdevice is being probed, runtime pm will be enabled.
>>>>> By using runtime autosuspend, the power save operations can be done
>>>>> when request inactivity occurs for a certain time. Right now the
>>>>> selected timeout value is set to 3 s.
>>>>>
>>>>> Moreover, when the blk device is being suspended, we make sure the device
>>>>> will be runtime resumed. The reason for doing this is that we want the
>>>>> host suspend sequence to be unaware of any runtime power save operations,
>>>>> so it can just handle the suspend as the device is fully powered from a
>>>>> runtime perspective.
>>>>>
>>>>> This patch is preparing to make it possible to move BKOPS handling into
>>>>> the runtime callbacks for the mmc bus_ops. Thus IDLE BKOPS can be
>>>>> accomplished.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>>>>> Acked-by: Maya Erez <merez@xxxxxxxxxxxxxx>
>>>>> Acked-by: Arnd Bergmann <arnd@xxxxxxxx>
>>>>> Acked-by: Kevin Liu <kliu5@xxxxxxxxxxx>
>>>>> ---
>>>>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>>>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>>>>
>>>>
>>>> There are debugfs uses of the card also (e.g.mmc_dbg_card_status_get)
>>>> that will need get/put added.
>>>
>>> In the end it all depends on what kind of operations you decide to do
>>> in the runtime_supend|resume callbacks.
>>> Since the callbacks is not yet implemented for sd and mmc, this is not
>>> required as of now.
>>>
>>> Nevertheless a most valid point that needs to be considered, while
>>> implementing the callbacks. Thanks for pointing this out.
>>>
>>>>
>>>> There might be others.  Please check.
>>>
>>> mmc_rescan needs to be considered at card removal and at resume. But,
>>> again this does not need to be handled as of now.
>>
>> I disagree.  If you are adding runtime PM for SD/MMC cards, it must not be
>> possible to access the card without going through runtime PM first.  That
>> includes *all* code paths. We should not leave holes for others to fall in.
>>
> 
> This patchset shall not be considered as full blown common solution
> for runtime pm for mmc/sd/sdio. It is an initial step that we can
> start build upon.

That does not justify leaving holes.

> I took the approach of only adding, pm_runtime_get|put from the mmc
> block layer, thus it will also _not_ affect the SDIO pm_runtime
> implementation, which is already being used today. Adding what you
> propose will affect SDIO as well, I think it is better to handle this
> in the next steps instead.

I disagree.  SDIO is easily avoided by coding for card->type.

Fix debugfs handling.

Don't enable runtime pm for removable cards.  Document why i.e. rescan of
removable cards needs special handling depending on the power-saving
strategy e.g. if the power is off there is a risk of confusing a new card
with an old card.

That covers two omissions that we know about.  Check for others -
essentially check all mmc_claim_host() / __mmc_claim_host() /
mmc_try_claim_host() calls.

SD-Combo cards also look like a problem, so don't enable runtime pm for them
either.

Document what works (i.e. non-removable SD and MMC cards) and what doesn't
(removable and SD-Combo cards) and why.


--
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