diff mbox series

dt-bindings: mmc: Add cap-aggressive-pm property

Message ID 20230129023630.830764-1-chenhuiz@axis.com
State New
Headers
Series dt-bindings: mmc: Add cap-aggressive-pm property |

Commit Message

Hermes Zhang Jan. 29, 2023, 2:36 a.m. UTC 
  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(+)

Comments

Krzysztof Kozlowski Jan. 29, 2023, 10:58 a.m. UTC | #1 
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
Krzysztof Kozlowski Jan. 31, 2023, 4:57 p.m. UTC | #2 
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
Ulf Hansson Feb. 2, 2023, 2:59 p.m. UTC | #3 
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
Krzysztof Kozlowski Feb. 2, 2023, 4:25 p.m. UTC | #4 
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
diff mbox series

Patch

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: