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

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

[Kevin Liu]

> -----Original Message-----
> From: Chris Ball [mailto:cjb@laptop.org]
> Sent: Wednesday, March 27, 2013 9:32 PM
> To: Ulf Hansson
> Cc: linux-mmc@vger.kernel.org; Johan Rudholm; Ulf Hansson; Maya Erez; Kevin Liu; Seungwon Jeon; jh80.chung@samsung.com
> Subject: Re: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>
> Hi Maya, Kevin, Seungwon, Jaehoon,
>
> On Fri, Mar 01 2013, Ulf Hansson wrote:
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> SDIO has been using runtime pm for a while to handle runtime power save
>> operations. This patchset is enabling the option to make the sd/mmc
>> blockdevices to use runtime pm as well.
>>
>> The runtime pm implementation for the block device will make use of
>> autosuspend to defer power save operation to after request inactivty for
>> a certain time.
>>
>> To actually perform some power save operations the corresponding bus ops
>> for mmc and sd shall be implemented. Typically it could make sense to do
>> BKOPS for eMMC in here.
>>
>> Ulf Hansson (3):
>>   mmc: core: Remove power_restore bus_ops for mmc and sd
>>   mmc: core: Add bus_ops for runtime pm callbacks
>>   mmc: block: Enable runtime pm for mmc blkdevice
>>
>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>  drivers/mmc/core/bus.c   |   14 ++++++++++++--
>>  drivers/mmc/core/core.h  |    2 ++
>>  drivers/mmc/core/mmc.c   |   14 --------------
>>  drivers/mmc/core/sd.c    |   14 --------------
>>  drivers/mmc/core/sdio.c  |   20 ++++++++++++++++++++
>>  6 files changed, 60 insertions(+), 32 deletions(-)
>
> This looks good to me, but I'd like to make sure the design works for
> you before we decide to merge it.  Any concerns?
>

Chris,

This patchset is ok for me.
We can discuss/review how sd/mmc runtime callback function implemented later.

Thanks
Kevin

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

[merez]

Hi Chris,

Sorry for the late response.
I am currently on vacation and would get back to work next week.
We would like to have an internal discussion to make sure there are no
additional concerns with this patch.
Can you please wait with this patch approval until the end of next week?

Thanks,
Maya

>> -----Original Message-----
>> From: Chris Ball [mailto:cjb@laptop.org]
>> Sent: Wednesday, March 27, 2013 9:32 PM
>> To: Ulf Hansson
>> Cc: linux-mmc@vger.kernel.org; Johan Rudholm; Ulf Hansson; Maya Erez;
>> Kevin Liu; Seungwon Jeon; jh80.chung@samsung.com
>> Subject: Re: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>
>> Hi Maya, Kevin, Seungwon, Jaehoon,
>>
>> On Fri, Mar 01 2013, Ulf Hansson wrote:
>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>
>>> SDIO has been using runtime pm for a while to handle runtime power save
>>> operations. This patchset is enabling the option to make the sd/mmc
>>> blockdevices to use runtime pm as well.
>>>
>>> The runtime pm implementation for the block device will make use of
>>> autosuspend to defer power save operation to after request inactivty
>>> for
>>> a certain time.
>>>
>>> To actually perform some power save operations the corresponding bus
>>> ops
>>> for mmc and sd shall be implemented. Typically it could make sense to
>>> do
>>> BKOPS for eMMC in here.
>>>
>>> Ulf Hansson (3):
>>>   mmc: core: Remove power_restore bus_ops for mmc and sd
>>>   mmc: core: Add bus_ops for runtime pm callbacks
>>>   mmc: block: Enable runtime pm for mmc blkdevice
>>>
>>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>>  drivers/mmc/core/bus.c   |   14 ++++++++++++--
>>>  drivers/mmc/core/core.h  |    2 ++
>>>  drivers/mmc/core/mmc.c   |   14 --------------
>>>  drivers/mmc/core/sd.c    |   14 --------------
>>>  drivers/mmc/core/sdio.c  |   20 ++++++++++++++++++++
>>>  6 files changed, 60 insertions(+), 32 deletions(-)
>>
>> This looks good to me, but I'd like to make sure the design works for
>> you before we decide to merge it.  Any concerns?
>>
>
> Chris,
>
> This patchset is ok for me.
> We can discuss/review how sd/mmc runtime callback function implemented
> later.
>
> Thanks
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[Ulf Hansson]

On 28 March 2013 22:05,  <merez@codeaurora.org> wrote:
> Hi Chris,
>
> Sorry for the late response.
> I am currently on vacation and would get back to work next week.
> We would like to have an internal discussion to make sure there are no
> additional concerns with this patch.
> Can you please wait with this patch approval until the end of next week?

Hi Maya,

Hope you have had a good vacation. I was thinking of the BKOPS patches
I assume you are working on. If you need some assistance to "rebase"
you work on top of this patchset, just ping me.

Kind regards
Ulf Hansson

