Re: [PATCH v4 14/14] arm64: dts: Convert to the hierarchical CPU topology layout for MSM8916

[Sudeep Holla <sudeep.holla@arm.com>]

On Wed, Dec 11, 2019 at 04:43:43PM +0100, Ulf Hansson wrote:
> To enable the OS to better support PSCI OS initiated CPU suspend mode,
> let's convert from the flattened layout to the hierarchical layout.
>
> In the hierarchical layout, let's create a power domain provider per CPU
> and describe the idle states for each CPU inside the power domain provider
> node. To group the CPUs into a cluster, let's add another power domain
> provider and make it act as the master domain. Note that, the CPU's idle
> states remains compatible with "arm,idle-state", while the cluster's idle
> state becomes compatible with "domain-idle-state".
>
> Co-developed-by: Lina Iyer <lina.iyer@linaro.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>
> Changes in v4:
> 	- None.
>
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 57 +++++++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 8686e101905c..282c36c8fa3b 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -102,10 +102,11 @@
>  			reg = <0x0>;
>  			next-level-cache = <&L2_0>;
>  			enable-method = "psci";
> -			cpu-idle-states = <&CPU_SLEEP_0>;
>  			clocks = <&apcs>;
>  			operating-points-v2 = <&cpu_opp_table>;
>  			#cooling-cells = <2>;
> +			power-domains = <&CPU_PD0>;
> +			power-domain-names = "psci";
>  		};
>
>  		CPU1: cpu@1 {
> @@ -114,10 +115,11 @@
>  			reg = <0x1>;
>  			next-level-cache = <&L2_0>;
>  			enable-method = "psci";
> -			cpu-idle-states = <&CPU_SLEEP_0>;
>  			clocks = <&apcs>;
>  			operating-points-v2 = <&cpu_opp_table>;
>  			#cooling-cells = <2>;
> +			power-domains = <&CPU_PD1>;
> +			power-domain-names = "psci";
>  		};
>
>  		CPU2: cpu@2 {
> @@ -126,10 +128,11 @@
>  			reg = <0x2>;
>  			next-level-cache = <&L2_0>;
>  			enable-method = "psci";
> -			cpu-idle-states = <&CPU_SLEEP_0>;
>  			clocks = <&apcs>;
>  			operating-points-v2 = <&cpu_opp_table>;
>  			#cooling-cells = <2>;
> +			power-domains = <&CPU_PD2>;
> +			power-domain-names = "psci";
>  		};
>
>  		CPU3: cpu@3 {
> @@ -138,10 +141,11 @@
>  			reg = <0x3>;
>  			next-level-cache = <&L2_0>;
>  			enable-method = "psci";
> -			cpu-idle-states = <&CPU_SLEEP_0>;
>  			clocks = <&apcs>;
>  			operating-points-v2 = <&cpu_opp_table>;
>  			#cooling-cells = <2>;
> +			power-domains = <&CPU_PD3>;
> +			power-domain-names = "psci";
>  		};
>
>  		L2_0: l2-cache {
> @@ -161,12 +165,57 @@
>  				min-residency-us = <2000>;
>  				local-timer-stop;
>  			};
> +
> +			CLUSTER_RET: cluster-retention {
> +				compatible = "domain-idle-state";
> +				arm,psci-suspend-param = <0x41000012>;

If I followed the thread correctly, we are now just using the param as is
without any extra logic like ORing like before. i.e. The domain state
parameter overrides the any param below it in the hierarchy.

If that's correct, then
Acked-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep