[PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
This enables runtime PM for eMMC/SD card.

Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
---
 drivers/mmc/host/sdhci-msm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index e00208535bd1..6657f7db1b8e 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -2626,6 +2626,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	msm_host->mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
 	msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
 
 	/* Set the timeout value to max possible */
-- 
2.17.1

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Dmitry Baryshkov]

On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
> This enables runtime PM for eMMC/SD card.

Could you please mention, which platforms were tested with this patch?
Note, upstream kernel supports a lot of platforms, including MSM8974, I
think the oldest one, which uses SDHCI.

> 
> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> ---
>  drivers/mmc/host/sdhci-msm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index e00208535bd1..6657f7db1b8e 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -2626,6 +2626,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		goto clk_disable;
>  	}
>  
> +	msm_host->mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
>  	msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
>  
>  	/* Set the timeout value to max possible */
> -- 
> 2.17.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>> This enables runtime PM for eMMC/SD card.
> 
> Could you please mention, which platforms were tested with this patch?
> Note, upstream kernel supports a lot of platforms, including MSM8974, I
> think the oldest one, which uses SDHCI.
>

This was tested with qdu1000 platform.

>>
>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>> ---
>>   drivers/mmc/host/sdhci-msm.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index e00208535bd1..6657f7db1b8e 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -2626,6 +2626,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>   		goto clk_disable;
>>   	}
>>   
>> +	msm_host->mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
>>   	msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
>>   
>>   	/* Set the timeout value to max possible */
>> -- 
>> 2.17.1
>>
> 

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Dmitry Baryshkov]

On Fri, 15 Nov 2024 at 12:23, Sarthak Garg <quic_sartgarg@quicinc.com> wrote:
>
>
>
> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
> > On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
> >> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
> >> This enables runtime PM for eMMC/SD card.
> >
> > Could you please mention, which platforms were tested with this patch?
> > Note, upstream kernel supports a lot of platforms, including MSM8974, I
> > think the oldest one, which uses SDHCI.
> >
>
> This was tested with qdu1000 platform.

Are you sure that it won't break other platforms?

>
> >>
> >> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> >> ---
> >>   drivers/mmc/host/sdhci-msm.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> >> index e00208535bd1..6657f7db1b8e 100644
> >> --- a/drivers/mmc/host/sdhci-msm.c
> >> +++ b/drivers/mmc/host/sdhci-msm.c
> >> @@ -2626,6 +2626,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> >>              goto clk_disable;
> >>      }
> >>
> >> +    msm_host->mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
> >>      msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
> >>
> >>      /* Set the timeout value to max possible */
> >> --
> >> 2.17.1
> >>
> >



-- 
With best wishes
Dmitry

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg <quic_sartgarg@quicinc.com> wrote:
>>
>>
>>
>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>>>> This enables runtime PM for eMMC/SD card.
>>>
>>> Could you please mention, which platforms were tested with this patch?
>>> Note, upstream kernel supports a lot of platforms, including MSM8974, I
>>> think the oldest one, which uses SDHCI.
>>>
>>
>> This was tested with qdu1000 platform.
> 
> Are you sure that it won't break other platforms?
>

Thanks for your valuable comment.
I am not sure about the older platforms so to avoid issues on older 
platforms we can enable this for all SDCC version 5.0 targets ?

>>
>>>>
>>>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>>>> ---
>>>>    drivers/mmc/host/sdhci-msm.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>>> index e00208535bd1..6657f7db1b8e 100644
>>>> --- a/drivers/mmc/host/sdhci-msm.c
>>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>>> @@ -2626,6 +2626,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>>>               goto clk_disable;
>>>>       }
>>>>
>>>> +    msm_host->mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
>>>>       msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
>>>>
>>>>       /* Set the timeout value to max possible */
>>>> --
>>>> 2.17.1
>>>>
>>>
> 
> 
> 

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Dmitry Baryshkov]

On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
> 
> 
> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
> > On Fri, 15 Nov 2024 at 12:23, Sarthak Garg <quic_sartgarg@quicinc.com> wrote:
> > > 
> > > 
> > > 
> > > On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
> > > > On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
> > > > > Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
> > > > > This enables runtime PM for eMMC/SD card.
> > > > 
> > > > Could you please mention, which platforms were tested with this patch?
> > > > Note, upstream kernel supports a lot of platforms, including MSM8974, I
> > > > think the oldest one, which uses SDHCI.
> > > > 
> > > 
> > > This was tested with qdu1000 platform.
> > 
> > Are you sure that it won't break other platforms?
> > 
> 
> Thanks for your valuable comment.
> I am not sure about the older platforms so to avoid issues on older
> platforms we can enable this for all SDCC version 5.0 targets ?

