[PATCH v3 15/15] ARM64: dts: Define CPU power domain for MSM8916

[Sudeep Holla]

On 05/08/16 00:05, Lina Iyer wrote:
> Define power domain and the power states for the domain as defined by
> the PSCI firmware.

> The 8916 firmware supports OS initiated method of
> powering off the CPU clusters.

How is that related to the this DTS change, more details below ?

>
> Cc: <devicetree at vger.kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer at linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 3029773..eb0aaed 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -64,6 +64,7 @@
>  			next-level-cache = <&L2_0>;
>  			enable-method = "psci";
>  			cpu-idle-states = <&CPU_SPC>;
> +			power-domains = <&CPU_PD>;

This is really messy. We need to have idle state information at one 
place. I prefer to have a hierarchal representation of power-domains
for CPU with idle-states at each level.

>  		};
>
>  		CPU1: cpu at 1 {
> @@ -73,6 +74,7 @@
>  			next-level-cache = <&L2_0>;
>  			enable-method = "psci";
>  			cpu-idle-states = <&CPU_SPC>;
> +			power-domains = <&CPU_PD>;
>  		};
>
>  		CPU2: cpu at 2 {
> @@ -82,6 +84,7 @@
>  			next-level-cache = <&L2_0>;
>  			enable-method = "psci";
>  			cpu-idle-states = <&CPU_SPC>;
> +			power-domains = <&CPU_PD>;
>  		};
>
>  		CPU3: cpu at 3 {
> @@ -91,6 +94,7 @@
>  			next-level-cache = <&L2_0>;
>  			enable-method = "psci";
>  			cpu-idle-states = <&CPU_SPC>;
> +			power-domains = <&CPU_PD>;
>  		};
>
>  		L2_0: l2-cache {
> @@ -113,6 +117,29 @@
>  	psci {
>  		compatible = "arm,psci-1.0";
>  		method = "smc";

Why is it inside PSCI node ? I don't see a need for that.
If it needs to be here, then amend the binding document.

> +
> +		CPU_PD: cpu-pd at 0 {
> +			#power-domain-cells = <0>;
> +			domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
> +		};
> +
> +		domain-states {
> +			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_gdhs {
> +				compatible = "arm,idle-state";
> +				arm,psci-suspend-param = <0x1000030>;
> +				entry-latency-us = <2000>;
> +				exit-latency-us = <2000>;
> +				min-residency-us = <6000>;
> +			};
> +		};

So how do you collapse these states into the cpu level states ?
We should be able to cope up with platform co-ordinated mode of idle.
For me, this binding and the representation here is designed only to
address OS co-ordinated mode of idle support but it should be other way
around. Design the bindings that can cater any mode (platform and OS
co-ordinated)

-- 
Regards,
Sudeep