diff mbox series

[V5,5/5] arm64: dts: imx8mm: Enable cpu-idle driver

Message ID 20190710063056.35689-5-Anson.Huang@nxp.com (mailing list archive)
State New, archived
Headers show
Series [V5,1/5] clocksource: imx-sysctr: Add internal clock divider handle | expand

Commit Message

Anson Huang July 10, 2019, 6:30 a.m. UTC 
From: Anson Huang <Anson.Huang@nxp.com>

Enable i.MX8MM cpu-idle using generic ARM cpu-idle driver, 2 states
are supported, details as below:

root@imx8mmevk:~# cat /sys/devices/system/cpu/cpu0/cpuidle/state0/name
WFI
root@imx8mmevk:~# cat /sys/devices/system/cpu/cpu0/cpuidle/state0/usage
3973
root@imx8mmevk:~# cat /sys/devices/system/cpu/cpu0/cpuidle/state1/name
cpu-sleep-wait
root@imx8mmevk:~# cat /sys/devices/system/cpu/cpu0/cpuidle/state1/usage
6647

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
New patch, as this patch is based on other patches in this series, so I include it.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Daniel Lezcano Aug. 15, 2019, 4:12 p.m. UTC | #1 
Hi Anson,

sorry for the late review, I've been pretty busy.

If Shawn is ok, I can pick the patches 1-4 in my tree and then this one
after you fix the comments below.

On 10/07/2019 08:30, Anson.Huang@nxp.com wrote:

[ ... ]

> +		idle-states {
> +			entry-method = "psci";
> +
> +			cpu_sleep_wait: cpu-sleep-wait {

Is that a retention state or a powerdown? It is preferable to change the
name to the idle state naming convention given in the PSCI documentation
[1] page 16-17


> +				compatible = "arm,idle-state";
> +				arm,psci-suspend-param = <0x0010033>;
> +				local-timer-stop;
> +				entry-latency-us = <1000>;
> +				exit-latency-us = <700>;
> +				min-residency-us = <2700>;
> +				wakeup-latency-us = <1500>;

It is pointless to specify the entry + exit *and* the wakeup-latency [2].

Thanks

  -- Daniel

[1]
http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpuidle/dt_idle_states.c#n41
Anson Huang Aug. 16, 2019, 1:03 a.m. UTC | #2 
Hi, Daniel

> Hi Anson,
> 
> sorry for the late review, I've been pretty busy.

That is OK for sure.

> 
> If Shawn is ok, I can pick the patches 1-4 in my tree and then this one after
> you fix the comments below.

Shawn should be OK for it, and he already took patch [PATCH V5 2/5] arm64: Enable TIMER_IMX_SYS_CTR for ARCH_MXC platforms
since I ever sent it before in other series when system counter driver is NOT landing on main line, now it landed, Shawn just apply
that old patch, so in V6 patch I just sent, I did NOT include this patch, you can just apply the 4 patches in V6.

Hi, Shawn
	Daniel will pick this whole patch series, please raise if you have any concern, thanks.

> 
> On 10/07/2019 08:30, Anson.Huang@nxp.com wrote:
> 
> [ ... ]
> 
> > +		idle-states {
> > +			entry-method = "psci";
> > +
> > +			cpu_sleep_wait: cpu-sleep-wait {
> 
> Is that a retention state or a powerdown? It is preferable to change the name
> to the idle state naming convention given in the PSCI documentation [1] page
> 16-17

Thanks for your detail reference, it is a power down state with SoC entering WAIT mode,
so in V6, I change the name to "cpu_pd_wait:cpu-pd-wait".

> 
> 
> > +				compatible = "arm,idle-state";
> > +				arm,psci-suspend-param = <0x0010033>;
> > +				local-timer-stop;
> > +				entry-latency-us = <1000>;
> > +				exit-latency-us = <700>;
> > +				min-residency-us = <2700>;
> > +				wakeup-latency-us = <1500>;
> 
> It is pointless to specify the entry + exit *and* the wakeup-latency [2].

Ah, yes, this is new to me, I will just remove the “wakeup-latency-us” property.

Thanks,
Anson.
Shawn Guo Aug. 19, 2019, 7:27 a.m. UTC | #3 
On Thu, Aug 15, 2019 at 06:12:50PM +0200, Daniel Lezcano wrote:
> 
> Hi Anson,
> 
> sorry for the late review, I've been pretty busy.
> 
> If Shawn is ok, I can pick the patches 1-4 in my tree and then this one
> after you fix the comments below.

I'm okay, so:

Acked-by: Shawn Guo <shawnguo@kernel.org>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 8cf7f34..8f3ed39 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -44,6 +44,20 @@ 
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		idle-states {
+			entry-method = "psci";
+
+			cpu_sleep_wait: cpu-sleep-wait {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x0010033>;
+				local-timer-stop;
+				entry-latency-us = <1000>;
+				exit-latency-us = <700>;
+				min-residency-us = <2700>;
+				wakeup-latency-us = <1500>;
+			};
+		};
+
 		A53_0: cpu@0 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a53";
@@ -56,6 +70,7 @@ 
 			nvmem-cells = <&cpu_speed_grade>;
 			nvmem-cell-names = "speed_grade";
 			#cooling-cells = <2>;
+			cpu-idle-states = <&cpu_sleep_wait>;
 		};
 
 		A53_1: cpu@1 {
@@ -68,6 +83,7 @@ 
 			next-level-cache = <&A53_L2>;
 			operating-points-v2 = <&a53_opp_table>;
 			#cooling-cells = <2>;
+			cpu-idle-states = <&cpu_sleep_wait>;
 		};
 
 		A53_2: cpu@2 {
@@ -80,6 +96,7 @@ 
 			next-level-cache = <&A53_L2>;
 			operating-points-v2 = <&a53_opp_table>;
 			#cooling-cells = <2>;
+			cpu-idle-states = <&cpu_sleep_wait>;
 		};
 
 		A53_3: cpu@3 {
@@ -92,6 +109,7 @@ 
 			next-level-cache = <&A53_L2>;
 			operating-points-v2 = <&a53_opp_table>;
 			#cooling-cells = <2>;
+			cpu-idle-states = <&cpu_sleep_wait>;
 		};
 
 		A53_L2: l2-cache0 {