>
> Thanks,
> Maya
>
>>> -----Original Message-----
>>> From: Chris Ball [mailto:cjb@laptop.org]
>>> Sent: Wednesday, March 27, 2013 9:32 PM
>>> To: Ulf Hansson
>>> Cc: linux-mmc@vger.kernel.org; Johan Rudholm; Ulf Hansson; Maya Erez;
>>> Kevin Liu; Seungwon Jeon; jh80.chung@samsung.com
>>> Subject: Re: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>>
>>> Hi Maya, Kevin, Seungwon, Jaehoon,
>>>
>>> On Fri, Mar 01 2013, Ulf Hansson wrote:
>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>>
>>>> SDIO has been using runtime pm for a while to handle runtime power save
>>>> operations. This patchset is enabling the option to make the sd/mmc
>>>> blockdevices to use runtime pm as well.
>>>>
>>>> The runtime pm implementation for the block device will make use of
>>>> autosuspend to defer power save operation to after request inactivty
>>>> for
>>>> a certain time.
>>>>
>>>> To actually perform some power save operations the corresponding bus
>>>> ops
>>>> for mmc and sd shall be implemented. Typically it could make sense to
>>>> do
>>>> BKOPS for eMMC in here.
>>>>
>>>> Ulf Hansson (3):
>>>>   mmc: core: Remove power_restore bus_ops for mmc and sd
>>>>   mmc: core: Add bus_ops for runtime pm callbacks
>>>>   mmc: block: Enable runtime pm for mmc blkdevice
>>>>
>>>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>>>  drivers/mmc/core/bus.c   |   14 ++++++++++++--
>>>>  drivers/mmc/core/core.h  |    2 ++
>>>>  drivers/mmc/core/mmc.c   |   14 --------------
>>>>  drivers/mmc/core/sd.c    |   14 --------------
>>>>  drivers/mmc/core/sdio.c  |   20 ++++++++++++++++++++
>>>>  6 files changed, 60 insertions(+), 32 deletions(-)
>>>
>>> This looks good to me, but I'd like to make sure the design works for
>>> you before we decide to merge it.  Any concerns?
>>>
>>
>> Chris,
>>
>> This patchset is ok for me.
>> We can discuss/review how sd/mmc runtime callback function implemented
>> later.
>>
>> Thanks
>> Kevin
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
> --
> Maya Erez
> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>

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

[Maya Erez]

בתאריך 4/2/2013 1:45 PM, ציטוט Ulf Hansson:
> On 28 March 2013 22:05,  <merez@codeaurora.org> wrote:
>> Hi Chris,
>>
>> Sorry for the late response.
>> I am currently on vacation and would get back to work next week.
>> We would like to have an internal discussion to make sure there are no
>> additional concerns with this patch.
>> Can you please wait with this patch approval until the end of next week?
> Hi Maya,
>
> Hope you have had a good vacation. I was thinking of the BKOPS patches
> I assume you are working on. If you need some assistance to "rebase"
> you work on top of this patchset, just ping me.
>
> Kind regards
> Ulf Hansson
>
>> Thanks,
>> Maya
>>
>>>> -----Original Message-----
>>>> From: Chris Ball [mailto:cjb@laptop.org]
>>>> Sent: Wednesday, March 27, 2013 9:32 PM
>>>> To: Ulf Hansson
>>>> Cc: linux-mmc@vger.kernel.org; Johan Rudholm; Ulf Hansson; Maya Erez;
>>>> Kevin Liu; Seungwon Jeon; jh80.chung@samsung.com
>>>> Subject: Re: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>>>
>>>> Hi Maya, Kevin, Seungwon, Jaehoon,
>>>>
>>>> On Fri, Mar 01 2013, Ulf Hansson wrote:
>>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>
>>>>> SDIO has been using runtime pm for a while to handle runtime power save
>>>>> operations. This patchset is enabling the option to make the sd/mmc
>>>>> blockdevices to use runtime pm as well.
>>>>>
>>>>> The runtime pm implementation for the block device will make use of
>>>>> autosuspend to defer power save operation to after request inactivty
>>>>> for
>>>>> a certain time.
>>>>>
>>>>> To actually perform some power save operations the corresponding bus
>>>>> ops
>>>>> for mmc and sd shall be implemented. Typically it could make sense to
>>>>> do
>>>>> BKOPS for eMMC in here.
>>>>>
>>>>> Ulf Hansson (3):
>>>>>    mmc: core: Remove power_restore bus_ops for mmc and sd
>>>>>    mmc: core: Add bus_ops for runtime pm callbacks
>>>>>    mmc: block: Enable runtime pm for mmc blkdevice
>>>>>
>>>>>   drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>>>>   drivers/mmc/core/bus.c   |   14 ++++++++++++--
>>>>>   drivers/mmc/core/core.h  |    2 ++
>>>>>   drivers/mmc/core/mmc.c   |   14 --------------
>>>>>   drivers/mmc/core/sd.c    |   14 --------------
>>>>>   drivers/mmc/core/sdio.c  |   20 ++++++++++++++++++++
>>>>>   6 files changed, 60 insertions(+), 32 deletions(-)
>>>> This looks good to me, but I'd like to make sure the design works for
>>>> you before we decide to merge it.  Any concerns?
>>>>
>>> Chris,
>>>
>>> This patchset is ok for me.
>>> We can discuss/review how sd/mmc runtime callback function implemented
>>> later.
>>>
>>> Thanks
>>> Kevin
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> --
>> Maya Erez
>> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Ulf,

Thanks for your offer.

The current plan is to start the BKOPS in the runtime suspend API 
(triggerred 3 secs after idle time) although it may be less efficient 
than the current approach which starts the BKOPS after 200ms from idle time.
I will try to send the BKOPS patch next week.

Chris - we are OK with this patch, you can continue with its merge.

Thanks,
Maya

-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

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

[Kevin Liu]

> From: Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
> Date: Fri, Mar 1, 2013 at 8:47 PM
> Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
> To: linux-mmc@vger.kernel.org<mailto:linux-mmc@vger.kernel.org>, Chris Ball <cjb@laptop.org<mailto:cjb@laptop.org>>
> Cc: Johan Rudholm <johan.rudholm@stericsson.com<mailto:johan.rudholm@stericsson.com>>, Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>
>
> From: Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>
> SDIO has been using runtime pm for a while to handle runtime power save
> operations. This patchset is enabling the option to make the sd/mmc
> blockdevices to use runtime pm as well.
>
> The runtime pm implementation for the block device will make use of
> autosuspend to defer power save operation to after request inactivty for
> a certain time.
>
> To actually perform some power save operations the corresponding bus ops
> for mmc and sd shall be implemented. Typically it could make sense to do
> BKOPS for eMMC in here.
>
> Ulf Hansson (3):
>   mmc: core: Remove power_restore bus_ops for mmc and sd
>   mmc: core: Add bus_ops for runtime pm callbacks
>   mmc: block: Enable runtime pm for mmc blkdevice
>
Ulf,

