On 26 December 2017 at 23:13, Rob Herring <robh@xxxxxxxxxx> wrote: > On Fri, Dec 22, 2017 at 03:32:07PM +0100, Ulf Hansson wrote: >> From: Lina Iyer <lina.iyer@xxxxxxxxxx> >> >> Update DT bindings to represent hierarchical CPU and CPU domain idle states >> for PSCI. Also update the PSCI examples to clearly show how flattened and >> hierarchical idle states can be represented in DT. >> >> Signed-off-by: Lina Iyer <lina.iyer@xxxxxxxxxx> >> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> >> --- >> >> For your information, I have picked up the work from Lina Iyer around the so >> called CPU cluster idling series [1,2] and I working on new versions. However, >> I decided to post the updates to the PSCI DT bindings first, as they will be >> needed to be agreed upon before further changes can be done to the PSCI firmware >> driver. >> >> Note, these bindings have been discussed over and over again, at LKML, but >> especially also at various Linux conferences, like LPC and Linaro Connect. We >> finally came to a conclusion and the changes we agreed upon, should be reflected >> in this update. >> >> Of course, it's a while ago since the latest discussions, but hopefully people >> don't have too hard time to remember. > > Vaguely... > >> >> Kind regards >> Uffe >> >> [1] >> https://www.spinics.net/lists/arm-kernel/msg566200.html >> >> [2] >> https://lwn.net/Articles/716300/ >> >> --- >> Documentation/devicetree/bindings/arm/psci.txt | 152 +++++++++++++++++++++++++ >> 1 file changed, 152 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt >> index a2c4f1d..5a8f11b 100644 >> --- a/Documentation/devicetree/bindings/arm/psci.txt >> +++ b/Documentation/devicetree/bindings/arm/psci.txt >> @@ -105,7 +105,159 @@ Case 3: PSCI v0.2 and PSCI v0.1. >> ... >> }; >> >> +PSCI v1.0 onwards, supports OS-Initiated mode for powering off CPUs and CPU >> +clusters from the firmware. For such topologies the PSCI firmware driver acts >> +as pseudo-controller, which may be specified in the psci DT node. The >> +definitions of the CPU and the CPU cluster topology, must conform to the domain >> +idle state specification [3]. The domain idle states themselves, must be >> +compatible with the defined 'domain-idle-state' binding [1], and also need to >> +specify the arm,psci-suspend-param property for each idle state. >> + >> +DT allows representing CPU and CPU cluster idle states in two different ways - >> + >> +The flattened model as given in Example 1, lists CPU's idle states followed by >> +the domain idle state that the CPUs may choose. This is the general practice >> +followed in PSCI firmwares that support Platform Coordinated mode. Note that >> +the idle states are all compatible with "arm,idle-state". >> + >> +Example 2 represents the hierarchical model of CPU and domain idle states. >> +CPUs define their domain provider in their DT node. The domain controls the >> +power to the CPU and possibly other h/w blocks that would be powered off when >> +the CPU is powered off. The CPU's idle states may therefore be considered as >> +the domain's idle states and have the compatible "arm,idle-state". Such domains >> +may be embedded within another domain that represents common h/w blocks between >> +these CPUs viz. the cluster. The idle states of the cluster would be >> +represented as the domain's idle states. In order to use OS-Initiated mode of >> +PSCI in the firmware, the hierarchical representation must be used. >> + >> +Example 1: Flattened representation of CPU and domain idle states >> + cpus { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + CPU0: cpu@0 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x0>; >> + enable-method = "psci"; >> + cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>, >> + <&CLUSTER_PWR_DWN>; >> + }; >> + >> + CPU1: cpu@1 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a57", "arm,armv8"; >> + reg = <0x100>; >> + enable-method = "psci"; >> + cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>, >> + <&CLUSTER_PWR_DWN>; >> + }; >> + >> + idle-states { >> + CPU_PWRDN: cpu_power_down{ > > Use '-' rather than '_'. dtc has more warnings since last time... Yes, I found this on a couple of more places and fixed them all. > >> + compatible = "arm,idle-state"; >> + arm,psci-suspend-param = <0x000001>; >> + entry-latency-us = <10>; >> + exit-latency-us = <10>; >> + min-residency-us = <100>; >> + }; >> + >> + CLUSTER_RET: domain_ret { >> + compatible = "arm,idle-state"; >> + arm,psci-suspend-param = <0x1000010>; >> + entry-latency-us = <500>; >> + exit-latency-us = <500>; >> + min-residency-us = <2000>; >> + }; >> + >> + CLUSTER_PWR_DWN: domain_off { >> + compatible = "arm,idle-state"; >> + arm,psci-suspend-param = <0x1000030>; >> + entry-latency-us = <2000>; >> + exit-latency-us = <2000>; >> + min-residency-us = <6000>; >> + }; >> + }; >> + >> + psci { >> + compatible = "arm,psci-0.2"; >> + method = "smc"; >> + }; >> + >> +Example 2: Hierarchical representation of CPU and domain idle states >> + >> + cpus { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + CPU0: cpu@0 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x0>; >> + enable-method = "psci"; >> + power-domains = <&CPU_PD0>; >> + }; >> + >> + CPU1: cpu@1 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a57", "arm,armv8"; >> + reg = <0x100>; >> + enable-method = "psci"; >> + power-domains = <&CPU_PD1>; >> + }; >> + >> + idle-states { >> + CPU_PWRDN: cpu_power_down{ >> + compatible = "arm,idle-state"; >> + arm,psci-suspend-param = <0x000001>; >> + entry-latency-us = <10>; >> + exit-latency-us = <10>; >> + min-residency-us = <100>; >> + }; >> + >> + CLUSTER_RET: domain_ret { >> + compatible = "domain-idle-state"; >> + arm,psci-suspend-param = <0x1000010>; >> + entry-latency-us = <500>; >> + exit-latency-us = <500>; >> + min-residency-us = <2000>; >> + }; >> + >> + CLUSTER_PWR_DWN: domain_off { >> + compatible = "domain-idle-state"; >> + arm,psci-suspend-param = <0x1000030>; >> + entry-latency-us = <2000>; >> + exit-latency-us = <2000>; >> + min-residency-us = <6000>; >> + }; >> + }; >> + }; >> + >> + psci { >> + compatible = "arm,psci-1.0"; >> + method = "smc"; >> + >> + CPU_PD0: cpu-pd@0 { > > A unit address without reg property is now a warning. There is currently no need for reg property, so let me replace this with: CPU_PD0: cpu-pd0 > >> + #power-domain-cells = <0>; >> + domain-idle-states = <&CPU_PWRDN>; >> + power-domains = <&CLUSTER_PD>; >> + }; >> + >> + CPU_PD1: cpu-pd@1 { >> + #power-domain-cells = <0>; >> + domain-idle-states = <&CPU_PWRDN>; >> + power-domains = <&CLUSTER_PD>; > > Could this node be a child of CLUSTER_PD rather than having a phandle? > Doesn't matter so much here, but when you have 3 levels? This follows existing bindings for power-domains as per Documentation/devicetree/bindings/power/power_domain.txt. I guess it's better to stick to that!? > >> + }; > > These 2 nodes are identical, so why do you need both? Each node represents a power domain, which contains devices (in this case CPUs) that are specific to each power domain. In other words, we are trying to describe the HW using a hierarchical layout. > >> + >> + CLUSTER_PD: cluster-pd@0 { >> + #power-domain-cells = <0>; >> + domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>; >> + }; >> + }; >> + >> [1] Kernel documentation - ARM idle states bindings >> Documentation/devicetree/bindings/arm/idle-states.txt >> [2] Power State Coordination Interface (PSCI) specification >> http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf >> +[3]. PM Domains description >> + Documentation/devicetree/bindings/power/power_domain.txt >> -- >> 2.7.4 >> Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
