From mboxrd@z Thu Jan 1 00:00:00 1970 From: Niklas Cassel Subject: Re: [PATCHv1 4/8] arm64: dts: qcom: msm8916: Use more generic idle state names Date: Tue, 14 May 2019 18:12:20 +0200 Message-ID: <20190514161220.GC1824@centauri.ideon.se> References: <2a0626da4d8d5a1018c351b24b63e5e0d7a45a10.1557486950.git.amit.kucheria@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <2a0626da4d8d5a1018c351b24b63e5e0d7a45a10.1557486950.git.amit.kucheria@linaro.org> Sender: linux-kernel-owner@vger.kernel.org To: Amit Kucheria Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, bjorn.andersson@linaro.org, andy.gross@linaro.org, David Brown , Li Yang , Shawn Guo , devicetree@vger.kernel.org List-Id: devicetree@vger.kernel.org On Fri, May 10, 2019 at 04:59:42PM +0530, Amit Kucheria wrote: > Instead of using Qualcomm-specific terminology, use generic node names > for the idle states that are easier to understand. Move the description > into the "idle-state-name" property. > > Signed-off-by: Amit Kucheria > --- > arch/arm64/boot/dts/qcom/msm8916.dtsi | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi > index ded1052e5693..400b609bb3fd 100644 > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi > @@ -110,7 +110,7 @@ > reg = <0x0>; > next-level-cache = <&L2_0>; > enable-method = "psci"; > - cpu-idle-states = <&CPU_SPC>; > + cpu-idle-states = <&CPU_SLEEP_0>; > clocks = <&apcs>; > operating-points-v2 = <&cpu_opp_table>; > #cooling-cells = <2>; > @@ -122,7 +122,7 @@ > reg = <0x1>; > next-level-cache = <&L2_0>; > enable-method = "psci"; > - cpu-idle-states = <&CPU_SPC>; > + cpu-idle-states = <&CPU_SLEEP_0>; > clocks = <&apcs>; > operating-points-v2 = <&cpu_opp_table>; > #cooling-cells = <2>; > @@ -134,7 +134,7 @@ > reg = <0x2>; > next-level-cache = <&L2_0>; > enable-method = "psci"; > - cpu-idle-states = <&CPU_SPC>; > + cpu-idle-states = <&CPU_SLEEP_0>; > clocks = <&apcs>; > operating-points-v2 = <&cpu_opp_table>; > #cooling-cells = <2>; > @@ -146,7 +146,7 @@ > reg = <0x3>; > next-level-cache = <&L2_0>; > enable-method = "psci"; > - cpu-idle-states = <&CPU_SPC>; > + cpu-idle-states = <&CPU_SLEEP_0>; > clocks = <&apcs>; > operating-points-v2 = <&cpu_opp_table>; > #cooling-cells = <2>; > @@ -160,8 +160,9 @@ > idle-states { > entry-method="psci"; Please add a space before and after "=". > > - CPU_SPC: spc { > + CPU_SLEEP_0: cpu-sleep-0 { While I like your idea of using power state names from Server Base System Architecture document (SBSA) where applicable, does each qcom power state have a matching state in SBSA? These are the qcom power states: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/Documentation/devicetree/bindings/arm/msm/lpm-levels.txt?h=msm-4.4#n53 Note that qcom defines: "wfi", "retention", "gdhs", "pc", "fpc" while SBSA simply defines "idle_standby" (aka wfi), "idle_retention", "sleep". Unless you know the equivalent name for each qcom power state (perhaps several qcom power states are really the same SBSA state?), I think that you should omit the renaming from this patch series. > compatible = "arm,idle-state"; > + idle-state-name = "standalone-power-collapse"; > arm,psci-suspend-param = <0x40000002>; > entry-latency-us = <130>; > exit-latency-us = <150>; > -- > 2.17.1 >