sdhci.c has added pm_runtime which also protect between request and
task finish. And some sdhci.c based host drivers has provided
pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
powersave viewpoint, I think adding pm_runtime in driver level is
better than doing that on bus level since the control granularity is
even smaller. And adding pm_runtime in both block.c and sdhci.c will
call pm_runtime twice. How do you think?

Thanks
Kevin

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

[Ulf Hansson]

On 7 March 2013 01:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> From: Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
>> Date: Fri, Mar 1, 2013 at 8:47 PM
>> Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>> To: linux-mmc@vger.kernel.org<mailto:linux-mmc@vger.kernel.org>, Chris Ball <cjb@laptop.org<mailto:cjb@laptop.org>>
>> Cc: Johan Rudholm <johan.rudholm@stericsson.com<mailto:johan.rudholm@stericsson.com>>, Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>
>>
>> From: Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>
>> SDIO has been using runtime pm for a while to handle runtime power save
>> operations. This patchset is enabling the option to make the sd/mmc
>> blockdevices to use runtime pm as well.
>>
>> The runtime pm implementation for the block device will make use of
>> autosuspend to defer power save operation to after request inactivty for
>> a certain time.
>>
>> To actually perform some power save operations the corresponding bus ops
>> for mmc and sd shall be implemented. Typically it could make sense to do
>> BKOPS for eMMC in here.
>>
>> Ulf Hansson (3):
>>   mmc: core: Remove power_restore bus_ops for mmc and sd
>>   mmc: core: Add bus_ops for runtime pm callbacks
>>   mmc: block: Enable runtime pm for mmc blkdevice
>>
> Ulf,
>
> sdhci.c has added pm_runtime which also protect between request and
> task finish. And some sdhci.c based host drivers has provided
> pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
> powersave viewpoint, I think adding pm_runtime in driver level is
> better than doing that on bus level since the control granularity is
> even smaller. And adding pm_runtime in both block.c and sdhci.c will
> call pm_runtime twice. How do you think?
>
> Thanks
> Kevin

Hi Kevin,

Thanks for your response!

It seems like we need some more clarification around this area.
Runtime pm for a host device driver shall ultimately be responsible
for taking care of runtime power management of the host device - only.
It should not handle runtime power management of a block device, which
in principle means BKOPS shall be handled in the blkdevice. At least
this is my view.

So, why is this? I will try to elaborate on the runtime pm support in
host drivers here.
The host device driver controls a MMC/SD/SDIO IP. This IP could very
well reside (for some SoC) in what you call a power domain. In
principle, once the IP needs to be used, a host driver has done a
pm_runtime_get of it's device. This will mean a reference to the power
domain has been fetched. Once the IP is not needed any more,
pm_runtime_put is done and the reference to the power domain is
released. Once no reference to the power domain exist the power domain
can enter lower sleep states, which is preferred to happen as soon as
possible and as long as possible - of course.

Hope this gives a better understanding. :-)

Kind regards
Ulf Hansson

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

[Kevin Liu]

2013/3/7 Ulf Hansson <ulf.hansson@linaro.org>:
> On 7 March 2013 01:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>> From: Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
>>> Date: Fri, Mar 1, 2013 at 8:47 PM
>>> Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>> To: linux-mmc@vger.kernel.org<mailto:linux-mmc@vger.kernel.org>, Chris Ball <cjb@laptop.org<mailto:cjb@laptop.org>>
>>> Cc: Johan Rudholm <johan.rudholm@stericsson.com<mailto:johan.rudholm@stericsson.com>>, Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>
>>>
>>> From: Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>
>>> SDIO has been using runtime pm for a while to handle runtime power save
>>> operations. This patchset is enabling the option to make the sd/mmc
>>> blockdevices to use runtime pm as well.
>>>
>>> The runtime pm implementation for the block device will make use of
>>> autosuspend to defer power save operation to after request inactivty for
>>> a certain time.
>>>
>>> To actually perform some power save operations the corresponding bus ops
>>> for mmc and sd shall be implemented. Typically it could make sense to do
>>> BKOPS for eMMC in here.
>>>
>>> Ulf Hansson (3):
>>>   mmc: core: Remove power_restore bus_ops for mmc and sd
>>>   mmc: core: Add bus_ops for runtime pm callbacks
>>>   mmc: block: Enable runtime pm for mmc blkdevice
>>>
>> Ulf,
>>
>> sdhci.c has added pm_runtime which also protect between request and
>> task finish. And some sdhci.c based host drivers has provided
>> pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
>> powersave viewpoint, I think adding pm_runtime in driver level is
>> better than doing that on bus level since the control granularity is
>> even smaller. And adding pm_runtime in both block.c and sdhci.c will
>> call pm_runtime twice. How do you think?
>>
>> Thanks
>> Kevin
>
> Hi Kevin,
>
> Thanks for your response!
>
> It seems like we need some more clarification around this area.
> Runtime pm for a host device driver shall ultimately be responsible
> for taking care of runtime power management of the host device - only.
> It should not handle runtime power management of a block device, which
> in principle means BKOPS shall be handled in the blkdevice. At least
> this is my view.
>
> So, why is this? I will try to elaborate on the runtime pm support in
> host drivers here.
> The host device driver controls a MMC/SD/SDIO IP. This IP could very
> well reside (for some SoC) in what you call a power domain. In
> principle, once the IP needs to be used, a host driver has done a
> pm_runtime_get of it's device. This will mean a reference to the power
> domain has been fetched. Once the IP is not needed any more,
> pm_runtime_put is done and the reference to the power domain is
> released. Once no reference to the power domain exist the power domain
> can enter lower sleep states, which is preferred to happen as soon as
> possible and as long as possible - of course.
>
> Hope this gives a better understanding. :-)
>
Ulf,

Thanks for the explanations!
Then do you mean to start bkops when blkdev pm_runtime auto suspended
while stop bkops when blkdev pm_runtime resumed?
My only concern is that we have implemented pm_runtime for host device
and its pm_runtime functions will turn on/off bus clock when host dev
runtime resume/suspend. Let's see below sequence when an issue request
come:
1. blkdev pm_runtime resumed in mmc_blk_issue_rq.
2. blkdev issue request
3. host dev pm_runtime resumed in host->ops->request.
4. host finished the transfer and host dev pm_runtime suspended.
5. 3s later, blkdev pm_runtime suspended.
The bus clock will be turn off in step 4 by host dev
pm_runtime_suspend function. Then how can bkops run in step 5?

Kevin

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

[Kevin Liu]

2013/3/7 Kevin Liu <keyuan.liu@gmail.com>:
> 2013/3/7 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 7 March 2013 01:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>> From: Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
>>>> Date: Fri, Mar 1, 2013 at 8:47 PM
>>>> Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>>> To: linux-mmc@vger.kernel.org<mailto:linux-mmc@vger.kernel.org>, Chris Ball <cjb@laptop.org<mailto:cjb@laptop.org>>
>>>> Cc: Johan Rudholm <johan.rudholm@stericsson.com<mailto:johan.rudholm@stericsson.com>>, Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>
>>>>
>>>> From: Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>
>>>> SDIO has been using runtime pm for a while to handle runtime power save
>>>> operations. This patchset is enabling the option to make the sd/mmc
>>>> blockdevices to use runtime pm as well.
>>>>
>>>> The runtime pm implementation for the block device will make use of
>>>> autosuspend to defer power save operation to after request inactivty for
>>>> a certain time.
>>>>
>>>> To actually perform some power save operations the corresponding bus ops
>>>> for mmc and sd shall be implemented. Typically it could make sense to do
>>>> BKOPS for eMMC in here.
>>>>
>>>> Ulf Hansson (3):
>>>>   mmc: core: Remove power_restore bus_ops for mmc and sd
>>>>   mmc: core: Add bus_ops for runtime pm callbacks
>>>>   mmc: block: Enable runtime pm for mmc blkdevice
>>>>
>>> Ulf,
>>>
>>> sdhci.c has added pm_runtime which also protect between request and
>>> task finish. And some sdhci.c based host drivers has provided
>>> pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
>>> powersave viewpoint, I think adding pm_runtime in driver level is
>>> better than doing that on bus level since the control granularity is
>>> even smaller. And adding pm_runtime in both block.c and sdhci.c will
>>> call pm_runtime twice. How do you think?
>>>
>>> Thanks
>>> Kevin
>>
>> Hi Kevin,
>>
>> Thanks for your response!
>>
>> It seems like we need some more clarification around this area.
>> Runtime pm for a host device driver shall ultimately be responsible
>> for taking care of runtime power management of the host device - only.
>> It should not handle runtime power management of a block device, which
>> in principle means BKOPS shall be handled in the blkdevice. At least
>> this is my view.
>>
>> So, why is this? I will try to elaborate on the runtime pm support in
>> host drivers here.
>> The host device driver controls a MMC/SD/SDIO IP. This IP could very
>> well reside (for some SoC) in what you call a power domain. In
>> principle, once the IP needs to be used, a host driver has done a
>> pm_runtime_get of it's device. This will mean a reference to the power
>> domain has been fetched. Once the IP is not needed any more,
>> pm_runtime_put is done and the reference to the power domain is
>> released. Once no reference to the power domain exist the power domain
>> can enter lower sleep states, which is preferred to happen as soon as
>> possible and as long as possible - of course.
>>
>> Hope this gives a better understanding. :-)
>>
> Ulf,
>
> Thanks for the explanations!
> Then do you mean to start bkops when blkdev pm_runtime auto suspended
> while stop bkops when blkdev pm_runtime resumed?
> My only concern is that we have implemented pm_runtime for host device
> and its pm_runtime functions will turn on/off bus clock when host dev
> runtime resume/suspend. Let's see below sequence when an issue request
> come:
> 1. blkdev pm_runtime resumed in mmc_blk_issue_rq.
> 2. blkdev issue request
> 3. host dev pm_runtime resumed in host->ops->request.
> 4. host finished the transfer and host dev pm_runtime suspended.
> 5. 3s later, blkdev pm_runtime suspended.
> The bus clock will be turn off in step 4 by host dev
> pm_runtime_suspend function. Then how can bkops run in step 5?
>
My question is host dev will stop bus clock by pm_runtime_suspend once
the request transfer is finished. But bkops on emmc chip should still
need the bus clock after bkops started. How to handle this?

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

[Ulf Hansson]

On 7 March 2013 22:14, Kevin Liu <keyuan.liu@gmail.com> wrote:
> 2013/3/7 Kevin Liu <keyuan.liu@gmail.com>:
>> 2013/3/7 Ulf Hansson <ulf.hansson@linaro.org>:
>>> On 7 March 2013 01:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>>> From: Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
>>>>> Date: Fri, Mar 1, 2013 at 8:47 PM
>>>>> Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>>>> To: linux-mmc@vger.kernel.org<mailto:linux-mmc@vger.kernel.org>, Chris Ball <cjb@laptop.org<mailto:cjb@laptop.org>>
>>>>> Cc: Johan Rudholm <johan.rudholm@stericsson.com<mailto:johan.rudholm@stericsson.com>>, Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>>
>>>>>
>>>>> From: Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>>
>>>>> SDIO has been using runtime pm for a while to handle runtime power save
>>>>> operations. This patchset is enabling the option to make the sd/mmc
>>>>> blockdevices to use runtime pm as well.
>>>>>
>>>>> The runtime pm implementation for the block device will make use of
>>>>> autosuspend to defer power save operation to after request inactivty for
>>>>> a certain time.
>>>>>
>>>>> To actually perform some power save operations the corresponding bus ops
>>>>> for mmc and sd shall be implemented. Typically it could make sense to do
>>>>> BKOPS for eMMC in here.
>>>>>
>>>>> Ulf Hansson (3):
>>>>>   mmc: core: Remove power_restore bus_ops for mmc and sd
>>>>>   mmc: core: Add bus_ops for runtime pm callbacks
>>>>>   mmc: block: Enable runtime pm for mmc blkdevice
>>>>>
>>>> Ulf,
>>>>
>>>> sdhci.c has added pm_runtime which also protect between request and
>>>> task finish. And some sdhci.c based host drivers has provided
>>>> pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
>>>> powersave viewpoint, I think adding pm_runtime in driver level is
>>>> better than doing that on bus level since the control granularity is
>>>> even smaller. And adding pm_runtime in both block.c and sdhci.c will
>>>> call pm_runtime twice. How do you think?
>>>>
>>>> Thanks
>>>> Kevin
>>>
>>> Hi Kevin,
>>>
>>> Thanks for your response!
>>>
>>> It seems like we need some more clarification around this area.
>>> Runtime pm for a host device driver shall ultimately be responsible
>>> for taking care of runtime power management of the host device - only.
>>> It should not handle runtime power management of a block device, which
>>> in principle means BKOPS shall be handled in the blkdevice. At least
>>> this is my view.
>>>
>>> So, why is this? I will try to elaborate on the runtime pm support in
>>> host drivers here.
>>> The host device driver controls a MMC/SD/SDIO IP. This IP could very
>>> well reside (for some SoC) in what you call a power domain. In
>>> principle, once the IP needs to be used, a host driver has done a
>>> pm_runtime_get of it's device. This will mean a reference to the power
>>> domain has been fetched. Once the IP is not needed any more,
>>> pm_runtime_put is done and the reference to the power domain is
>>> released. Once no reference to the power domain exist the power domain
>>> can enter lower sleep states, which is preferred to happen as soon as
>>> possible and as long as possible - of course.
>>>
>>> Hope this gives a better understanding. :-)
>>>
>> Ulf,
>>
>> Thanks for the explanations!
>> Then do you mean to start bkops when blkdev pm_runtime auto suspended
>> while stop bkops when blkdev pm_runtime resumed?
>> My only concern is that we have implemented pm_runtime for host device
>> and its pm_runtime functions will turn on/off bus clock when host dev
>> runtime resume/suspend. Let's see below sequence when an issue request
>> come:
>> 1. blkdev pm_runtime resumed in mmc_blk_issue_rq.
>> 2. blkdev issue request
>> 3. host dev pm_runtime resumed in host->ops->request.
>> 4. host finished the transfer and host dev pm_runtime suspended.
>> 5. 3s later, blkdev pm_runtime suspended.
>> The bus clock will be turn off in step 4 by host dev
>> pm_runtime_suspend function. Then how can bkops run in step 5?
>>
> My question is host dev will stop bus clock by pm_runtime_suspend once
> the request transfer is finished. But bkops on emmc chip should still
> need the bus clock after bkops started. How to handle this?

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.

Kind regards
Ulf Hansson

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

[Kevin Liu]

2013/3/8 Ulf Hansson <ulf.hansson@linaro.org>:
> On 7 March 2013 22:14, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> 2013/3/7 Kevin Liu <keyuan.liu@gmail.com>:
>>> 2013/3/7 Ulf Hansson <ulf.hansson@linaro.org>:
>>>> On 7 March 2013 01:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>>>> From: Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
>>>>>> Date: Fri, Mar 1, 2013 at 8:47 PM
>>>>>> Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>>>>> To: linux-mmc@vger.kernel.org<mailto:linux-mmc@vger.kernel.org>, Chris Ball <cjb@laptop.org<mailto:cjb@laptop.org>>
>>>>>> Cc: Johan Rudholm <johan.rudholm@stericsson.com<mailto:johan.rudholm@stericsson.com>>, Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>>>
>>>>>>
>>>>>> From: Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>>>
>>>>>> SDIO has been using runtime pm for a while to handle runtime power save
>>>>>> operations. This patchset is enabling the option to make the sd/mmc
>>>>>> blockdevices to use runtime pm as well.
>>>>>>
>>>>>> The runtime pm implementation for the block device will make use of
>>>>>> autosuspend to defer power save operation to after request inactivty for
>>>>>> a certain time.
>>>>>>
>>>>>> To actually perform some power save operations the corresponding bus ops
>>>>>> for mmc and sd shall be implemented. Typically it could make sense to do
>>>>>> BKOPS for eMMC in here.
>>>>>>
>>>>>> Ulf Hansson (3):
>>>>>>   mmc: core: Remove power_restore bus_ops for mmc and sd
>>>>>>   mmc: core: Add bus_ops for runtime pm callbacks
>>>>>>   mmc: block: Enable runtime pm for mmc blkdevice
>>>>>>
>>>>> Ulf,
>>>>>
>>>>> sdhci.c has added pm_runtime which also protect between request and
>>>>> task finish. And some sdhci.c based host drivers has provided
>>>>> pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
>>>>> powersave viewpoint, I think adding pm_runtime in driver level is
>>>>> better than doing that on bus level since the control granularity is
>>>>> even smaller. And adding pm_runtime in both block.c and sdhci.c will
>>>>> call pm_runtime twice. How do you think?
>>>>>
>>>>> Thanks
>>>>> Kevin
>>>>
>>>> Hi Kevin,
>>>>
>>>> Thanks for your response!
>>>>
>>>> It seems like we need some more clarification around this area.
>>>> Runtime pm for a host device driver shall ultimately be responsible
>>>> for taking care of runtime power management of the host device - only.
>>>> It should not handle runtime power management of a block device, which
>>>> in principle means BKOPS shall be handled in the blkdevice. At least
>>>> this is my view.
>>>>
>>>> So, why is this? I will try to elaborate on the runtime pm support in
>>>> host drivers here.
>>>> The host device driver controls a MMC/SD/SDIO IP. This IP could very
>>>> well reside (for some SoC) in what you call a power domain. In
>>>> principle, once the IP needs to be used, a host driver has done a
>>>> pm_runtime_get of it's device. This will mean a reference to the power
>>>> domain has been fetched. Once the IP is not needed any more,
>>>> pm_runtime_put is done and the reference to the power domain is
>>>> released. Once no reference to the power domain exist the power domain
>>>> can enter lower sleep states, which is preferred to happen as soon as
>>>> possible and as long as possible - of course.
>>>>
>>>> Hope this gives a better understanding. :-)
>>>>
>>> Ulf,
>>>
>>> Thanks for the explanations!
>>> Then do you mean to start bkops when blkdev pm_runtime auto suspended
>>> while stop bkops when blkdev pm_runtime resumed?
>>> My only concern is that we have implemented pm_runtime for host device
>>> and its pm_runtime functions will turn on/off bus clock when host dev
>>> runtime resume/suspend. Let's see below sequence when an issue request
>>> come:
>>> 1. blkdev pm_runtime resumed in mmc_blk_issue_rq.
>>> 2. blkdev issue request
>>> 3. host dev pm_runtime resumed in host->ops->request.
>>> 4. host finished the transfer and host dev pm_runtime suspended.
>>> 5. 3s later, blkdev pm_runtime suspended.
>>> The bus clock will be turn off in step 4 by host dev
>>> pm_runtime_suspend function. Then how can bkops run in step 5?
>>>
>> My question is host dev will stop bus clock by pm_runtime_suspend once
>> the request transfer is finished. But bkops on emmc chip should still
>> need the bus clock after bkops started. How to handle this?
>
> 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.
>

Got it. Thanks!

> Kind regards
> Ulf Hansson

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

[Sujit Reddy Thumma]

On 3/8/2013 8:44 AM, Ulf Hansson wrote:
> On 7 March 2013 22:14, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> 2013/3/7 Kevin Liu <keyuan.liu@gmail.com>:
>>> 2013/3/7 Ulf Hansson <ulf.hansson@linaro.org>:
>>>> On 7 March 2013 01:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>>>> From: Ulf Hansson <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
>>>>>> Date: Fri, Mar 1, 2013 at 8:47 PM
>>>>>> Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>>>>> To: linux-mmc@vger.kernel.org<mailto:linux-mmc@vger.kernel.org>, Chris Ball <cjb@laptop.org<mailto:cjb@laptop.org>>
>>>>>> Cc: Johan Rudholm <johan.rudholm@stericsson.com<mailto:johan.rudholm@stericsson.com>>, Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>>>
>>>>>>
>>>>>> From: Ulf Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>>>
>>>>>> SDIO has been using runtime pm for a while to handle runtime power save
>>>>>> operations. This patchset is enabling the option to make the sd/mmc
>>>>>> blockdevices to use runtime pm as well.
>>>>>>
>>>>>> The runtime pm implementation for the block device will make use of
>>>>>> autosuspend to defer power save operation to after request inactivty for
>>>>>> a certain time.
>>>>>>
>>>>>> To actually perform some power save operations the corresponding bus ops
>>>>>> for mmc and sd shall be implemented. Typically it could make sense to do
>>>>>> BKOPS for eMMC in here.
>>>>>>
>>>>>> Ulf Hansson (3):
>>>>>>    mmc: core: Remove power_restore bus_ops for mmc and sd
>>>>>>    mmc: core: Add bus_ops for runtime pm callbacks
>>>>>>    mmc: block: Enable runtime pm for mmc blkdevice
>>>>>>
>>>>> Ulf,
>>>>>
>>>>> sdhci.c has added pm_runtime which also protect between request and
>>>>> task finish. And some sdhci.c based host drivers has provided
>>>>> pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
>>>>> powersave viewpoint, I think adding pm_runtime in driver level is
>>>>> better than doing that on bus level since the control granularity is
>>>>> even smaller. And adding pm_runtime in both block.c and sdhci.c will
>>>>> call pm_runtime twice. How do you think?
>>>>>
>>>>> Thanks
>>>>> Kevin
>>>>
>>>> Hi Kevin,
>>>>
>>>> Thanks for your response!
>>>>
>>>> It seems like we need some more clarification around this area.
>>>> Runtime pm for a host device driver shall ultimately be responsible
>>>> for taking care of runtime power management of the host device - only.
>>>> It should not handle runtime power management of a block device, which
>>>> in principle means BKOPS shall be handled in the blkdevice. At least
>>>> this is my view.
>>>>
>>>> So, why is this? I will try to elaborate on the runtime pm support in
>>>> host drivers here.
>>>> The host device driver controls a MMC/SD/SDIO IP. This IP could very
>>>> well reside (for some SoC) in what you call a power domain. In
>>>> principle, once the IP needs to be used, a host driver has done a
>>>> pm_runtime_get of it's device. This will mean a reference to the power
>>>> domain has been fetched. Once the IP is not needed any more,
>>>> pm_runtime_put is done and the reference to the power domain is
>>>> released. Once no reference to the power domain exist the power domain
>>>> can enter lower sleep states, which is preferred to happen as soon as
>>>> possible and as long as possible - of course.
>>>>
>>>> Hope this gives a better understanding. :-)
>>>>
>>> Ulf,
>>>
>>> Thanks for the explanations!
>>> Then do you mean to start bkops when blkdev pm_runtime auto suspended
>>> while stop bkops when blkdev pm_runtime resumed?
>>> My only concern is that we have implemented pm_runtime for host device
>>> and its pm_runtime functions will turn on/off bus clock when host dev
>>> runtime resume/suspend. Let's see below sequence when an issue request
>>> come:
>>> 1. blkdev pm_runtime resumed in mmc_blk_issue_rq.
>>> 2. blkdev issue request
>>> 3. host dev pm_runtime resumed in host->ops->request.
>>> 4. host finished the transfer and host dev pm_runtime suspended.
>>> 5. 3s later, blkdev pm_runtime suspended.
>>> The bus clock will be turn off in step 4 by host dev
>>> pm_runtime_suspend function. Then how can bkops run in step 5?
>>>
>> My question is host dev will stop bus clock by pm_runtime_suspend once
>> the request transfer is finished. But bkops on emmc chip should still
>> need the bus clock after bkops started. How to handle this?
>
> 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?


