diff mbox series

[RFC,v2] dt-bindings: arm/msm/qcom,idle-state convert to YAML

Message ID 20210908171453.53259-1-david@ixit.cz (mailing list archive)
State Changes Requested
Headers show
Series [RFC,v2] dt-bindings: arm/msm/qcom,idle-state convert to YAML | expand

Commit Message

David Heidelberg Sept. 8, 2021, 5:14 p.m. UTC
Switched maintainer from Lina to Bjorn.

Doesn't fix:
```
idle-states: 'spc' does not match any of the regexes: '^(cpu|cluster)-', 'pinctrl-[0-9]+'
```
from colliding arm/idle-states.yaml .

Signed-off-by: David Heidelberg <david@ixit.cz>
---

v2
 - maintainer is now Bjorn
 - fixed some newlines
 - specified exact state names (added $)

 .../bindings/arm/msm/qcom,idle-state.txt      |  84 ------------
 .../bindings/arm/msm/qcom,idle-state.yaml     | 126 ++++++++++++++++++
 2 files changed, 126 insertions(+), 84 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml

Comments

Rob Herring (Arm) Sept. 9, 2021, 12:32 p.m. UTC | #1
On Wed, 08 Sep 2021 19:14:53 +0200, David Heidelberg wrote:
> Switched maintainer from Lina to Bjorn.
> 
> Doesn't fix:
> ```
> idle-states: 'spc' does not match any of the regexes: '^(cpu|cluster)-', 'pinctrl-[0-9]+'
> ```
> from colliding arm/idle-states.yaml .
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
> 
> v2
>  - maintainer is now Bjorn
>  - fixed some newlines
>  - specified exact state names (added $)
> 
>  .../bindings/arm/msm/qcom,idle-state.txt      |  84 ------------
>  .../bindings/arm/msm/qcom,idle-state.yaml     | 126 ++++++++++++++++++
>  2 files changed, 126 insertions(+), 84 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml:87:13: [warning] wrong indentation: expected 14 but found 12 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/psci.example.dt.yaml: idle-states: 'cpu-power-down' does not match any of the regexes: '^(ret|spc|pc)$', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/idle-states.example.dt.yaml: idle-states: 'cluster-retention-0', 'cluster-retention-1', 'cluster-sleep-0', 'cluster-sleep-1', 'cpu-retention-0-0', 'cpu-retention-1-0', 'cpu-sleep-0-0', 'cpu-sleep-1-0', 'entry-method' do not match any of the regexes: '^(ret|spc|pc)$', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/idle-states.example.dt.yaml: idle-states: 'cluster-sleep-0', 'cluster-sleep-1', 'cpu-sleep-0-0', 'cpu-sleep-1-0' do not match any of the regexes: '^(ret|spc|pc)$', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.example.dt.yaml: idle-states: 'spc' does not match any of the regexes: '^(cpu|cluster)-', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/idle-states.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1525975

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring (Arm) Sept. 10, 2021, 10:04 p.m. UTC | #2
On Wed, Sep 08, 2021 at 07:14:53PM +0200, David Heidelberg wrote:
> Switched maintainer from Lina to Bjorn.
> 
> Doesn't fix:
> ```
> idle-states: 'spc' does not match any of the regexes: '^(cpu|cluster)-', 'pinctrl-[0-9]+'
> ```
> from colliding arm/idle-states.yaml .

Your options are:

- Drop this and add your node names and compatible strings to 
idle-states.yaml. A variation of this is change the QCom node names 
in dts files to match. Those look like the only real differences.

- Extract the common idle state node properties to a common schema to 
reference from both schemas.

I'd lean towards option 1 unless there are other variations of 
idle-state nodes that also need option 2.

Rob
David Heidelberg Sept. 11, 2021, 4:51 p.m. UTC | #3
Thank you for the input, for now I applied option 1 to my another 
tegra-ehci binding (instead of having own file) and it lead to success, 
so I'll choose option 1 for this case too.
Best regards
David Heidelberg

On Fri, Sep 10 2021 at 17:04:25 -0500, Rob Herring <robh@kernel.org> 
wrote:
> On Wed, Sep 08, 2021 at 07:14:53PM +0200, David Heidelberg wrote:
>>  Switched maintainer from Lina to Bjorn.
>> 
>>  Doesn't fix:
>>  ```
>>  idle-states: 'spc' does not match any of the regexes: 
>> '^(cpu|cluster)-', 'pinctrl-[0-9]+'
>>  ```
>>  from colliding arm/idle-states.yaml .
> 
> Your options are:
> 
> - Drop this and add your node names and compatible strings to
> idle-states.yaml. A variation of this is change the QCom node names
> in dts files to match. Those look like the only real differences.
> 
> - Extract the common idle state node properties to a common schema to
> reference from both schemas.
> 
> I'd lean towards option 1 unless there are other variations of
> idle-state nodes that also need option 2.
> 
> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
deleted file mode 100644
index 6ce0b212ec6d..000000000000
--- a/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
+++ /dev/null
@@ -1,84 +0,0 @@ 
-QCOM Idle States for cpuidle driver
-
-ARM provides idle-state node to define the cpuidle states, as defined in [1].
-cpuidle-qcom is the cpuidle driver for Qualcomm SoCs and uses these idle
-states. Idle states have different enter/exit latency and residency values.
-The idle states supported by the QCOM SoC are defined as -
-
-    * Standby
-    * Retention
-    * Standalone Power Collapse (Standalone PC or SPC)
-    * Power Collapse (PC)
-
-Standby: Standby does a little more in addition to architectural clock gating.
-When the WFI instruction is executed the ARM core would gate its internal
-clocks. In addition to gating the clocks, QCOM cpus use this instruction as a
-trigger to execute the SPM state machine. The SPM state machine waits for the
-interrupt to trigger the core back in to active. This triggers the cache
-hierarchy to enter standby states, when all cpus are idle. An interrupt brings
-the SPM state machine out of its wait, the next step is to ensure that the
-cache hierarchy is also out of standby, and then the cpu is allowed to resume
-execution. This state is defined as a generic ARM WFI state by the ARM cpuidle
-driver and is not defined in the DT. The SPM state machine should be
-configured to execute this state by default and after executing every other
-state below.
-
-Retention: Retention is a low power state where the core is clock gated and
-the memory and the registers associated with the core are retained. The
-voltage may be reduced to the minimum value needed to keep the processor
-registers active. The SPM should be configured to execute the retention
-sequence and would wait for interrupt, before restoring the cpu to execution
-state. Retention may have a slightly higher latency than Standby.
-
-Standalone PC: A cpu can power down and warmboot if there is a sufficient time
-between the time it enters idle and the next known wake up. SPC mode is used
-to indicate a core entering a power down state without consulting any other
-cpu or the system resources. This helps save power only on that core.  The SPM
-sequence for this idle state is programmed to power down the supply to the
-core, wait for the interrupt, restore power to the core, and ensure the
-system state including cache hierarchy is ready before allowing core to
-resume. Applying power and resetting the core causes the core to warmboot
-back into Elevation Level (EL) which trampolines the control back to the
-kernel. Entering a power down state for the cpu, needs to be done by trapping
-into a EL. Failing to do so, would result in a crash enforced by the warm boot
-code in the EL for the SoC. On SoCs with write-back L1 cache, the cache has to
-be flushed in s/w, before powering down the core.
-
-Power Collapse: This state is similar to the SPC mode, but distinguishes
-itself in that the cpu acknowledges and permits the SoC to enter deeper sleep
-modes. In a hierarchical power domain SoC, this means L2 and other caches can
-be flushed, system bus, clocks - lowered, and SoC main XO clock gated and
-voltages reduced, provided all cpus enter this state.  Since the span of low
-power modes possible at this state is vast, the exit latency and the residency
-of this low power mode would be considered high even though at a cpu level,
-this essentially is cpu power down. The SPM in this state also may handshake
-with the Resource power manager (RPM) processor in the SoC to indicate a
-complete application processor subsystem shut down.
-
-The idle-state for QCOM SoCs are distinguished by the compatible property of
-the idle-states device node.
-
-The devicetree representation of the idle state should be -
-
-Required properties:
-
-- compatible: Must be one of -
-			"qcom,idle-state-ret",
-			"qcom,idle-state-spc",
-			"qcom,idle-state-pc",
-		and "arm,idle-state".
-
-Other required and optional properties are specified in [1].
-
-Example:
-
-	idle-states {
-		CPU_SPC: spc {
-			compatible = "qcom,idle-state-spc", "arm,idle-state";
-			entry-latency-us = <150>;
-			exit-latency-us = <200>;
-			min-residency-us = <2000>;
-		};
-	};
-
-[1]. Documentation/devicetree/bindings/arm/idle-states.yaml
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml
new file mode 100644
index 000000000000..254868e09520
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.yaml
@@ -0,0 +1,126 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/msm/qcom,idle-state.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: QCOM Idle States binding description
+
+maintainers:
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+description: |
+  ARM provides idle-state node to define the cpuidle states, as defined in [1].
+  cpuidle-qcom is the cpuidle driver for Qualcomm SoCs and uses these idle
+  states. Idle states have different enter/exit latency and residency values.
+  The idle states supported by the QCOM SoC are defined as -
+
+    * Standby
+    * Retention
+    * Standalone Power Collapse (Standalone PC or SPC)
+    * Power Collapse (PC)
+
+  Standby: Standby does a little more in addition to architectural clock gating.
+  When the WFI instruction is executed the ARM core would gate its internal
+  clocks. In addition to gating the clocks, QCOM cpus use this instruction as a
+  trigger to execute the SPM state machine. The SPM state machine waits for the
+  interrupt to trigger the core back in to active. This triggers the cache
+  hierarchy to enter standby states, when all cpus are idle. An interrupt brings
+  the SPM state machine out of its wait, the next step is to ensure that the
+  cache hierarchy is also out of standby, and then the cpu is allowed to resume
+  execution. This state is defined as a generic ARM WFI state by the ARM cpuidle
+  driver and is not defined in the DT. The SPM state machine should be
+  configured to execute this state by default and after executing every other
+  state below.
+
+  Retention: Retention is a low power state where the core is clock gated and
+  the memory and the registers associated with the core are retained. The
+  voltage may be reduced to the minimum value needed to keep the processor
+  registers active. The SPM should be configured to execute the retention
+  sequence and would wait for interrupt, before restoring the cpu to execution
+  state. Retention may have a slightly higher latency than Standby.
+
+  Standalone PC: A cpu can power down and warmboot if there is a sufficient time
+  between the time it enters idle and the next known wake up. SPC mode is used
+  to indicate a core entering a power down state without consulting any other
+  cpu or the system resources. This helps save power only on that core.  The SPM
+  sequence for this idle state is programmed to power down the supply to the
+  core, wait for the interrupt, restore power to the core, and ensure the
+  system state including cache hierarchy is ready before allowing core to
+  resume. Applying power and resetting the core causes the core to warmboot
+  back into Elevation Level (EL) which trampolines the control back to the
+  kernel. Entering a power down state for the cpu, needs to be done by trapping
+  into a EL. Failing to do so, would result in a crash enforced by the warm boot
+  code in the EL for the SoC. On SoCs with write-back L1 cache, the cache has to
+  be flushed in s/w, before powering down the core.
+
+  Power Collapse: This state is similar to the SPC mode, but distinguishes
+  itself in that the cpu acknowledges and permits the SoC to enter deeper sleep
+  modes. In a hierarchical power domain SoC, this means L2 and other caches can
+  be flushed, system bus, clocks - lowered, and SoC main XO clock gated and
+  voltages reduced, provided all cpus enter this state.  Since the span of low
+  power modes possible at this state is vast, the exit latency and the residency
+  of this low power mode would be considered high even though at a cpu level,
+  this essentially is cpu power down. The SPM in this state also may handshake
+  with the Resource power manager (RPM) processor in the SoC to indicate a
+  complete application processor subsystem shut down.
+
+  The idle-state for QCOM SoCs are distinguished by the compatible property of
+  the idle-states device node.
+
+  [1] Documentation/devicetree/bindings/arm/idle-states.yaml
+
+properties:
+  $nodename:
+    const: idle-states
+
+patternProperties:
+  "^(ret|spc|pc)$":
+    type: object
+    description:
+      Each state node represents a domain idle state description.
+
+    properties:
+      compatible:
+        items:
+          - enum:
+            - qcom,idle-state-ret
+            - qcom,idle-state-spc
+            - qcom,idle-state-pc
+          - const: arm,idle-state
+
+      entry-latency-us:
+        description:
+          The worst case latency in microseconds required to enter the idle
+          state. Note that, the exit-latency-us duration may be guaranteed only
+          after the entry-latency-us has passed.
+
+      exit-latency-us:
+        description:
+          The worst case latency in microseconds required to exit the idle
+          state.
+
+      min-residency-us:
+        description:
+          The minimum residency duration in microseconds after which the idle
+          state will yield power benefits, after overcoming the overhead while
+          entering the idle state.
+
+    required:
+      - compatible
+      - entry-latency-us
+      - exit-latency-us
+      - min-residency-us
+
+additionalProperties: false
+
+examples:
+  - |
+    idle-states {
+      CPU_SPC: spc {
+        compatible = "qcom,idle-state-spc", "arm,idle-state";
+        entry-latency-us = <150>;
+        exit-latency-us = <200>;
+        min-residency-us = <2000>;
+      };
+    };