drivers/mmc/host/sdhci-msm.c | 1 + 1 file changed, 1 insertion(+)
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
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 >
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 >>
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
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 >> >
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
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 >>>> >>> > > >
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
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?
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
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.
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
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
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
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
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
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
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
© 2016 - 2026 Red Hat, Inc.