linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states
@ 2018-10-15 17:02 Raju P.L.S.S.S.N
  2018-10-19 19:48 ` Lina Iyer
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Raju P.L.S.S.S.N @ 2018-10-15 17:02 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, devicetree,
	sboyd, evgreen, dianders, mka, ilina, Raju P.L.S.S.S.N

Add device bindings for cpuidle states for cpu devices.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0c9a2aa..32262b0 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -96,6 +96,7 @@
 			reg = <0x0 0x0>;
 			enable-method = "psci";
 			next-level-cache = <&L2_0>;
+			cpu-idle-states = <&C0_CPU_SPC &C0_CPU_PC &CLUSTER_PC>;
 			L2_0: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -111,6 +112,7 @@
 			reg = <0x0 0x100>;
 			enable-method = "psci";
 			next-level-cache = <&L2_100>;
+			cpu-idle-states = <&C0_CPU_SPC &C0_CPU_PC &CLUSTER_PC>;
 			L2_100: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -123,6 +125,7 @@
 			reg = <0x0 0x200>;
 			enable-method = "psci";
 			next-level-cache = <&L2_200>;
+			cpu-idle-states = <&C0_CPU_SPC &C0_CPU_PC &CLUSTER_PC>;
 			L2_200: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -135,6 +138,7 @@
 			reg = <0x0 0x300>;
 			enable-method = "psci";
 			next-level-cache = <&L2_300>;
+			cpu-idle-states = <&C0_CPU_SPC &C0_CPU_PC &CLUSTER_PC>;
 			L2_300: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -147,6 +151,7 @@
 			reg = <0x0 0x400>;
 			enable-method = "psci";
 			next-level-cache = <&L2_400>;
+			cpu-idle-states = <&C4_CPU_SPC &C4_CPU_PC &CLUSTER_PC>;
 			L2_400: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -159,6 +164,7 @@
 			reg = <0x0 0x500>;
 			enable-method = "psci";
 			next-level-cache = <&L2_500>;
+			cpu-idle-states = <&C4_CPU_SPC &C4_CPU_PC &CLUSTER_PC>;
 			L2_500: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -171,6 +177,7 @@
 			reg = <0x0 0x600>;
 			enable-method = "psci";
 			next-level-cache = <&L2_600>;
+			cpu-idle-states = <&C4_CPU_SPC &C4_CPU_PC &CLUSTER_PC>;
 			L2_600: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -183,11 +190,66 @@
 			reg = <0x0 0x700>;
 			enable-method = "psci";
 			next-level-cache = <&L2_700>;
+			cpu-idle-states = <&C4_CPU_SPC &C4_CPU_PC &CLUSTER_PC>;
 			L2_700: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
 			};
 		};
+
+		idle-states {
+			entry-method = "psci";
+
+			C0_CPU_SPC: c0_spc {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x40000003>;
+				entry-latency-us = <350>;
+				exit-latency-us = <461>;
+				min-residency-us = <1890>;
+				local-timer-stop;
+				idle-state-name = "pc";
+			};
+
+			C0_CPU_PC: c0_pc {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x40000004>;
+				entry-latency-us = <360>;
+				exit-latency-us = <531>;
+				min-residency-us = <3934>;
+				local-timer-stop;
+				idle-state-name = "rail pc";
+			};
+
+			C4_CPU_SPC: c4_spc {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x40000003>;
+				entry-latency-us = <264>;
+				exit-latency-us = <621>;
+				min-residency-us = <952>;
+				local-timer-stop;
+				idle-state-name = "pc";
+			};
+
+			C4_CPU_PC: c4_pc {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x40000004>;
+				entry-latency-us = <702>;
+				exit-latency-us = <1061>;
+				min-residency-us = <4488>;
+				local-timer-stop;
+				idle-state-name = "rail pc";
+			};
+
+			CLUSTER_PC: cluster_pc {
+				compatible = "arm,idle-state";
+				arm,psci-suspend-param = <0x400000F4>;
+				entry-latency-us = <3263>;
+				exit-latency-us = <6562>;
+				min-residency-us = <9987>;
+				local-timer-stop;
+				idle-state-name = "cluster pc";
+			};
+		};
 	};
 
 	pmu {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of the Code Aurora Forum, hosted by The Linux Foundation.


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-10-15 17:02 [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states Raju P.L.S.S.S.N
@ 2018-10-19 19:48 ` Lina Iyer
  2018-10-22 18:34 ` Evan Green
  2018-10-23 18:36 ` Doug Anderson
  2 siblings, 0 replies; 6+ messages in thread
From: Lina Iyer @ 2018-10-19 19:48 UTC (permalink / raw)
  To: Raju P.L.S.S.S.N
  Cc: andy.gross, david.brown, linux-arm-msm, linux-soc, rnayak,
	bjorn.andersson, linux-kernel, linux-pm, devicetree, sboyd,
	evgreen, dianders, mka

On Mon, Oct 15 2018 at 11:02 -0600, Raju P.L.S.S.S.N wrote:
>Add device bindings for cpuidle states for cpu devices.
>
>Cc: <devicetree@vger.kernel.org>
>Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
>---
Reviewed-by: Lina Iyer <ilina@codeaurora.org>

> arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
>diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>index 0c9a2aa..32262b0 100644
>--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>@@ -96,6 +96,7 @@
> 			reg = <0x0 0x0>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_0>;
>+			cpu-idle-states = <&C0_CPU_SPC &C0_CPU_PC &CLUSTER_PC>;
> 			L2_0: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -111,6 +112,7 @@
> 			reg = <0x0 0x100>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_100>;
>+			cpu-idle-states = <&C0_CPU_SPC &C0_CPU_PC &CLUSTER_PC>;
> 			L2_100: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -123,6 +125,7 @@
> 			reg = <0x0 0x200>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_200>;
>+			cpu-idle-states = <&C0_CPU_SPC &C0_CPU_PC &CLUSTER_PC>;
> 			L2_200: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -135,6 +138,7 @@
> 			reg = <0x0 0x300>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_300>;
>+			cpu-idle-states = <&C0_CPU_SPC &C0_CPU_PC &CLUSTER_PC>;
> 			L2_300: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -147,6 +151,7 @@
> 			reg = <0x0 0x400>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_400>;
>+			cpu-idle-states = <&C4_CPU_SPC &C4_CPU_PC &CLUSTER_PC>;
> 			L2_400: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -159,6 +164,7 @@
> 			reg = <0x0 0x500>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_500>;
>+			cpu-idle-states = <&C4_CPU_SPC &C4_CPU_PC &CLUSTER_PC>;
> 			L2_500: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -171,6 +177,7 @@
> 			reg = <0x0 0x600>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_600>;
>+			cpu-idle-states = <&C4_CPU_SPC &C4_CPU_PC &CLUSTER_PC>;
> 			L2_600: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
>@@ -183,11 +190,66 @@
> 			reg = <0x0 0x700>;
> 			enable-method = "psci";
> 			next-level-cache = <&L2_700>;
>+			cpu-idle-states = <&C4_CPU_SPC &C4_CPU_PC &CLUSTER_PC>;
> 			L2_700: l2-cache {
> 				compatible = "cache";
> 				next-level-cache = <&L3_0>;
> 			};
> 		};
>+
>+		idle-states {
>+			entry-method = "psci";
>+
>+			C0_CPU_SPC: c0_spc {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x40000003>;
>+				entry-latency-us = <350>;
>+				exit-latency-us = <461>;
>+				min-residency-us = <1890>;
>+				local-timer-stop;
>+				idle-state-name = "pc";
>+			};
>+
>+			C0_CPU_PC: c0_pc {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x40000004>;
>+				entry-latency-us = <360>;
>+				exit-latency-us = <531>;
>+				min-residency-us = <3934>;
>+				local-timer-stop;
>+				idle-state-name = "rail pc";
>+			};
>+
>+			C4_CPU_SPC: c4_spc {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x40000003>;
>+				entry-latency-us = <264>;
>+				exit-latency-us = <621>;
>+				min-residency-us = <952>;
>+				local-timer-stop;
>+				idle-state-name = "pc";
>+			};
>+
>+			C4_CPU_PC: c4_pc {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x40000004>;
>+				entry-latency-us = <702>;
>+				exit-latency-us = <1061>;
>+				min-residency-us = <4488>;
>+				local-timer-stop;
>+				idle-state-name = "rail pc";
>+			};
>+
>+			CLUSTER_PC: cluster_pc {
>+				compatible = "arm,idle-state";
>+				arm,psci-suspend-param = <0x400000F4>;
>+				entry-latency-us = <3263>;
>+				exit-latency-us = <6562>;
>+				min-residency-us = <9987>;
>+				local-timer-stop;
>+				idle-state-name = "cluster pc";
>+			};
>+		};
> 	};
>
> 	pmu {
>--
>QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>of the Code Aurora Forum, hosted by The Linux Foundation.
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-10-15 17:02 [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states Raju P.L.S.S.S.N
  2018-10-19 19:48 ` Lina Iyer
@ 2018-10-22 18:34 ` Evan Green
  2018-10-23 18:36 ` Doug Anderson
  2 siblings, 0 replies; 6+ messages in thread
From: Evan Green @ 2018-10-22 18:34 UTC (permalink / raw)
  To: rplsssn
  Cc: Andy Gross, David Brown, linux-arm-msm, linux-soc,
	Rajendra Nayak, Bjorn Andersson, linux-kernel, linux-pm,
	devicetree, sboyd, Doug Anderson, mka, Lina Iyer

On Mon, Oct 15, 2018 at 10:02 AM Raju P.L.S.S.S.N
<rplsssn@codeaurora.org> wrote:
>
> Add device bindings for cpuidle states for cpu devices.
>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 62 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>

Reviewed-by: Evan Green <evgreen@chromium.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-10-15 17:02 [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states Raju P.L.S.S.S.N
  2018-10-19 19:48 ` Lina Iyer
  2018-10-22 18:34 ` Evan Green
@ 2018-10-23 18:36 ` Doug Anderson
  2018-10-30 16:23   ` Raju P L S S S N
  2 siblings, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2018-10-23 18:36 UTC (permalink / raw)
  To: Raju P L S S S N
  Cc: Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, linux-pm, devicetree, Stephen Boyd, Evan Green,
	Matthias Kaehlcke, Lina Iyer

Hi,

On Mon, Oct 15, 2018 at 10:02 AM Raju P.L.S.S.S.N
<rplsssn@codeaurora.org> wrote:
> +               idle-states {
> +                       entry-method = "psci";
> +
> +                       C0_CPU_SPC: c0_spc {

nit: all these nodes should have dashes instead of spaces in the node
names (labels can still have spaces).  AKA:

C0_CPU_SPC: c0-spc {


> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000003>;
> +                               entry-latency-us = <350>;
> +                               exit-latency-us = <461>;
> +                               min-residency-us = <1890>;
> +                               local-timer-stop;
> +                               idle-state-name = "pc";

It seems weird that the idle state with the node name "spc" has the
name "pc" and the idle state with the node name "pc" has the name
"rail pc".  Can this be more consistent or is there a reason why they
need to be mismatched?

Also: AAHTUFSWDKWTM (acronyms are hard to understand for someone who
doesn't know what they mean).  If you really need to use an the
acronyms "PC" and "SPC" please document them somewhere.  In the very
least the commit message, but having a comment in the file is good
too.  ...or (even better) don't use the acronym and spell out what
you're talking about.  Please correct me if I'm wrong, but I don't
think it's obvious what the "PC" and "SPC" idle states mean.


-Doug

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-10-23 18:36 ` Doug Anderson
@ 2018-10-30 16:23   ` Raju P L S S S N
  2018-10-30 16:52     ` Doug Anderson
  0 siblings, 1 reply; 6+ messages in thread
From: Raju P L S S S N @ 2018-10-30 16:23 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, linux-pm, devicetree, Stephen Boyd, Evan Green,
	Matthias Kaehlcke, Lina Iyer



On 10/24/2018 12:06 AM, Doug Anderson wrote:
> Hi,
> 
> On Mon, Oct 15, 2018 at 10:02 AM Raju P.L.S.S.S.N
> <rplsssn@codeaurora.org> wrote:
>> +               idle-states {
>> +                       entry-method = "psci";
>> +
>> +                       C0_CPU_SPC: c0_spc {
> 
> nit: all these nodes should have dashes instead of spaces in the node
> names (labels can still have spaces).  AKA:
> 
> C0_CPU_SPC: c0-spc {

Sure. I will change this. (However, from device tree specification v0.2, 
I see that Table 2.1 mentions underscore as valid character for node. 
Please correct me if I'm missing something)

> 
> 
>> +                               compatible = "arm,idle-state";
>> +                               arm,psci-suspend-param = <0x40000003>;
>> +                               entry-latency-us = <350>;
>> +                               exit-latency-us = <461>;
>> +                               min-residency-us = <1890>;
>> +                               local-timer-stop;
>> +                               idle-state-name = "pc";
> 
> It seems weird that the idle state with the node name "spc" has the
> name "pc" and the idle state with the node name "pc" has the name
> "rail pc".  Can this be more consistent or is there a reason why they
> need to be mismatched?

Agree. They are confusing. Will change this.

> 
> Also: AAHTUFSWDKWTM (acronyms are hard to understand for someone who
> doesn't know what they mean).  If you really need to use an the
> acronyms "PC" and "SPC" please document them somewhere.  In the very
> least the commit message, but having a comment in the file is good
> too.  ...or (even better) don't use the acronym and spell out what
> you're talking about.  Please correct me if I'm wrong, but I don't
> think it's obvious what the "PC" and "SPC" idle states mean.

Sure. Will change this.


Thanks for feedback Doug.

> 
> 
> -Doug
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states
  2018-10-30 16:23   ` Raju P L S S S N
@ 2018-10-30 16:52     ` Doug Anderson
  0 siblings, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2018-10-30 16:52 UTC (permalink / raw)
  To: Raju P L S S S N
  Cc: Andy Gross, David Brown, linux-arm-msm,
	open list:ARM/QUALCOMM SUPPORT, Rajendra Nayak, Bjorn Andersson,
	LKML, linux-pm, devicetree, Stephen Boyd, Evan Green,
	Matthias Kaehlcke, Lina Iyer

Resending since I accidentally clicked back to HTML and lists all bounced.  :(

On Tue, Oct 30, 2018 at 9:24 AM Raju P L S S S N <rplsssn@codeaurora.org> wrote:
>
> On 10/24/2018 12:06 AM, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Oct 15, 2018 at 10:02 AM Raju P.L.S.S.S.N
> > <rplsssn@codeaurora.org> wrote:
> >> +               idle-states {
> >> +                       entry-method = "psci";
> >> +
> >> +                       C0_CPU_SPC: c0_spc {
> >
> > nit: all these nodes should have dashes instead of spaces in the node
> > names (labels can still have spaces).  AKA:
> >
> > C0_CPU_SPC: c0-spc {
>
> Sure. I will change this. (However, from device tree specification v0.2,
> I see that Table 2.1 mentions underscore as valid character for node.
> Please correct me if I'm missing something)

You're right that I don't see it there.  I've always heard that they
are allowed but discouraged.  One place that mentions this:

https://elinux.org/Device_Tree_Linux

> Linux conventions
> * node names
>   * use dash "-" instead of underscore "_"

That same site points that the majority of people use dashes in node
names instead of underscores.

-Doug

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-10-30 16:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-15 17:02 [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states Raju P.L.S.S.S.N
2018-10-19 19:48 ` Lina Iyer
2018-10-22 18:34 ` Evan Green
2018-10-23 18:36 ` Doug Anderson
2018-10-30 16:23   ` Raju P L S S S N
2018-10-30 16:52     ` Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).