[PATCH] dt-bindings: mmc: Add cap-aggressive-pm property

[PATCH] dt-bindings: mmc: Add cap-aggressive-pm property

[Hermes Zhang]

This commit add a new property: cap-aggressive-pm to enable the
MMC_CAP_AGGRESSIVE_PM feature for (e)MMC/SD power saving.

Signed-off-by: Hermes Zhang <chenhuiz@axis.com>
---
 Documentation/devicetree/bindings/mmc/mmc-controller.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
index 86c73fd825fd..7ca674263dba 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
+++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
@@ -177,6 +177,11 @@ properties:
     description:
       enable SDIO IRQ signalling on this interface
 
+  cap-aggressive-pm:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      enable MMC_CAP_AGGRESSIVE_PM feature
+
   full-pwr-cycle:
     $ref: /schemas/types.yaml#/definitions/flag
     description:
-- 
2.30.2

Re: [PATCH] dt-bindings: mmc: Add cap-aggressive-pm property

[Krzysztof Kozlowski]

On 29/01/2023 03:36, Hermes Zhang wrote:
> This commit add a new property: cap-aggressive-pm to enable the

Do not use "This commit/patch".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> MMC_CAP_AGGRESSIVE_PM feature for (e)MMC/SD power saving.

Why this is a property suitable for DT? IOW, why this isn't enabled always?

> 
> Signed-off-by: Hermes Zhang <chenhuiz@axis.com>
> ---
>  Documentation/devicetree/bindings/mmc/mmc-controller.yaml | 5 +++++
>  1 file changed, 5 insertions(+)

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: mmc: Add cap-aggressive-pm property

[Krzysztof Kozlowski]

On 30/01/2023 07:54, Hermes Zhang wrote:
> On 2023/1/29 18:58, Krzysztof Kozlowski wrote:
>> On 29/01/2023 03:36, Hermes Zhang wrote:
>>> This commit add a new property: cap-aggressive-pm to enable the
>> Do not use "This commit/patch".
>> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> 
> Done
> 
>>> MMC_CAP_AGGRESSIVE_PM feature for (e)MMC/SD power saving.
>> Why this is a property suitable for DT? IOW, why this isn't enabled always?
> 
> This property will benfit for the power consumption, but it also may 
> degradation in performance as it will prevent the
> 
> the card from executing internal house-keeping operations in idle mode. 
> So it's better to config it from DT.

Why? DT is not for policy. How you described it, this is policy or
system tuning choice thus the job for Linux (OS), not for DT. So I will
repeat - why this property fits the purpose of DT (describe the hardware).

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: mmc: Add cap-aggressive-pm property

[Ulf Hansson]

On Tue, 31 Jan 2023 at 17:57, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 30/01/2023 07:54, Hermes Zhang wrote:
> > On 2023/1/29 18:58, Krzysztof Kozlowski wrote:
> >> On 29/01/2023 03:36, Hermes Zhang wrote:
> >>> This commit add a new property: cap-aggressive-pm to enable the
> >> Do not use "This commit/patch".
> >> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> >
> > Done
> >
> >>> MMC_CAP_AGGRESSIVE_PM feature for (e)MMC/SD power saving.
> >> Why this is a property suitable for DT? IOW, why this isn't enabled always?
> >
> > This property will benfit for the power consumption, but it also may
> > degradation in performance as it will prevent the
> >
> > the card from executing internal house-keeping operations in idle mode.
> > So it's better to config it from DT.
>
> Why? DT is not for policy. How you described it, this is policy or
> system tuning choice thus the job for Linux (OS), not for DT. So I will
> repeat - why this property fits the purpose of DT (describe the hardware).
>

I guess the HW perspective here, is that it might not fit all
platforms nor the actual eMMC/SD card to support this feature.
However, it still seems like a policy rather than a strict HW
constraint.

Perhaps there is a way to figure out in the host driver, to
conditionally set the MMC_CAP_AGGRESSIVE_PM for the host, when needed
instead?

Kind regards
Uffe

Re: [PATCH] dt-bindings: mmc: Add cap-aggressive-pm property

[Krzysztof Kozlowski]

On 02/02/2023 15:59, Ulf Hansson wrote:
> On Tue, 31 Jan 2023 at 17:57, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 30/01/2023 07:54, Hermes Zhang wrote:
>>> On 2023/1/29 18:58, Krzysztof Kozlowski wrote:
>>>> On 29/01/2023 03:36, Hermes Zhang wrote:
>>>>> This commit add a new property: cap-aggressive-pm to enable the
>>>> Do not use "This commit/patch".
>>>> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>>>
>>> Done
>>>
>>>>> MMC_CAP_AGGRESSIVE_PM feature for (e)MMC/SD power saving.
>>>> Why this is a property suitable for DT? IOW, why this isn't enabled always?
>>>
>>> This property will benfit for the power consumption, but it also may
>>> degradation in performance as it will prevent the
>>>
>>> the card from executing internal house-keeping operations in idle mode.
>>> So it's better to config it from DT.
>>
>> Why? DT is not for policy. How you described it, this is policy or
>> system tuning choice thus the job for Linux (OS), not for DT. So I will
>> repeat - why this property fits the purpose of DT (describe the hardware).
>>
> 
> I guess the HW perspective here, is that it might not fit all
> platforms nor the actual eMMC/SD card to support this feature.
> However, it still seems like a policy rather than a strict HW
> constraint.
> 
> Perhaps there is a way to figure out in the host driver, to
> conditionally set the MMC_CAP_AGGRESSIVE_PM for the host, when needed
> instead?

What also worries me is that there is no user of this property: no DTS,
no driver, so it is tricky to deduct out when it is applicable.

Anyway things which might be obvious for the submitter, might not be for
the reviewer, thus I would really like to see justification why
different boards (or memories) need this property.

Best regards,
Krzysztof