| Message ID | 20160717105208.9596-3-bshah@kde.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote: > + > + smd { > + rpm { > + rpm_requests { > + pm8841-regulators { > + s1 { > + regulator-min-microvolt = <675000>; > + regulator-max-microvolt = <1050000>; > + }; Maybe add a label at either the rpm_requests or the pm8841-regulators node so you can add properties in the leaf nodes withoutout having to specify the whole path? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jul 17, 2016 at 09:21:48PM +0200, Arnd Bergmann wrote: > On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote: > > + > > + smd { > > + rpm { > > + rpm_requests { > > + pm8841-regulators { > > + s1 { > > + regulator-min-microvolt = <675000>; > > + regulator-max-microvolt = <1050000>; > > + }; > > Maybe add a label at either the rpm_requests or the pm8841-regulators > node so you can add properties in the leaf nodes withoutout having to > specify the whole path? Sure, I will adjust patch. > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jul 17, 2016 at 8:34 PM, Bhushan Shah <bshah@kde.org> wrote: > On Sun, Jul 17, 2016 at 09:21:48PM +0200, Arnd Bergmann wrote: >> On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote: >> > + >> > + smd { >> > + rpm { >> > + rpm_requests { >> > + pm8841-regulators { >> > + s1 { >> > + regulator-min-microvolt = <675000>; >> > + regulator-max-microvolt = <1050000>; >> > + }; >> >> Maybe add a label at either the rpm_requests or the pm8841-regulators >> node so you can add properties in the leaf nodes withoutout having to >> specify the whole path? > > Sure, I will adjust patch. Please don't. After running into several cases where this would end us up in having a multitude of nodes each describing just a snippet of each level we decided not to do so in the general case for the Qualcomm boards. There are a few where we apparently ended up doing so anyways, but for all other cases of regulators we express the full tree in the dts, so please follow that so we don't mix the styles too much. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday, July 17, 2016 8:44:01 PM CEST Bjorn Andersson wrote: > On Sun, Jul 17, 2016 at 8:34 PM, Bhushan Shah <bshah@kde.org> wrote: > > On Sun, Jul 17, 2016 at 09:21:48PM +0200, Arnd Bergmann wrote: > >> On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote: > >> > + > >> > + smd { > >> > + rpm { > >> > + rpm_requests { > >> > + pm8841-regulators { > >> > + s1 { > >> > + regulator-min-microvolt = <675000>; > >> > + regulator-max-microvolt = <1050000>; > >> > + }; > >> > >> Maybe add a label at either the rpm_requests or the pm8841-regulators > >> node so you can add properties in the leaf nodes withoutout having to > >> specify the whole path? > > > > Sure, I will adjust patch. > > Please don't. After running into several cases where this would end us > up in having a multitude of nodes each describing just a snippet of > each level we decided not to do so in the general case for the > Qualcomm boards. > > There are a few where we apparently ended up doing so anyways, but for > all other cases of regulators we express the full tree in the dts, so > please follow that so we don't mix the styles too much. Ok, then how about this instead: /smd/rpm/rpm_requests/pm8841-regulators { s1 { regulator-min-microvolt = <675000>; regulator-max-microvolt = <1050000>; }; ... }; That avoids the ridiculous intendation level but uses no labels. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 18 Jul 00:44 PDT 2016, Arnd Bergmann wrote: > On Sunday, July 17, 2016 8:44:01 PM CEST Bjorn Andersson wrote: > > On Sun, Jul 17, 2016 at 8:34 PM, Bhushan Shah <bshah@kde.org> wrote: > > > On Sun, Jul 17, 2016 at 09:21:48PM +0200, Arnd Bergmann wrote: > > >> On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote: > > >> > + > > >> > + smd { > > >> > + rpm { > > >> > + rpm_requests { > > >> > + pm8841-regulators { > > >> > + s1 { > > >> > + regulator-min-microvolt = <675000>; > > >> > + regulator-max-microvolt = <1050000>; > > >> > + }; > > >> > > >> Maybe add a label at either the rpm_requests or the pm8841-regulators > > >> node so you can add properties in the leaf nodes withoutout having to > > >> specify the whole path? > > > > > > Sure, I will adjust patch. > > > > Please don't. After running into several cases where this would end us > > up in having a multitude of nodes each describing just a snippet of > > each level we decided not to do so in the general case for the > > Qualcomm boards. > > > > There are a few where we apparently ended up doing so anyways, but for > > all other cases of regulators we express the full tree in the dts, so > > please follow that so we don't mix the styles too much. > > Ok, then how about this instead: > > /smd/rpm/rpm_requests/pm8841-regulators { The problem I have with this is that in the dtsi we have properties and other nodes under each one of these, hence we end up with completely different overall structure depending on if I look in the dtsi or the dts. The problem I have with it in the dts is that we have properties and nodes under "smd" and "rpm_requests". So siblings are no longer grouped together. I have a hard time finding my way through flattened trees, often spread out over multiple files, that I need to puzzle together in my head. Perhaps there are better ways to keep this comprehensible, without maintaining the structure. > s1 { > regulator-min-microvolt = <675000>; > regulator-max-microvolt = <1050000>; > }; > > ... > }; > > That avoids the ridiculous intendation level but uses no labels. > I do share your dislike of the indentation level. I do have a few other concerns about style and scalability in other places. How about we follow how I've done this in the other files for now (i.e. keep the structure of the patch as is) and sit down at LAS16 to discuss what to do about this? Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun 17 Jul 03:52 PDT 2016, Bhushan Shah wrote: [..] > diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts > index 88d494f..29fa0bb 100644 > --- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts > +++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts > @@ -1,6 +1,8 @@ > #include "qcom-msm8974.dtsi" > #include "qcom-pm8841.dtsi" > #include "qcom-pm8941.dtsi" > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> > > / { > model = "LGE MSM 8974 HAMMERHEAD"; > @@ -14,6 +16,234 @@ > chosen { > stdout-path = "serial0:115200n8"; > }; > + > + smd { > + rpm { > + rpm_requests { > + pm8841-regulators { [..] > + }; > + > + pm8941-regulators { > + vdd_l1_l3-supply = <&pm8941_s1>; > + vdd_l2_lvs1_2_3-supply = <&pm8941_s3>; > + vdd_l4_l11-supply = <&pm8941_s1>; > + vdd_l5_l7-supply = <&pm8941_s2>; > + vdd_l6_l12_l14_l15-supply = <&pm8941_s2>; > + vdd_l8_l16_l18_l19-supply = <&vph_pwr_reg>; > + vdd_l9_l10_l17_l22-supply = <&vreg_boost>; > + vdd_l13_l20_l23_l24-supply = <&vreg_boost>; > + vdd_l21-supply = <&vreg_boost>; > + [..] > + }; > + > + vreg_boost: vreg-boost { > + compatible = "regulator-fixed"; > + > + regulator-name = "vreg-boost"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; vreg_boost is not to be confused with the boost regulator (PM8941 S4), this boot regulator is responsible for keeping voltage high enough for e.g. eMMC operation during voltage drops of the battery during high current operations. I have it listed in the Sony file as 3.15V, which seems more reasonable (I can't find the number in the Qcom docs right now). As this has turned out to be part of the Qualcomm platform rather than something Sony specific I believe you should add a patch before this one that moves the vreg-boost node from the honami.dts to the msm8974.dtsi. > + > + regulator-always-on; > + regulator-boot-on; > + > + gpio = <&pm8941_gpios 21 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&boost_bypass_n_pin>; > + }; > + > + vph_pwr_reg: vph_pwr_reg { This is also Qualcomm generic, I just skipped it in the Honami case. But please add this to the msm8974.dtsi instead. You are not allowed to use _ in node names (but should do so in label names), also just to follow naming please name this: vreg_vph_pwr: vreg-vph-pwr { ... }; > + compatible = "regulator-fixed"; > + regulator-name = "vph-pwr"; > + > + regulator-min-microvolt = <3600000>; > + regulator-max-microvolt = <3600000>; > + > + regulator-always-on; > + }; > }; > > &soc { > @@ -23,3 +253,14 @@ > }; > > }; > + > +&spmi_bus { > + pm8941@0 { > + gpios@c000 { > + boost_bypass_n_pin: boost-bypass { > + pins = "gpio21"; > + function = "normal"; > + }; > + }; This is also part of the platform, so move it from honami.dts to qcom-pm8941.dtsi when you move the vreg above. > + }; > +}; Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, July 18, 2016 10:11:55 AM CEST Bjorn Andersson wrote: > On Mon 18 Jul 00:44 PDT 2016, Arnd Bergmann wrote: > > On Sunday, July 17, 2016 8:44:01 PM CEST Bjorn Andersson wrote: > > Ok, then how about this instead: > > > > /smd/rpm/rpm_requests/pm8841-regulators { > > The problem I have with this is that in the dtsi we have properties and > other nodes under each one of these, hence we end up with completely > different overall structure depending on if I look in the dtsi or the > dts. > > The problem I have with it in the dts is that we have properties and > nodes under "smd" and "rpm_requests". So siblings are no longer grouped > together. > > > I have a hard time finding my way through flattened trees, often spread out > over multiple files, that I need to puzzle together in my head. Perhaps > there are better ways to keep this comprehensible, without maintaining > the structure. > > > s1 { > > regulator-min-microvolt = <675000>; > > regulator-max-microvolt = <1050000>; > > }; > > > > ... > > }; > > > > That avoids the ridiculous intendation level but uses no labels. > > > > I do share your dislike of the indentation level. > > > I do have a few other concerns about style and scalability in other > places. How about we follow how I've done this in the other files for > now (i.e. keep the structure of the patch as is) and sit down at LAS16 > to discuss what to do about this? Fair enough, let's do that for now. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts index 88d494f..29fa0bb 100644 --- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts +++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts @@ -1,6 +1,8 @@ #include "qcom-msm8974.dtsi" #include "qcom-pm8841.dtsi" #include "qcom-pm8941.dtsi" +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> / { model = "LGE MSM 8974 HAMMERHEAD"; @@ -14,6 +16,234 @@ chosen { stdout-path = "serial0:115200n8"; }; + + smd { + rpm { + rpm_requests { + pm8841-regulators { + s1 { + regulator-min-microvolt = <675000>; + regulator-max-microvolt = <1050000>; + }; + + s2 { + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <1050000>; + }; + + s3 { + regulator-min-microvolt = <1050000>; + regulator-max-microvolt = <1050000>; + }; + + s4 { + regulator-min-microvolt = <815000>; + regulator-max-microvolt = <900000>; + }; + }; + + pm8941-regulators { + vdd_l1_l3-supply = <&pm8941_s1>; + vdd_l2_lvs1_2_3-supply = <&pm8941_s3>; + vdd_l4_l11-supply = <&pm8941_s1>; + vdd_l5_l7-supply = <&pm8941_s2>; + vdd_l6_l12_l14_l15-supply = <&pm8941_s2>; + vdd_l8_l16_l18_l19-supply = <&vph_pwr_reg>; + vdd_l9_l10_l17_l22-supply = <&vreg_boost>; + vdd_l13_l20_l23_l24-supply = <&vreg_boost>; + vdd_l21-supply = <&vreg_boost>; + + s1 { + regulator-min-microvolt = <1300000>; + regulator-max-microvolt = <1300000>; + + regulator-always-on; + regulator-boot-on; + }; + + s2 { + regulator-min-microvolt = <2150000>; + regulator-max-microvolt = <2150000>; + + regulator-boot-on; + }; + + s3 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + regulator-always-on; + regulator-boot-on; + }; + + l1 { + regulator-min-microvolt = <1225000>; + regulator-max-microvolt = <1225000>; + + regulator-always-on; + regulator-boot-on; + }; + + l2 { + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + + l3 { + regulator-min-microvolt = <1225000>; + regulator-max-microvolt = <1225000>; + }; + + l4 { + regulator-min-microvolt = <1225000>; + regulator-max-microvolt = <1225000>; + }; + + l5 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + l6 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + regulator-boot-on; + }; + + l7 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + regulator-boot-on; + }; + + l8 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + l9 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2950000>; + }; + + l10 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2950000>; + }; + + l11 { + regulator-min-microvolt = <1300000>; + regulator-max-microvolt = <1300000>; + }; + + l12 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + regulator-always-on; + regulator-boot-on; + }; + + l13 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2950000>; + + regulator-boot-on; + }; + + l14 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + l15 { + regulator-min-microvolt = <2050000>; + regulator-max-microvolt = <2050000>; + }; + + l16 { + regulator-min-microvolt = <2700000>; + regulator-max-microvolt = <2700000>; + }; + + l17 { + regulator-min-microvolt = <2850000>; + regulator-max-microvolt = <2850000>; + }; + + l18 { + regulator-min-microvolt = <2850000>; + regulator-max-microvolt = <2850000>; + }; + + l19 { + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3300000>; + }; + + l20 { + regulator-min-microvolt = <2950000>; + regulator-max-microvolt = <2950000>; + + regulator-boot-on; + }; + + l21 { + regulator-min-microvolt = <2950000>; + regulator-max-microvolt = <2950000>; + + regulator-boot-on; + }; + + l22 { + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3300000>; + }; + + l23 { + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + }; + + l24 { + regulator-min-microvolt = <3075000>; + regulator-max-microvolt = <3075000>; + + regulator-boot-on; + }; + }; + }; + }; + }; + + vreg_boost: vreg-boost { + compatible = "regulator-fixed"; + + regulator-name = "vreg-boost"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + + regulator-always-on; + regulator-boot-on; + + gpio = <&pm8941_gpios 21 GPIO_ACTIVE_HIGH>; + enable-active-high; + + pinctrl-names = "default"; + pinctrl-0 = <&boost_bypass_n_pin>; + }; + + vph_pwr_reg: vph_pwr_reg { + compatible = "regulator-fixed"; + regulator-name = "vph-pwr"; + + regulator-min-microvolt = <3600000>; + regulator-max-microvolt = <3600000>; + + regulator-always-on; + }; }; &soc { @@ -23,3 +253,14 @@ }; }; + +&spmi_bus { + pm8941@0 { + gpios@c000 { + boost_bypass_n_pin: boost-bypass { + pins = "gpio21"; + function = "normal"; + }; + }; + }; +};
Cc: Andy Gross <andy.gross@linaro.org> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: David Brown <david.brown@linaro.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Russell King <linux@armlinux.org.uk> Cc: linux-arm-msm@vger.kernel.org Cc: linux-soc@vger.kernel.org Cc: devicetree@vger.kernel.org Signed-off-by: Bhushan Shah <bshah@kde.org> --- .../dts/qcom-msm8974-lge-nexus5-hammerhead.dts | 241 +++++++++++++++++++++ 1 file changed, 241 insertions(+)