diff mbox series

[v3] arm64: dts: qcom: sc7280: revert back to PSCI PC mode for herobrine

Message ID 20231222190311.3344572-1-olvaffe@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series [v3] arm64: dts: qcom: sc7280: revert back to PSCI PC mode for herobrine | expand

Commit Message

Chia-I Wu Dec. 22, 2023, 7:03 p.m. UTC
Commit 7925ca85e9561 ("arm64: dts: qcom: sc7280: Add power-domains for
cpuidle states") transitioned all SC7280 devices to PSCI OS initiated
mode, which doesn't work on TFA-based SC7280 devices.  This effectively
revert the commit for sc7280-herobrine.

Fixes: 7925ca85e9561 ("arm64: dts: qcom: sc7280: Add power-domains for cpuidle states")
Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
---

v2: improved commit message
v3: improved commit message.  I hope it's better now!

 .../boot/dts/qcom/sc7280-firmware-tfa.dtsi    | 107 ++++++++++++++++++
 .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi |   1 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi          |   4 +-
 3 files changed, 110 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-firmware-tfa.dtsi

Comments

Nikita Travkin Dec. 23, 2023, 8:05 a.m. UTC | #1
On Fri, Dec 22, 2023 at 11:03:03AM -0800, Chia-I Wu wrote:
> Commit 7925ca85e9561 ("arm64: dts: qcom: sc7280: Add power-domains for
> cpuidle states") transitioned all SC7280 devices to PSCI OS initiated
> mode, which doesn't work on TFA-based SC7280 devices.  This effectively
> revert the commit for sc7280-herobrine.
>

Hi!

I believe modern TF-A includes OSI mode, and it was added pretty much
specifically for sc7280, as described in [1]. More to that, I think
the original commit that introduced OSI mode for sc7280 did it using
the TF-A specific suspend params (I believe they are different from
qcom firmware) so the leftover state of the base soc dtsi would be
weird...

I can't understand why this change is needed...

Nikita

[1] https://trustedfirmware-a.readthedocs.io/en/latest/design_documents/psci_osi_mode.html

> Fixes: 7925ca85e9561 ("arm64: dts: qcom: sc7280: Add power-domains for cpuidle states")
> Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
> ---
> 
> v2: improved commit message
> v3: improved commit message.  I hope it's better now!
> 
>  .../boot/dts/qcom/sc7280-firmware-tfa.dtsi    | 107 ++++++++++++++++++
>  .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi |   1 +
>  arch/arm64/boot/dts/qcom/sc7280.dtsi          |   4 +-
>  3 files changed, 110 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/qcom/sc7280-firmware-tfa.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-firmware-tfa.dtsi b/arch/arm64/boot/dts/qcom/sc7280-firmware-tfa.dtsi
> new file mode 100644
> index 0000000000000..b3fc03da244d6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7280-firmware-tfa.dtsi
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +
> +/*
> + * Devices that use SC7280 with TrustedFirmware-A
> + * need PSCI PC mode instead of the OSI mode provided
> + * by Qualcomm firmware.
> + */
> +
> +&CPU0 {
> +	/delete-property/ power-domains;
> +	/delete-property/ power-domain-names;
> +
> +	cpu-idle-states = <&LITTLE_CPU_SLEEP_0
> +			   &LITTLE_CPU_SLEEP_1
> +			   &CLUSTER_SLEEP_0>;
> +};
> +
> +&CPU1 {
> +	/delete-property/ power-domains;
> +	/delete-property/ power-domain-names;
> +
> +	cpu-idle-states = <&LITTLE_CPU_SLEEP_0
> +			   &LITTLE_CPU_SLEEP_1
> +			   &CLUSTER_SLEEP_0>;
> +};
> +
> +&CPU2 {
> +	/delete-property/ power-domains;
> +	/delete-property/ power-domain-names;
> +
> +	cpu-idle-states = <&LITTLE_CPU_SLEEP_0
> +			   &LITTLE_CPU_SLEEP_1
> +			   &CLUSTER_SLEEP_0>;
> +};
> +
> +&CPU3 {
> +	/delete-property/ power-domains;
> +	/delete-property/ power-domain-names;
> +
> +	cpu-idle-states = <&LITTLE_CPU_SLEEP_0
> +			   &LITTLE_CPU_SLEEP_1
> +			   &CLUSTER_SLEEP_0>;
> +};
> +
> +&CPU4 {
> +	/delete-property/ power-domains;
> +	/delete-property/ power-domain-names;
> +
> +	cpu-idle-states = <&BIG_CPU_SLEEP_0
> +			   &BIG_CPU_SLEEP_1
> +			   &CLUSTER_SLEEP_0>;
> +};
> +
> +&CPU5 {
> +	/delete-property/ power-domains;
> +	/delete-property/ power-domain-names;
> +
> +	cpu-idle-states = <&BIG_CPU_SLEEP_0
> +			   &BIG_CPU_SLEEP_1
> +			   &CLUSTER_SLEEP_0>;
> +};
> +
> +&CPU6 {
> +	/delete-property/ power-domains;
> +	/delete-property/ power-domain-names;
> +
> +	cpu-idle-states = <&BIG_CPU_SLEEP_0
> +			   &BIG_CPU_SLEEP_1
> +			   &CLUSTER_SLEEP_0>;
> +};
> +
> +&CPU7 {
> +	/delete-property/ power-domains;
> +	/delete-property/ power-domain-names;
> +
> +	cpu-idle-states = <&BIG_CPU_SLEEP_0
> +			   &BIG_CPU_SLEEP_1
> +			   &CLUSTER_SLEEP_0>;
> +};
> +
> +/delete-node/ &domain_idle_states;
> +
> +&idle_states {
> +	CLUSTER_SLEEP_0: cluster-sleep-0 {
> +		compatible = "arm,idle-state";
> +		idle-state-name = "cluster-power-down";
> +		arm,psci-suspend-param = <0x40003444>;
> +		entry-latency-us = <3263>;
> +		exit-latency-us = <6562>;
> +		min-residency-us = <9926>;
> +		local-timer-stop;
> +	};
> +};
> +
> +/delete-node/ &CPU_PD0;
> +/delete-node/ &CPU_PD1;
> +/delete-node/ &CPU_PD2;
> +/delete-node/ &CPU_PD3;
> +/delete-node/ &CPU_PD4;
> +/delete-node/ &CPU_PD5;
> +/delete-node/ &CPU_PD6;
> +/delete-node/ &CPU_PD7;
> +/delete-node/ &CLUSTER_PD;
> +
> +&apps_rsc {
> +	/delete-property/ power-domains;
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index 9ea6636125ad9..09b2d370bf7e0 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -19,6 +19,7 @@
>  
>  #include "sc7280-qcard.dtsi"
>  #include "sc7280-chrome-common.dtsi"
> +#include "sc7280-firmware-tfa.dtsi"
>  
>  / {
>  	chosen {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 66f1eb83cca7e..354bf2868eba6 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -383,7 +383,7 @@ core7 {
>  			};
>  		};
>  
> -		idle-states {
> +		idle_states: idle-states {
>  			entry-method = "psci";
>  
>  			LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
> @@ -427,7 +427,7 @@ BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
>  			};
>  		};
>  
> -		domain-idle-states {
> +		domain_idle_states: domain-idle-states {
>  			CLUSTER_SLEEP_0: cluster-sleep-0 {
>  				compatible = "domain-idle-state";
>  				idle-state-name = "cluster-power-down";
> -- 
> 2.43.0.195.gebba966016-goog
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280-firmware-tfa.dtsi b/arch/arm64/boot/dts/qcom/sc7280-firmware-tfa.dtsi
new file mode 100644
index 0000000000000..b3fc03da244d6
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7280-firmware-tfa.dtsi
@@ -0,0 +1,107 @@ 
+// SPDX-License-Identifier: BSD-3-Clause
+
+/*
+ * Devices that use SC7280 with TrustedFirmware-A
+ * need PSCI PC mode instead of the OSI mode provided
+ * by Qualcomm firmware.
+ */
+
+&CPU0 {
+	/delete-property/ power-domains;
+	/delete-property/ power-domain-names;
+
+	cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+			   &LITTLE_CPU_SLEEP_1
+			   &CLUSTER_SLEEP_0>;
+};
+
+&CPU1 {
+	/delete-property/ power-domains;
+	/delete-property/ power-domain-names;
+
+	cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+			   &LITTLE_CPU_SLEEP_1
+			   &CLUSTER_SLEEP_0>;
+};
+
+&CPU2 {
+	/delete-property/ power-domains;
+	/delete-property/ power-domain-names;
+
+	cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+			   &LITTLE_CPU_SLEEP_1
+			   &CLUSTER_SLEEP_0>;
+};
+
+&CPU3 {
+	/delete-property/ power-domains;
+	/delete-property/ power-domain-names;
+
+	cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+			   &LITTLE_CPU_SLEEP_1
+			   &CLUSTER_SLEEP_0>;
+};
+
+&CPU4 {
+	/delete-property/ power-domains;
+	/delete-property/ power-domain-names;
+
+	cpu-idle-states = <&BIG_CPU_SLEEP_0
+			   &BIG_CPU_SLEEP_1
+			   &CLUSTER_SLEEP_0>;
+};
+
+&CPU5 {
+	/delete-property/ power-domains;
+	/delete-property/ power-domain-names;
+
+	cpu-idle-states = <&BIG_CPU_SLEEP_0
+			   &BIG_CPU_SLEEP_1
+			   &CLUSTER_SLEEP_0>;
+};
+
+&CPU6 {
+	/delete-property/ power-domains;
+	/delete-property/ power-domain-names;
+
+	cpu-idle-states = <&BIG_CPU_SLEEP_0
+			   &BIG_CPU_SLEEP_1
+			   &CLUSTER_SLEEP_0>;
+};
+
+&CPU7 {
+	/delete-property/ power-domains;
+	/delete-property/ power-domain-names;
+
+	cpu-idle-states = <&BIG_CPU_SLEEP_0
+			   &BIG_CPU_SLEEP_1
+			   &CLUSTER_SLEEP_0>;
+};
+
+/delete-node/ &domain_idle_states;
+
+&idle_states {
+	CLUSTER_SLEEP_0: cluster-sleep-0 {
+		compatible = "arm,idle-state";
+		idle-state-name = "cluster-power-down";
+		arm,psci-suspend-param = <0x40003444>;
+		entry-latency-us = <3263>;
+		exit-latency-us = <6562>;
+		min-residency-us = <9926>;
+		local-timer-stop;
+	};
+};
+
+/delete-node/ &CPU_PD0;
+/delete-node/ &CPU_PD1;
+/delete-node/ &CPU_PD2;
+/delete-node/ &CPU_PD3;
+/delete-node/ &CPU_PD4;
+/delete-node/ &CPU_PD5;
+/delete-node/ &CPU_PD6;
+/delete-node/ &CPU_PD7;
+/delete-node/ &CLUSTER_PD;
+
+&apps_rsc {
+	/delete-property/ power-domains;
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index 9ea6636125ad9..09b2d370bf7e0 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -19,6 +19,7 @@ 
 
 #include "sc7280-qcard.dtsi"
 #include "sc7280-chrome-common.dtsi"
+#include "sc7280-firmware-tfa.dtsi"
 
 / {
 	chosen {
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 66f1eb83cca7e..354bf2868eba6 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -383,7 +383,7 @@  core7 {
 			};
 		};
 
-		idle-states {
+		idle_states: idle-states {
 			entry-method = "psci";
 
 			LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
@@ -427,7 +427,7 @@  BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
 			};
 		};
 
-		domain-idle-states {
+		domain_idle_states: domain-idle-states {
 			CLUSTER_SLEEP_0: cluster-sleep-0 {
 				compatible = "domain-idle-state";
 				idle-state-name = "cluster-power-down";