| Message ID | 1539622953-4188-1-git-send-email-rplsssn@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: sdm845: Add PSCI cpuidle low power states | expand |
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. >
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>
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
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 >
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
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 {
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(+)