No, there are still a lot of platforms. Either explain why this is
required for all v5 platforms (and won't break those) or find some other
way, e.g. limit the change to QDU1000, explaining why it is _not_
applicable to other platforms.

-- 
With best wishes
Dmitry

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
> On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
>>
>>
>> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
>>> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg <quic_sartgarg@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
>>>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>>>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>>>>>> This enables runtime PM for eMMC/SD card.
>>>>>
>>>>> Could you please mention, which platforms were tested with this patch?
>>>>> Note, upstream kernel supports a lot of platforms, including MSM8974, I
>>>>> think the oldest one, which uses SDHCI.
>>>>>
>>>>
>>>> This was tested with qdu1000 platform.
>>>
>>> Are you sure that it won't break other platforms?
>>>
>>
>> Thanks for your valuable comment.
>> I am not sure about the older platforms so to avoid issues on older
>> platforms we can enable this for all SDCC version 5.0 targets ?
> 
> No, there are still a lot of platforms. Either explain why this is
> required for all v5 platforms (and won't break those) or find some other
> way, e.g. limit the change to QDU1000, explaining why it is _not_
> applicable to other platforms.
> 

Thanks for your comment.
I agree with your concern but for me also its not possible to test on 
all the platforms.
Lets say if I want to enable this caps for QDU1000 for which it has been 
tested and on any other upcoming target after testing, then how can I 
proceed to enable?

One option I had thought of was to implement this using compatible 
string, then for all the upcoming platforms using this compatible string 
as a fallback.
But this doesn't look optimal to use compatible string for just one flag 
and also this capability is not platform specific and we will be needing 
for all the platforms. Please share your opinion on this.

Another option that I could have thought of is using device tree based 
approach but seems that was not accepted earlier :
https://patchwork.kernel.org/project/linux-mmc/patch/20230129023630.830764-1-chenhuiz@axis.com/

So it would be helpful if you can suggest some approach?

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Dmitry Baryshkov]

On 21/05/2025 17:35, Sarthak Garg wrote:
> 
> 
> On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
>> On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
>>>
>>>
>>> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
>>>> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg 
>>>> <quic_sartgarg@quicinc.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
>>>>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>>>>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>>>>>>> This enables runtime PM for eMMC/SD card.
>>>>>>
>>>>>> Could you please mention, which platforms were tested with this 
>>>>>> patch?
>>>>>> Note, upstream kernel supports a lot of platforms, including 
>>>>>> MSM8974, I
>>>>>> think the oldest one, which uses SDHCI.
>>>>>>
>>>>>
>>>>> This was tested with qdu1000 platform.
>>>>
>>>> Are you sure that it won't break other platforms?
>>>>
>>>
>>> Thanks for your valuable comment.
>>> I am not sure about the older platforms so to avoid issues on older
>>> platforms we can enable this for all SDCC version 5.0 targets ?
>>
>> No, there are still a lot of platforms. Either explain why this is
>> required for all v5 platforms (and won't break those) or find some other
>> way, e.g. limit the change to QDU1000, explaining why it is _not_
>> applicable to other platforms.
>>
> 
> Thanks for your comment.

No need to.

> I agree with your concern but for me also its not possible to test on 
> all the platforms.

Sure.

> Lets say if I want to enable this caps for QDU1000 for which it has been 
> tested and on any other upcoming target after testing, then how can I 
> proceed to enable?

Let's start from the beginning: why do you want to enable it on QDU1000?

> 
> One option I had thought of was to implement this using compatible 
> string, then for all the upcoming platforms using this compatible string 
> as a fallback.
> But this doesn't look optimal to use compatible string for just one flag 
> and also this capability is not platform specific and we will be needing 
> for all the platforms. Please share your opinion on this.
> 
> Another option that I could have thought of is using device tree based 
> approach but seems that was not accepted earlier :
> https://patchwork.kernel.org/project/linux-mmc/ 
> patch/20230129023630.830764-1-chenhuiz@axis.com/
> 
> So it would be helpful if you can suggest some approach?

Worst case, just tie it to the SoC-specific compat string that is 
already a part of the bindings.


-- 
With best wishes
Dmitry

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

On 5/21/2025 8:19 PM, Dmitry Baryshkov wrote:
> On 21/05/2025 17:35, Sarthak Garg wrote:
>>
>>
>> On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
>>> On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
>>>>
>>>>
>>>> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
>>>>> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg 
>>>>> <quic_sartgarg@quicinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
>>>>>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>>>>>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>>>>>>>> This enables runtime PM for eMMC/SD card.
>>>>>>>
>>>>>>> Could you please mention, which platforms were tested with this 
>>>>>>> patch?
>>>>>>> Note, upstream kernel supports a lot of platforms, including 
>>>>>>> MSM8974, I
>>>>>>> think the oldest one, which uses SDHCI.
>>>>>>>
>>>>>>
>>>>>> This was tested with qdu1000 platform.
>>>>>
>>>>> Are you sure that it won't break other platforms?
>>>>>
>>>>
>>>> Thanks for your valuable comment.
>>>> I am not sure about the older platforms so to avoid issues on older
>>>> platforms we can enable this for all SDCC version 5.0 targets ?
>>>
>>> No, there are still a lot of platforms. Either explain why this is
>>> required for all v5 platforms (and won't break those) or find some other
>>> way, e.g. limit the change to QDU1000, explaining why it is _not_
>>> applicable to other platforms.
>>>
>>
>> Thanks for your comment.
> 
> No need to.
>  >> I agree with your concern but for me also its not possible to test on
>> all the platforms.
> 
> Sure.
> >> Lets say if I want to enable this caps for QDU1000 for which it has
>> been tested and on any other upcoming target after testing, then how 
>> can I proceed to enable?
> 
> Let's start from the beginning: why do you want to enable it on QDU1000?
> 

QDU1000 is one latest available target where we have enabled this and 
tested. This has been enabled to save power.

>>
>> One option I had thought of was to implement this using compatible 
>> string, then for all the upcoming platforms using this compatible 
>> string as a fallback.
>> But this doesn't look optimal to use compatible string for just one 
>> flag and also this capability is not platform specific and we will be 
>> needing for all the platforms. Please share your opinion on this.
>>
>> Another option that I could have thought of is using device tree based 
>> approach but seems that was not accepted earlier :
>> https://patchwork.kernel.org/project/linux-mmc/ 
>> patch/20230129023630.830764-1-chenhuiz@axis.com/
>>
>> So it would be helpful if you can suggest some approach?
> 
> Worst case, just tie it to the SoC-specific compat string that is 
> already a part of the bindings.
> 
> 

Sure will try to explore better solution before going for the worst case.

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Dmitry Baryshkov]

On 21/05/2025 18:36, Sarthak Garg wrote:
> 
> 
> On 5/21/2025 8:19 PM, Dmitry Baryshkov wrote:
>> On 21/05/2025 17:35, Sarthak Garg wrote:
>>>
>>>
>>> On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
>>>> On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
>>>>>
>>>>>
>>>>> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
>>>>>> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg 
>>>>>> <quic_sartgarg@quicinc.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>>>>>>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>>>>>>>>> This enables runtime PM for eMMC/SD card.
>>>>>>>>
>>>>>>>> Could you please mention, which platforms were tested with this 
>>>>>>>> patch?
>>>>>>>> Note, upstream kernel supports a lot of platforms, including 
>>>>>>>> MSM8974, I
>>>>>>>> think the oldest one, which uses SDHCI.
>>>>>>>>
>>>>>>>
>>>>>>> This was tested with qdu1000 platform.
>>>>>>
>>>>>> Are you sure that it won't break other platforms?
>>>>>>
>>>>>
>>>>> Thanks for your valuable comment.
>>>>> I am not sure about the older platforms so to avoid issues on older
>>>>> platforms we can enable this for all SDCC version 5.0 targets ?
>>>>
>>>> No, there are still a lot of platforms. Either explain why this is
>>>> required for all v5 platforms (and won't break those) or find some 
>>>> other
>>>> way, e.g. limit the change to QDU1000, explaining why it is _not_
>>>> applicable to other platforms.
>>>>
>>>
>>> Thanks for your comment.
>>
>> No need to.
>>  >> I agree with your concern but for me also its not possible to test on
>>> all the platforms.
>>
>> Sure.
>> >> Lets say if I want to enable this caps for QDU1000 for which it has
>>> been tested and on any other upcoming target after testing, then how 
>>> can I proceed to enable?
>>
>> Let's start from the beginning: why do you want to enable it on QDU1000?
>>
> 
> QDU1000 is one latest available target where we have enabled this and 
> tested. This has been enabled to save power.

Isn't it a powered device? How much power is the save? Is it worth it?

-- 
With best wishes
Dmitry

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Ulf Hansson]

On Wed, 21 May 2025 at 17:41, Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On 21/05/2025 18:36, Sarthak Garg wrote:
> >
> >
> > On 5/21/2025 8:19 PM, Dmitry Baryshkov wrote:
> >> On 21/05/2025 17:35, Sarthak Garg wrote:
> >>>
> >>>
> >>> On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
> >>>> On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
> >>>>>
> >>>>>
> >>>>> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
> >>>>>> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg
> >>>>>> <quic_sartgarg@quicinc.com> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
> >>>>>>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
> >>>>>>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
> >>>>>>>>> This enables runtime PM for eMMC/SD card.
> >>>>>>>>
> >>>>>>>> Could you please mention, which platforms were tested with this
> >>>>>>>> patch?
> >>>>>>>> Note, upstream kernel supports a lot of platforms, including
> >>>>>>>> MSM8974, I
> >>>>>>>> think the oldest one, which uses SDHCI.
> >>>>>>>>
> >>>>>>>
> >>>>>>> This was tested with qdu1000 platform.
> >>>>>>
> >>>>>> Are you sure that it won't break other platforms?
> >>>>>>
> >>>>>
> >>>>> Thanks for your valuable comment.
> >>>>> I am not sure about the older platforms so to avoid issues on older
> >>>>> platforms we can enable this for all SDCC version 5.0 targets ?
> >>>>
> >>>> No, there are still a lot of platforms. Either explain why this is
> >>>> required for all v5 platforms (and won't break those) or find some
> >>>> other
> >>>> way, e.g. limit the change to QDU1000, explaining why it is _not_
> >>>> applicable to other platforms.
> >>>>
> >>>
> >>> Thanks for your comment.
> >>
> >> No need to.
> >>  >> I agree with your concern but for me also its not possible to test on
> >>> all the platforms.
> >>
> >> Sure.
> >> >> Lets say if I want to enable this caps for QDU1000 for which it has
> >>> been tested and on any other upcoming target after testing, then how
> >>> can I proceed to enable?
> >>
> >> Let's start from the beginning: why do you want to enable it on QDU1000?
> >>
> >
> > QDU1000 is one latest available target where we have enabled this and
> > tested. This has been enabled to save power.
>
> Isn't it a powered device? How much power is the save? Is it worth it?

Just wanted to share my view around this, in a slightly more generic
way. My answer to the above, would be, yes, for any battery driven
platform, it should be worth it.

Unfortunately, I don't have any fresh numbers to share for eMMC/SD,
but just searching for some vendor specific information about their
eMMC/SD cards, should tell us I think. In fact, this problem isn't
even limited to eMMC/SD, but rather applies to most flash based
storage (UFS/NVMe etc) that are used on these types of platforms.

How much there is to gain, obviously depends on the internal behaviour
of the storage device. Of course, the number of cards being attached
is important too.

That said, enabling this feature (MMC_CAP_AGGRESSIVE_PM) needs to be
done by taking into account that being *too* aggressive (too
frequently) with turning off the power to the card, may cause a
potential wear-out/brake of the card if we end up preventing it from
doing internal house-keeping for too long.

The current default autosuspend timeout
(pm_runtime_set_autosuspend_delay()) is set to 3s in mmc_blk_probe().
That seems way too aggressive in my opinion, so perhaps increasing
that value to ~180s could allow us to enable this, even if 180s is
just a guesstimate from my side.

Also note that, during system wide suspend we always turn off the
power to the card - and we really don't know if that is too frequent
too. It depends on how the platform is used, compare a laptop with a
smartphone, the frequency greatly differs.

Kind regards
Uffe

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

On 5/27/2025 8:50 PM, Ulf Hansson wrote:
> On Wed, 21 May 2025 at 17:41, Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
>>
>> On 21/05/2025 18:36, Sarthak Garg wrote:
>>>
>>>
>>> On 5/21/2025 8:19 PM, Dmitry Baryshkov wrote:
>>>> On 21/05/2025 17:35, Sarthak Garg wrote:
>>>>>
>>>>>
>>>>> On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
>>>>>> On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg
>>>>>>>> <quic_sartgarg@quicinc.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
>>>>>>>>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>>>>>>>>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>>>>>>>>>>> This enables runtime PM for eMMC/SD card.
>>>>>>>>>>
>>>>>>>>>> Could you please mention, which platforms were tested with this
>>>>>>>>>> patch?
>>>>>>>>>> Note, upstream kernel supports a lot of platforms, including
>>>>>>>>>> MSM8974, I
>>>>>>>>>> think the oldest one, which uses SDHCI.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This was tested with qdu1000 platform.
>>>>>>>>
>>>>>>>> Are you sure that it won't break other platforms?
>>>>>>>>
>>>>>>>
>>>>>>> Thanks for your valuable comment.
>>>>>>> I am not sure about the older platforms so to avoid issues on older
>>>>>>> platforms we can enable this for all SDCC version 5.0 targets ?
>>>>>>
>>>>>> No, there are still a lot of platforms. Either explain why this is
>>>>>> required for all v5 platforms (and won't break those) or find some
>>>>>> other
>>>>>> way, e.g. limit the change to QDU1000, explaining why it is _not_
>>>>>> applicable to other platforms.
>>>>>>
>>>>>
>>>>> Thanks for your comment.
>>>>
>>>> No need to.
>>>>   >> I agree with your concern but for me also its not possible to test on
>>>>> all the platforms.
>>>>
>>>> Sure.
>>>>>> Lets say if I want to enable this caps for QDU1000 for which it has
>>>>> been tested and on any other upcoming target after testing, then how
>>>>> can I proceed to enable?
>>>>
>>>> Let's start from the beginning: why do you want to enable it on QDU1000?
>>>>
>>>
>>> QDU1000 is one latest available target where we have enabled this and
>>> tested. This has been enabled to save power.
>>
>> Isn't it a powered device? How much power is the save? Is it worth it?
> 
> Just wanted to share my view around this, in a slightly more generic
> way. My answer to the above, would be, yes, for any battery driven
> platform, it should be worth it.
> 
> Unfortunately, I don't have any fresh numbers to share for eMMC/SD,
> but just searching for some vendor specific information about their
> eMMC/SD cards, should tell us I think. In fact, this problem isn't
> even limited to eMMC/SD, but rather applies to most flash based
> storage (UFS/NVMe etc) that are used on these types of platforms.
> 
> How much there is to gain, obviously depends on the internal behaviour
> of the storage device. Of course, the number of cards being attached
> is important too.
> 
> That said, enabling this feature (MMC_CAP_AGGRESSIVE_PM) needs to be
> done by taking into account that being *too* aggressive (too
> frequently) with turning off the power to the card, may cause a
> potential wear-out/brake of the card if we end up preventing it from
> doing internal house-keeping for too long.
> 
> The current default autosuspend timeout
> (pm_runtime_set_autosuspend_delay()) is set to 3s in mmc_blk_probe().
> That seems way too aggressive in my opinion, so perhaps increasing
> that value to ~180s could allow us to enable this, even if 180s is
> just a guesstimate from my side.
> 
> Also note that, during system wide suspend we always turn off the
> power to the card - and we really don't know if that is too frequent
> too. It depends on how the platform is used, compare a laptop with a
> smartphone, the frequency greatly differs.
> 
> Kind regards
> Uffe


Hi ulf,

We already have AGGRESSIVE_PM enabled for all our internal targets and 
they are in production for quite long time (5-6 years) and haven't seen 
and performance degradations due to garbage collection. Also wear tears 
are not observed as per my current observations so far.

Like you rightly mentioned we may have battery powered devices where 
this caps will be useful and on the other hand we have always powered 
devices where this caps may not be needed, so can we make device tree 
changes and enable this PM property per board selectively?

Also I see there are four below vendor files who have already enabled 
this caps in their vendor file.

sdhci-pci-core.c sdhci-omap.c sdhci-acpi.c rtsx_pci_sdmmc.c

Let me know how can we proceed.

Regards,
Sarthak

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Dmitry Baryshkov]

On Thu, Jul 24, 2025 at 05:12:56PM +0530, Sarthak Garg wrote:
> 
> 
> On 5/27/2025 8:50 PM, Ulf Hansson wrote:
> > On Wed, 21 May 2025 at 17:41, Dmitry Baryshkov
> > <dmitry.baryshkov@oss.qualcomm.com> wrote:
> > > 
> > > On 21/05/2025 18:36, Sarthak Garg wrote:
> > > > 
> > > > 
> > > > On 5/21/2025 8:19 PM, Dmitry Baryshkov wrote:
> > > > > On 21/05/2025 17:35, Sarthak Garg wrote:
> > > > > > 
> > > > > > 
> > > > > > On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
> > > > > > > On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
> > > > > > > > > On Fri, 15 Nov 2024 at 12:23, Sarthak Garg
> > > > > > > > > <quic_sartgarg@quicinc.com> wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
> > > > > > > > > > > On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
> > > > > > > > > > > > Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
> > > > > > > > > > > > This enables runtime PM for eMMC/SD card.
> > > > > > > > > > > 
> > > > > > > > > > > Could you please mention, which platforms were tested with this
> > > > > > > > > > > patch?
> > > > > > > > > > > Note, upstream kernel supports a lot of platforms, including
> > > > > > > > > > > MSM8974, I
> > > > > > > > > > > think the oldest one, which uses SDHCI.
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > This was tested with qdu1000 platform.
> > > > > > > > > 
> > > > > > > > > Are you sure that it won't break other platforms?
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > Thanks for your valuable comment.
> > > > > > > > I am not sure about the older platforms so to avoid issues on older
> > > > > > > > platforms we can enable this for all SDCC version 5.0 targets ?
> > > > > > > 
> > > > > > > No, there are still a lot of platforms. Either explain why this is
> > > > > > > required for all v5 platforms (and won't break those) or find some
> > > > > > > other
> > > > > > > way, e.g. limit the change to QDU1000, explaining why it is _not_
> > > > > > > applicable to other platforms.
> > > > > > > 
> > > > > > 
> > > > > > Thanks for your comment.
> > > > > 
> > > > > No need to.
> > > > >   >> I agree with your concern but for me also its not possible to test on
> > > > > > all the platforms.
> > > > > 
> > > > > Sure.
> > > > > > > Lets say if I want to enable this caps for QDU1000 for which it has
> > > > > > been tested and on any other upcoming target after testing, then how
> > > > > > can I proceed to enable?
> > > > > 
> > > > > Let's start from the beginning: why do you want to enable it on QDU1000?
> > > > > 
> > > > 
> > > > QDU1000 is one latest available target where we have enabled this and
> > > > tested. This has been enabled to save power.
> > > 
> > > Isn't it a powered device? How much power is the save? Is it worth it?
> > 
> > Just wanted to share my view around this, in a slightly more generic
> > way. My answer to the above, would be, yes, for any battery driven
> > platform, it should be worth it.
> > 
> > Unfortunately, I don't have any fresh numbers to share for eMMC/SD,
> > but just searching for some vendor specific information about their
> > eMMC/SD cards, should tell us I think. In fact, this problem isn't
> > even limited to eMMC/SD, but rather applies to most flash based
> > storage (UFS/NVMe etc) that are used on these types of platforms.
> > 
> > How much there is to gain, obviously depends on the internal behaviour
> > of the storage device. Of course, the number of cards being attached
> > is important too.
> > 
> > That said, enabling this feature (MMC_CAP_AGGRESSIVE_PM) needs to be
> > done by taking into account that being *too* aggressive (too
> > frequently) with turning off the power to the card, may cause a
> > potential wear-out/brake of the card if we end up preventing it from
> > doing internal house-keeping for too long.
> > 
> > The current default autosuspend timeout
> > (pm_runtime_set_autosuspend_delay()) is set to 3s in mmc_blk_probe().
> > That seems way too aggressive in my opinion, so perhaps increasing
> > that value to ~180s could allow us to enable this, even if 180s is
> > just a guesstimate from my side.
> > 
> > Also note that, during system wide suspend we always turn off the
> > power to the card - and we really don't know if that is too frequent
> > too. It depends on how the platform is used, compare a laptop with a
> > smartphone, the frequency greatly differs.
> > 
> > Kind regards
> > Uffe
> 
> 
> Hi ulf,
> 
> We already have AGGRESSIVE_PM enabled for all our internal targets and they
> are in production for quite long time (5-6 years) and haven't seen and

Upstream kernels support targets starting from APQ8060, so they are much
older than 5-6 years. For example, I'd like to point out several SDHCI
v4 targets (MSM8974 - MSM8998).

> performance degradations due to garbage collection. Also wear tears are not
> observed as per my current observations so far.
> 
> Like you rightly mentioned we may have battery powered devices where this
> caps will be useful and on the other hand we have always powered devices
> where this caps may not be needed, so can we make device tree changes and
> enable this PM property per board selectively?
> 
> Also I see there are four below vendor files who have already enabled this
> caps in their vendor file.
> 
> sdhci-pci-core.c sdhci-omap.c sdhci-acpi.c rtsx_pci_sdmmc.c
> 
> Let me know how can we proceed.
> 
> Regards,
> Sarthak
> 
> 
> 
> 
> 

-- 
With best wishes
Dmitry

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

On 5/21/2025 9:11 PM, Dmitry Baryshkov wrote:
> On 21/05/2025 18:36, Sarthak Garg wrote:
>>
>>
>> On 5/21/2025 8:19 PM, Dmitry Baryshkov wrote:
>>> On 21/05/2025 17:35, Sarthak Garg wrote:
>>>>
>>>>
>>>> On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
>>>>>>
>>>>>>
>>>>>> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
>>>>>>> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg 
>>>>>>> <quic_sartgarg@quicinc.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
>>>>>>>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>>>>>>>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>>>>>>>>>> This enables runtime PM for eMMC/SD card.
>>>>>>>>>
>>>>>>>>> Could you please mention, which platforms were tested with this 
>>>>>>>>> patch?
>>>>>>>>> Note, upstream kernel supports a lot of platforms, including 
>>>>>>>>> MSM8974, I
>>>>>>>>> think the oldest one, which uses SDHCI.
>>>>>>>>>
>>>>>>>>
>>>>>>>> This was tested with qdu1000 platform.
>>>>>>>
>>>>>>> Are you sure that it won't break other platforms?
>>>>>>>
>>>>>>
>>>>>> Thanks for your valuable comment.
>>>>>> I am not sure about the older platforms so to avoid issues on older
>>>>>> platforms we can enable this for all SDCC version 5.0 targets ?
>>>>>
>>>>> No, there are still a lot of platforms. Either explain why this is
>>>>> required for all v5 platforms (and won't break those) or find some 
>>>>> other
>>>>> way, e.g. limit the change to QDU1000, explaining why it is _not_
>>>>> applicable to other platforms.
>>>>>
>>>>
>>>> Thanks for your comment.
>>>
>>> No need to.
>>>  >> I agree with your concern but for me also its not possible to 
>>> test on
>>>> all the platforms.
>>>
>>> Sure.
>>> >> Lets say if I want to enable this caps for QDU1000 for which it has
>>>> been tested and on any other upcoming target after testing, then how 
>>>> can I proceed to enable?
>>>
>>> Let's start from the beginning: why do you want to enable it on QDU1000?
>>>
>>
>> QDU1000 is one latest available target where we have enabled this and 
>> tested. This has been enabled to save power.
> 
> Isn't it a powered device? How much power is the save? Is it worth it?
> 

Sorry I just did basic sanity on QDU1000 device to confirm its not 
breaking any eMMC functionality and we have also tested SD card on 
SM8550 as well.
For power no's we have stared internal discussions and based on target 
available for power profiling with eMMC device we will come back.

Regards,
Sarthak

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Dmitry Baryshkov]

On Thu, Jul 24, 2025 at 04:45:38PM +0530, Sarthak Garg wrote:
> 
> 
> On 5/21/2025 9:11 PM, Dmitry Baryshkov wrote:
> > On 21/05/2025 18:36, Sarthak Garg wrote:
> > > 
> > > 
> > > On 5/21/2025 8:19 PM, Dmitry Baryshkov wrote:
> > > > On 21/05/2025 17:35, Sarthak Garg wrote:
> > > > > 
> > > > > 
> > > > > On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
> > > > > > On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
> > > > > > > > On Fri, 15 Nov 2024 at 12:23, Sarthak Garg
> > > > > > > > <quic_sartgarg@quicinc.com> wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
> > > > > > > > > > On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
> > > > > > > > > > > Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
> > > > > > > > > > > This enables runtime PM for eMMC/SD card.
> > > > > > > > > > 
> > > > > > > > > > Could you please mention, which
> > > > > > > > > > platforms were tested with this patch?
> > > > > > > > > > Note, upstream kernel supports a lot of
> > > > > > > > > > platforms, including MSM8974, I
> > > > > > > > > > think the oldest one, which uses SDHCI.
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > This was tested with qdu1000 platform.
> > > > > > > > 
> > > > > > > > Are you sure that it won't break other platforms?
> > > > > > > > 
> > > > > > > 
> > > > > > > Thanks for your valuable comment.
> > > > > > > I am not sure about the older platforms so to avoid issues on older
> > > > > > > platforms we can enable this for all SDCC version 5.0 targets ?
> > > > > > 
> > > > > > No, there are still a lot of platforms. Either explain why this is
> > > > > > required for all v5 platforms (and won't break those) or
> > > > > > find some other
> > > > > > way, e.g. limit the change to QDU1000, explaining why it is _not_
> > > > > > applicable to other platforms.
> > > > > > 
> > > > > 
> > > > > Thanks for your comment.
> > > > 
> > > > No need to.
> > > >  >> I agree with your concern but for me also its not possible
> > > > to test on
> > > > > all the platforms.
> > > > 
> > > > Sure.
> > > > >> Lets say if I want to enable this caps for QDU1000 for which it has
> > > > > been tested and on any other upcoming target after testing,
> > > > > then how can I proceed to enable?
> > > > 
> > > > Let's start from the beginning: why do you want to enable it on QDU1000?
> > > > 
> > > 
> > > QDU1000 is one latest available target where we have enabled this
> > > and tested. This has been enabled to save power.
> > 
> > Isn't it a powered device? How much power is the save? Is it worth it?
> > 
> 
> Sorry I just did basic sanity on QDU1000 device to confirm its not breaking
> any eMMC functionality and we have also tested SD card on SM8550 as well.
> For power no's we have stared internal discussions and based on target
> available for power profiling with eMMC device we will come back.

So, again, _why_ do we want to enable it? If you haven't measured the
actual power savings, then it's obviously not a primary reason.

As for the v5 targets only, they start from SDM845. Have you tested it?
Does it bring any actual benefits?

-- 
With best wishes
Dmitry

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

On 7/24/2025 5:31 PM, Dmitry Baryshkov wrote:
> On Thu, Jul 24, 2025 at 04:45:38PM +0530, Sarthak Garg wrote:
>>
>>
>> On 5/21/2025 9:11 PM, Dmitry Baryshkov wrote:
>>> On 21/05/2025 18:36, Sarthak Garg wrote:
>>>>
>>>>
>>>> On 5/21/2025 8:19 PM, Dmitry Baryshkov wrote:
>>>>> On 21/05/2025 17:35, Sarthak Garg wrote:
>>>>>>
>>>>>>
>>>>>> On 5/21/2025 6:25 PM, Dmitry Baryshkov wrote:
>>>>>>> On Wed, May 21, 2025 at 12:46:49PM +0530, Sarthak Garg wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/15/2024 6:53 PM, Dmitry Baryshkov wrote:
>>>>>>>>> On Fri, 15 Nov 2024 at 12:23, Sarthak Garg
>>>>>>>>> <quic_sartgarg@quicinc.com> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 11/4/2024 4:19 PM, Dmitry Baryshkov wrote:
>>>>>>>>>>> On Mon, Nov 04, 2024 at 11:37:22AM +0530, Sarthak Garg wrote:
>>>>>>>>>>>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>>>>>>>>>>>> This enables runtime PM for eMMC/SD card.
>>>>>>>>>>>
>>>>>>>>>>> Could you please mention, which
>>>>>>>>>>> platforms were tested with this patch?
>>>>>>>>>>> Note, upstream kernel supports a lot of
>>>>>>>>>>> platforms, including MSM8974, I
>>>>>>>>>>> think the oldest one, which uses SDHCI.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This was tested with qdu1000 platform.
>>>>>>>>>
>>>>>>>>> Are you sure that it won't break other platforms?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for your valuable comment.
>>>>>>>> I am not sure about the older platforms so to avoid issues on older
>>>>>>>> platforms we can enable this for all SDCC version 5.0 targets ?
>>>>>>>
>>>>>>> No, there are still a lot of platforms. Either explain why this is
>>>>>>> required for all v5 platforms (and won't break those) or
>>>>>>> find some other
>>>>>>> way, e.g. limit the change to QDU1000, explaining why it is _not_
>>>>>>> applicable to other platforms.
>>>>>>>
>>>>>>
>>>>>> Thanks for your comment.
>>>>>
>>>>> No need to.
>>>>>   >> I agree with your concern but for me also its not possible
>>>>> to test on
>>>>>> all the platforms.
>>>>>
>>>>> Sure.
>>>>>>> Lets say if I want to enable this caps for QDU1000 for which it has
>>>>>> been tested and on any other upcoming target after testing,
>>>>>> then how can I proceed to enable?
>>>>>
>>>>> Let's start from the beginning: why do you want to enable it on QDU1000?
>>>>>
>>>>
>>>> QDU1000 is one latest available target where we have enabled this
>>>> and tested. This has been enabled to save power.
>>>
>>> Isn't it a powered device? How much power is the save? Is it worth it?
>>>
>>
>> Sorry I just did basic sanity on QDU1000 device to confirm its not breaking
>> any eMMC functionality and we have also tested SD card on SM8550 as well.
>> For power no's we have stared internal discussions and based on target
>> available for power profiling with eMMC device we will come back.
> 
> So, again, _why_ do we want to enable it? If you haven't measured the
> actual power savings, then it's obviously not a primary reason.
> 
> As for the v5 targets only, they start from SDM845. Have you tested it?
> Does it bring any actual benefits?
> 

Sure will capture the power savings on our target with runtime PM and 
will come back.
In our downstream code its enabled for all our targets and tested for 
quite some time.


~Regards
Sarthak

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Ulf Hansson]

On Mon, 4 Nov 2024 at 07:07, Sarthak Garg <quic_sartgarg@quicinc.com> wrote:
>
> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
> This enables runtime PM for eMMC/SD card.
>
> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>

In general I think using MMC_CAP_AGGRESSIVE_PM needs to be carefully
selected. I am not saying it's a bad idea to use it, but the commit
message above kind of indicates that this has only been enabled to
make sure we avoid wasting energy at any cost. Maybe I am wrong?

Today the default autosuspend timeout is set to 3000 ms, which means
that beyond this idle-period the card internally will no longer be
able to manage "garbage collect". For a poorly behaving SD card, for
example, that could hurt future read/writes. Or maybe that isn't such
a big problem after all?

Also note that userspace via sysfs is able to change the autosuspend
timeout and even disable runtime PM for the card, if that is needed.

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-msm.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index e00208535bd1..6657f7db1b8e 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -2626,6 +2626,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>                 goto clk_disable;
>         }
>
> +       msm_host->mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
>         msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
>
>         /* Set the timeout value to max possible */
> --
> 2.17.1
>

Re: [PATCH V1] mmc: sdhci-msm: Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers

[Sarthak Garg]

On 11/15/2024 4:28 PM, Ulf Hansson wrote:
> On Mon, 4 Nov 2024 at 07:07, Sarthak Garg <quic_sartgarg@quicinc.com> wrote:
>>
>> Enable MMC_CAP_AGGRESSIVE_PM for qualcomm controllers.
>> This enables runtime PM for eMMC/SD card.
>>
>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> 
> In general I think using MMC_CAP_AGGRESSIVE_PM needs to be carefully
> selected. I am not saying it's a bad idea to use it, but the commit
> message above kind of indicates that this has only been enabled to
> make sure we avoid wasting energy at any cost. Maybe I am wrong?
> 
> Today the default autosuspend timeout is set to 3000 ms, which means
> that beyond this idle-period the card internally will no longer be
> able to manage "garbage collect". For a poorly behaving SD card, for
> example, that could hurt future read/writes. Or maybe that isn't such
> a big problem after all?
> 
> Also note that userspace via sysfs is able to change the autosuspend
> timeout and even disable runtime PM for the card, if that is needed.
> 
> Kind regards
> Uffe
>

Thanks for your valuable comment.
First of all sorry Ulf for the late reply as I was on break and came 
back now.
Yes you are right this has been enabled to make sure we avoid wasting 
power at any cost.
Moreover for a poorly behaving SD card we can't penalize the power 
consumption of all the SOC's so thats we won't to enable this flag.

>> ---
>>   drivers/mmc/host/sdhci-msm.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index e00208535bd1..6657f7db1b8e 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -2626,6 +2626,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>                  goto clk_disable;
>>          }
>>
>> +       msm_host->mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
>>          msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
>>
>>          /* Set the timeout value to max possible */
>> --
>> 2.17.1
>>