-- 
Regards,
Sujit

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

[Ulf Hansson]

On 15 March 2013 05:18, Sujit Reddy Thumma <sthumma@codeaurora.org> wrote:
> On 3/8/2013 8:44 AM, Ulf Hansson wrote:
>>
>> On 7 March 2013 22:14, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>
>>> 2013/3/7 Kevin Liu <keyuan.liu@gmail.com>:
>>>>
>>>> 2013/3/7 Ulf Hansson <ulf.hansson@linaro.org>:
>>>>>
>>>>> On 7 March 2013 01:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>>>>>
>>>>>>> From: Ulf Hansson
>>>>>>> <ulf.hansson@stericsson.com<mailto:ulf.hansson@stericsson.com>>
>>>>>>> Date: Fri, Mar 1, 2013 at 8:47 PM
>>>>>>> Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice
>>>>>>> To: linux-mmc@vger.kernel.org<mailto:linux-mmc@vger.kernel.org>,
>>>>>>> Chris Ball <cjb@laptop.org<mailto:cjb@laptop.org>>
>>>>>>> Cc: Johan Rudholm
>>>>>>> <johan.rudholm@stericsson.com<mailto:johan.rudholm@stericsson.com>>, Ulf
>>>>>>> Hansson <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>>>>
>>>>>>>
>>>>>>> From: Ulf Hansson
>>>>>>> <ulf.hansson@linaro.org<mailto:ulf.hansson@linaro.org>>
>>>>>>>
>>>>>>> SDIO has been using runtime pm for a while to handle runtime power
>>>>>>> save
>>>>>>> operations. This patchset is enabling the option to make the sd/mmc
>>>>>>> blockdevices to use runtime pm as well.
>>>>>>>
>>>>>>> The runtime pm implementation for the block device will make use of
>>>>>>> autosuspend to defer power save operation to after request inactivty
>>>>>>> for
>>>>>>> a certain time.
>>>>>>>
>>>>>>> To actually perform some power save operations the corresponding bus
>>>>>>> ops
>>>>>>> for mmc and sd shall be implemented. Typically it could make sense to
>>>>>>> do
>>>>>>> BKOPS for eMMC in here.
>>>>>>>
>>>>>>> Ulf Hansson (3):
>>>>>>>    mmc: core: Remove power_restore bus_ops for mmc and sd
>>>>>>>    mmc: core: Add bus_ops for runtime pm callbacks
>>>>>>>    mmc: block: Enable runtime pm for mmc blkdevice
>>>>>>>
>>>>>> Ulf,
>>>>>>
>>>>>> sdhci.c has added pm_runtime which also protect between request and
>>>>>> task finish. And some sdhci.c based host drivers has provided
>>>>>> pm_runtime_suspend/resume functions like sdhci-pxav3.c. From the
>>>>>> powersave viewpoint, I think adding pm_runtime in driver level is
>>>>>> better than doing that on bus level since the control granularity is
>>>>>> even smaller. And adding pm_runtime in both block.c and sdhci.c will
>>>>>> call pm_runtime twice. How do you think?
>>>>>>
>>>>>> Thanks
>>>>>> Kevin
>>>>>
>>>>>
>>>>> Hi Kevin,
>>>>>
>>>>> Thanks for your response!
>>>>>
>>>>> It seems like we need some more clarification around this area.
>>>>> Runtime pm for a host device driver shall ultimately be responsible
>>>>> for taking care of runtime power management of the host device - only.
>>>>> It should not handle runtime power management of a block device, which
>>>>> in principle means BKOPS shall be handled in the blkdevice. At least
>>>>> this is my view.
>>>>>
>>>>> So, why is this? I will try to elaborate on the runtime pm support in
>>>>> host drivers here.
>>>>> The host device driver controls a MMC/SD/SDIO IP. This IP could very
>>>>> well reside (for some SoC) in what you call a power domain. In
>>>>> principle, once the IP needs to be used, a host driver has done a
>>>>> pm_runtime_get of it's device. This will mean a reference to the power
>>>>> domain has been fetched. Once the IP is not needed any more,
>>>>> pm_runtime_put is done and the reference to the power domain is
>>>>> released. Once no reference to the power domain exist the power domain
>>>>> can enter lower sleep states, which is preferred to happen as soon as
>>>>> possible and as long as possible - of course.
>>>>>
>>>>> Hope this gives a better understanding. :-)
>>>>>
>>>> Ulf,
>>>>
>>>> Thanks for the explanations!
>>>> Then do you mean to start bkops when blkdev pm_runtime auto suspended
>>>> while stop bkops when blkdev pm_runtime resumed?
>>>> My only concern is that we have implemented pm_runtime for host device
>>>> and its pm_runtime functions will turn on/off bus clock when host dev
>>>> runtime resume/suspend. Let's see below sequence when an issue request
>>>> come:
>>>> 1. blkdev pm_runtime resumed in mmc_blk_issue_rq.
>>>> 2. blkdev issue request
>>>> 3. host dev pm_runtime resumed in host->ops->request.
>>>> 4. host finished the transfer and host dev pm_runtime suspended.
>>>> 5. 3s later, blkdev pm_runtime suspended.
>>>> The bus clock will be turn off in step 4 by host dev
>>>> pm_runtime_suspend function. Then how can bkops run in step 5?
>>>>
>>> My question is host dev will stop bus clock by pm_runtime_suspend once
>>> the request transfer is finished. But bkops on emmc chip should still
>>> need the bus clock after bkops started. How to handle this?
>>
>>
>> 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.

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


>
>
> --
> Regards,
> Sujit

Kind regards
Ulf Hansson

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

[Sujit Reddy Thumma]

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

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.

>
> 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()?

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


-- 
Regards,
Sujit

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

[Ulf Hansson]

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

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

[Ulf Hansson]

On 20 March 2013 22:58, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 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.

Sorry, I mean when runtime resume get 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

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

[Sujit Reddy Thumma]

Hi Ulf,

Sorry for the delayed response.
The patches looks good to me except one concern mentioned below.

On 3/21/2013 3:28 AM, Ulf Hansson wrote:

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

Well.. my point here is that runtime PM framework unnecessarily wakeup 
processor (if idle) every "x" secs without doing any useful work. If 
that is agreeable then I am okay with not having the optimization.

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

Thanks for the details. It looks clear to me now.

>
>>
>> "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 agree.

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

Possibly, the races can be avoided using pm_runtime_suspended() check, 
but I am not sure if it is the clean way.


-- 
Regards,
Sujit

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

[Ulf Hansson]

From: Ulf Hansson <ulf.hansson@linaro.org>

SDIO has been using runtime pm for a while to handle runtime power save
operations. This patchset is enabling the option to make the sd/mmc
blockdevices to use runtime pm as well.

The runtime pm implementation for the block device will make use of
autosuspend to defer power save operation to after request inactivty for
a certain time.

To actually perform some power save operations the corresponding bus ops
for mmc and sd shall be implemented. Typically it could make sense to do
BKOPS for eMMC in here.

Ulf Hansson (3):
  mmc: core: Remove power_restore bus_ops for mmc and sd
  mmc: core: Add bus_ops for runtime pm callbacks
  mmc: block: Enable runtime pm for mmc blkdevice

 drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
 drivers/mmc/core/bus.c   |   14 ++++++++++++--
 drivers/mmc/core/core.h  |    2 ++
 drivers/mmc/core/mmc.c   |   14 --------------
 drivers/mmc/core/sd.c    |   14 --------------
 drivers/mmc/core/sdio.c  |   20 ++++++++++++++++++++
 6 files changed, 60 insertions(+), 32 deletions(-)

-- 
1.7.10

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

[Maya Erez]

Thanks Ulf.
I will go over the new patch and will develop the periodic BKOPS on top of
it.

Thanks,
Maya

-----Original Message-----
From: linux-mmc-owner@vger.kernel.org
[mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Ulf Hansson
Sent: Friday, March 01, 2013 2:47 PM
To: linux-mmc@vger.kernel.org; Chris Ball
Cc: Johan Rudholm; Ulf Hansson
Subject: [PATCH 0/3] mmc: Use runtime pm for blkdevice

From: Ulf Hansson <ulf.hansson@linaro.org>

SDIO has been using runtime pm for a while to handle runtime power save
operations. This patchset is enabling the option to make the sd/mmc
blockdevices to use runtime pm as well.

The runtime pm implementation for the block device will make use of
autosuspend to defer power save operation to after request inactivty for a
certain time.

To actually perform some power save operations the corresponding bus ops for
mmc and sd shall be implemented. Typically it could make sense to do BKOPS
for eMMC in here.

Ulf Hansson (3):
  mmc: core: Remove power_restore bus_ops for mmc and sd
  mmc: core: Add bus_ops for runtime pm callbacks
  mmc: block: Enable runtime pm for mmc blkdevice

 drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
 drivers/mmc/core/bus.c   |   14 ++++++++++++--
 drivers/mmc/core/core.h  |    2 ++
 drivers/mmc/core/mmc.c   |   14 --------------
 drivers/mmc/core/sd.c    |   14 --------------
 drivers/mmc/core/sdio.c  |   20 ++++++++++++++++++++
 6 files changed, 60 insertions(+), 32 deletions(-)

--
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the
body of a message to majordomo@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html

Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[Chris Ball]

Hi Maya, Kevin, Seungwon, Jaehoon,

On Fri, Mar 01 2013, Ulf Hansson wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> SDIO has been using runtime pm for a while to handle runtime power save
> operations. This patchset is enabling the option to make the sd/mmc
> blockdevices to use runtime pm as well.
>
> The runtime pm implementation for the block device will make use of
> autosuspend to defer power save operation to after request inactivty for
> a certain time.
>
> To actually perform some power save operations the corresponding bus ops
> for mmc and sd shall be implemented. Typically it could make sense to do
> BKOPS for eMMC in here.
>
> Ulf Hansson (3):
>   mmc: core: Remove power_restore bus_ops for mmc and sd
>   mmc: core: Add bus_ops for runtime pm callbacks
>   mmc: block: Enable runtime pm for mmc blkdevice
>
>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>  drivers/mmc/core/bus.c   |   14 ++++++++++++--
>  drivers/mmc/core/core.h  |    2 ++
>  drivers/mmc/core/mmc.c   |   14 --------------
>  drivers/mmc/core/sd.c    |   14 --------------
>  drivers/mmc/core/sdio.c  |   20 ++++++++++++++++++++
>  6 files changed, 60 insertions(+), 32 deletions(-)

This looks good to me, but I'd like to make sure the design works for
you before we decide to merge it.  Any concerns?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

[Arnd Bergmann]

On Friday 01 March 2013, Ulf Hansson wrote:
> 
> From: Ulf Hansson <ulf.hansson@linaro.org>
> 
> SDIO has been using runtime pm for a while to handle runtime power save
> operations. This patchset is enabling the option to make the sd/mmc
> blockdevices to use runtime pm as well.
> 
> The runtime pm implementation for the block device will make use of
> autosuspend to defer power save operation to after request inactivty for
> a certain time.
> 
> To actually perform some power save operations the corresponding bus ops
> for mmc and sd shall be implemented. Typically it could make sense to do
> BKOPS for eMMC in here.

As per IRC discussion:

Acked-by: Arnd Bergmann <arnd@arndb.de>