Patchwork [v5,7/8] arm64: dts: sdm845: Add rpmh powercontroller node

login
register
mail settings
Submitter Rajendra Nayak
Date Dec. 4, 2018, 5:21 a.m.
Message ID <20181204052119.806-8-rnayak@codeaurora.org>
Download mbox | patch
Permalink /patch/671575/
State New
Headers show

Comments

Rajendra Nayak - Dec. 4, 2018, 5:21 a.m.
Add the DT node for the rpmhpd powercontroller.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 51 ++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
Viresh Kumar - Dec. 4, 2018, 5:25 a.m.
On 04-12-18, 10:51, Rajendra Nayak wrote:
> Add the DT node for the rpmhpd powercontroller.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 51 ++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Stephen Boyd - Dec. 4, 2018, 11:16 p.m.
Quoting Rajendra Nayak (2018-12-03 21:21:18)
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index b72bdb0a31a5..a6d0cd8d17b0 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1324,6 +1325,56 @@
>                                 compatible = "qcom,sdm845-rpmh-clk";
>                                 #clock-cells = <1>;
>                         };
> +
> +                       rpmhpd: power-controller {
> +                               compatible = "qcom,sdm845-rpmhpd";
> +                               #power-domain-cells = <1>;
> +                               operating-points-v2 = <&rpmhpd_opp_table>;
> +                       };
> +
> +                       rpmhpd_opp_table: opp-table {

This table should go somewhere else? I don't understand why it's in the
rpmh node because it's not an rpmh device. Does it go to the root? Or
does it go under rpmhpd itself? I'm not sure.
Rajendra Nayak - Dec. 5, 2018, 7:07 a.m.
On 12/5/2018 4:46 AM, Stephen Boyd wrote:
> Quoting Rajendra Nayak (2018-12-03 21:21:18)
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> index b72bdb0a31a5..a6d0cd8d17b0 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> @@ -1324,6 +1325,56 @@
>>                                  compatible = "qcom,sdm845-rpmh-clk";
>>                                  #clock-cells = <1>;
>>                          };
>> +
>> +                       rpmhpd: power-controller {
>> +                               compatible = "qcom,sdm845-rpmhpd";
>> +                               #power-domain-cells = <1>;
>> +                               operating-points-v2 = <&rpmhpd_opp_table>;
>> +                       };
>> +
>> +                       rpmhpd_opp_table: opp-table {
> 
> This table should go somewhere else? I don't understand why it's in the
> rpmh node because it's not an rpmh device. Does it go to the root? Or
> does it go under rpmhpd itself? I'm not sure.

I could move it to root perhaps, we seem to do that atleast in the case of
GPU. The power domain bindings (Documentation/devicetree/bindings/power/power_domain.txt)
seem to suggest it can't be under the power-controller node itself.
Rob Herring - Dec. 7, 2018, 5:36 p.m.
On Wed, Dec 05, 2018 at 12:37:29PM +0530, Rajendra Nayak wrote:
> 
> 
> On 12/5/2018 4:46 AM, Stephen Boyd wrote:
> > Quoting Rajendra Nayak (2018-12-03 21:21:18)
> > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > index b72bdb0a31a5..a6d0cd8d17b0 100644
> > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > @@ -1324,6 +1325,56 @@
> > >                                  compatible = "qcom,sdm845-rpmh-clk";
> > >                                  #clock-cells = <1>;
> > >                          };
> > > +
> > > +                       rpmhpd: power-controller {
> > > +                               compatible = "qcom,sdm845-rpmhpd";
> > > +                               #power-domain-cells = <1>;
> > > +                               operating-points-v2 = <&rpmhpd_opp_table>;
> > > +                       };
> > > +
> > > +                       rpmhpd_opp_table: opp-table {
> > 
> > This table should go somewhere else? I don't understand why it's in the
> > rpmh node because it's not an rpmh device. Does it go to the root? Or
> > does it go under rpmhpd itself? I'm not sure.
> 
> I could move it to root perhaps, we seem to do that atleast in the case of
> GPU. The power domain bindings (Documentation/devicetree/bindings/power/power_domain.txt)
> seem to suggest it can't be under the power-controller node itself.

Why not? I don't see anything forbidding that like already having some 
other type of child nodes. It's a little weird to have 
operating-points-v2 point to a child node, but that will still work.

Rob

Patch

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index b72bdb0a31a5..a6d0cd8d17b0 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -10,6 +10,7 @@ 
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/phy/phy-qcom-qusb2.h>
+#include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/reset/qcom,sdm845-aoss.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
 
@@ -1324,6 +1325,56 @@ 
 				compatible = "qcom,sdm845-rpmh-clk";
 				#clock-cells = <1>;
 			};
+
+			rpmhpd: power-controller {
+				compatible = "qcom,sdm845-rpmhpd";
+				#power-domain-cells = <1>;
+				operating-points-v2 = <&rpmhpd_opp_table>;
+			};
+
+			rpmhpd_opp_table: opp-table {
+				compatible = "operating-points-v2-qcom-level";
+
+				rpmhpd_opp_ret: opp1 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_RETENTION>;
+				};
+
+				rpmhpd_opp_min_svs: opp2 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
+				};
+
+				rpmhpd_opp_low_svs: opp3 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+				};
+
+				rpmhpd_opp_svs: opp4 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_SVS>;
+				};
+
+				rpmhpd_opp_svs_l1: opp5 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+				};
+
+				rpmhpd_opp_nom: opp6 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_NOM>;
+				};
+
+				rpmhpd_opp_nom_l1: opp7 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_NOM_L1>;
+				};
+
+				rpmhpd_opp_nom_l2: opp8 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_NOM_L2>;
+				};
+
+				rpmhpd_opp_turbo: opp9 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_TURBO>;
+				};
+
+				rpmhpd_opp_turbo_l1: opp10 {
+					qcom,level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;
+				};
+			};
 		};
 
 		intc: interrupt-controller@17a00000 {