On Mon, 13 Jan 2020 at 20:53, Rob Herring [off-list ref] wrote:

On Mon, Dec 30, 2019 at 8:44 AM Ulf Hansson [off-list ref] wrote:

quoted

Update PSCI DT bindings to allow to represent idle states for CPUs and the
CPU topology, by using a hierarchical layout. Primarily this is done by
re-using the existing DT bindings for PM domains [1] and for PM domain idle
states [2].

Let's also add an example into the document for the PSCI DT bindings, to
clearly show the new hierarchical based layout. The currently supported
flattened layout, is already described in the ARM idle states bindings [3],
so let's leave that as is.

[1] Documentation/devicetree/bindings/power/power_domain.txt
[2] Documentation/devicetree/bindings/power/domain-idle-state.txt
[3] Documentation/devicetree/bindings/arm/idle-states.txt

Co-developed-by: Lina Iyer <redacted>
Signed-off-by: Lina Iyer <redacted>
Reviewed-by: Sudeep Holla <redacted>
Signed-off-by: Ulf Hansson <redacted>
---

Changes in v5:
        - None.

First I'm seeing this as the DT list was not copied. The example has
problems when running 'make dt_binding_check':

Documentation/devicetree/bindings/arm/psci.example.dt.yaml: cpu@0:
compatible: Additional items are not allowed ('arm,armv8' was
unexpected)
Documentation/devicetree/bindings/arm/psci.example.dt.yaml: cpu@0:
compatible: ['arm,cortex-a53', 'arm,armv8'] is too long
Documentation/devicetree/bindings/arm/psci.example.dt.yaml: cpu@1:
compatible: Additional items are not allowed ('arm,armv8' was
unexpected)
Documentation/devicetree/bindings/arm/psci.example.dt.yaml: cpu@1:
compatible: ['arm,cortex-a57', 'arm,armv8'] is too long

'arm,armv8' is only valid for s/w models.

Perhaps you have a different version of the tools than I have (I have
tried both on v.5.5-rc5 and todays linux-next), because I can't
reproduce these errors at my side when running "make
dt_binding_check".

Can you please check again?

Documentation/devicetree/bindings/arm/psci.example.dt.yaml:
idle-states: cluster-retention:compatible:0: 'arm,idle-state' was
expected
Documentation/devicetree/bindings/arm/psci.example.dt.yaml:
idle-states: cluster-power-down:compatible:0: 'arm,idle-state' was
expected

The last 2 are due to my conversion of the idle-states binding which
is in my tree now. Probably need to add 'domain-idle-state' as a
compatible at a minimum. It looks like domain-idle-state.txt is pretty
much the same as arm/idle-state.txt, so we should perhaps merge them.

Ahh, so maybe *all* of the above problems are caused by conflicts in
the arm-soc tree with changes from your tree!?

In regards to merging files, I am fine by that if that helps.

There's some bigger issues though.

quoted

---
 .../devicetree/bindings/arm/cpus.yaml         |  15 +++
 .../devicetree/bindings/arm/psci.yaml         | 104 ++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml
index c23c24ff7575..7a9c3ce2dbef 100644
--- a/Documentation/devicetree/bindings/arm/cpus.yaml
+++ b/Documentation/devicetree/bindings/arm/cpus.yaml

@@ -242,6 +242,21 @@ properties:

       where voltage is in V, frequency is in MHz.

+  power-domains:
+    $ref: '/schemas/types.yaml#/definitions/phandle-array'
+    description:
+      List of phandles and PM domain specifiers, as defined by bindings of the
+      PM domain provider (see also ../power_domain.txt).
+
+  power-domain-names:
+    $ref: '/schemas/types.yaml#/definitions/string-array'
+    description:
+      A list of power domain name strings sorted in the same order as the
+      power-domains property.
+
+      For PSCI based platforms, the name corresponding to the index of the PSCI
+      PM domain provider, must be "psci".
+
   qcom,saw:
     $ref: '/schemas/types.yaml#/definitions/phandle'
     description: |

diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
index 7abdf58b335e..8ef85420b2ab 100644
--- a/Documentation/devicetree/bindings/arm/psci.yaml
+++ b/Documentation/devicetree/bindings/arm/psci.yaml

@@ -102,6 +102,34 @@ properties:
       [1] Kernel documentation - ARM idle states bindings
         Documentation/devicetree/bindings/arm/idle-states.txt

+  "#power-domain-cells":

This is wrong because you are saying the /psci node should have these
properties. You need to define the child nodes (at least a pattern you
can match on) and put these properties there.

Right, good point.

I searched for some similar examples for how to encode this, but
couldn't really find something useful. One more thing, it seems like
this change is also needed for the common power-domain bindings, as
that also specifies parent/childs domains.

Anyway, I would really appreciate if you can suggest something more
detailed for you think this should be done!?

quoted

+    description:
+      The number of cells in a PM domain specifier as per binding in [3].
+      Must be 0 as to represent a single PM domain.
+
+      ARM systems can have multiple cores, sometimes in an hierarchical
+      arrangement. This often, but not always, maps directly to the processor
+      power topology of the system. Individual nodes in a topology have their
+      own specific power states and can be better represented hierarchically.
+
+      For these cases, the definitions of the idle states for the CPUs and the
+      CPU topology, must conform to the binding in [3]. The idle states
+      themselves must conform to the binding in [4] and must specify the
+      arm,psci-suspend-param property.
+
+      It should also be noted that, in PSCI firmware v1.0 the OS-Initiated
+      (OSI) CPU suspend mode is introduced. Using a hierarchical representation
+      helps to implement support for OSI mode and OS implementations may choose
+      to mandate it.
+
+      [3] Documentation/devicetree/bindings/power/power_domain.txt
+      [4] Documentation/devicetree/bindings/power/domain-idle-state.txt
+
+  power-domains:
+    $ref: '/schemas/types.yaml#/definitions/phandle-array'
+    description:
+      List of phandles and PM domain specifiers, as defined by bindings of the
+      PM domain provider.

A schema for 'domain-idle-states' property is missing.

Right, let's figure out the best way for how to add that.

quoted

 required:
   - compatible

@@ -160,4 +188,80 @@ examples:
       cpu_on = <0x95c10002>;
       cpu_off = <0x95c10001>;
     };
+
+  - |+
+
+    // Case 4: CPUs and CPU idle states described using the hierarchical model.
+
+    cpus {
+      #size-cells = <0>;
+      #address-cells = <1>;
+
+      CPU0: cpu@0 {
+        device_type = "cpu";
+        compatible = "arm,cortex-a53", "arm,armv8";
+        reg = <0x0>;
+        enable-method = "psci";
+        power-domains = <&CPU_PD0>;
+        power-domain-names = "psci";
+      };
+
+      CPU1: cpu@1 {
+        device_type = "cpu";
+        compatible = "arm,cortex-a57", "arm,armv8";
+        reg = <0x100>;
+        enable-method = "psci";
+        power-domains = <&CPU_PD1>;
+        power-domain-names = "psci";
+      };
+
+      idle-states {
+
+        CPU_PWRDN: cpu-power-down {
+          compatible = "arm,idle-state";
+          arm,psci-suspend-param = <0x0000001>;
+          entry-latency-us = <10>;
+          exit-latency-us = <10>;
+          min-residency-us = <100>;
+        };
+
+        CLUSTER_RET: cluster-retention {
+          compatible = "domain-idle-state";
+          arm,psci-suspend-param = <0x1000011>;
+          entry-latency-us = <500>;
+          exit-latency-us = <500>;
+          min-residency-us = <2000>;
+        };
+
+        CLUSTER_PWRDN: cluster-power-down {
+          compatible = "domain-idle-state";
+          arm,psci-suspend-param = <0x1000031>;
+          entry-latency-us = <2000>;
+          exit-latency-us = <2000>;
+          min-residency-us = <6000>;
+        };
+      };
+    };
+
+    psci {
+      compatible = "arm,psci-1.0";
+      method = "smc";
+
+      CPU_PD0: cpu-pd0 {
+        #power-domain-cells = <0>;
+        domain-idle-states = <&CPU_PWRDN>;
+        power-domains = <&CLUSTER_PD>;
+      };
+
+      CPU_PD1: cpu-pd1 {
+        #power-domain-cells = <0>;
+        domain-idle-states =  <&CPU_PWRDN>;
+        power-domains = <&CLUSTER_PD>;
+      };
+
+      CLUSTER_PD: cluster-pd {
+        #power-domain-cells = <0>;
+        domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWRDN>;
+      };
+    };
 ...
--

2.17.1

Thanks for your feedback!

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel