| Message ID | 20230129023630.830764-1-chenhuiz@axis.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: mmc: Add cap-aggressive-pm property | expand |
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
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
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
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 --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:
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(+)