Messages in this thread Patch in this message | | | From | Stephen Boyd <> | Date | Thu, 28 Oct 2021 13:40:04 -0700 | Subject | Re: [PATCH V3 4/4] arm64: dts: qcom: sc7280: Add pm8008 regulators support for sc7280-idp |
| |
Quoting Satya Priya (2021-10-28 08:14:32) > Add pm8008 regulators support for sc7280 idp. > > Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> > --- > Changes in V2: > - As per Stephen's comments, replaced '_' with '-' for node names. > > Changes in V3: > - Changed the regulator node names as l1, l2 etc > - Changed "pm8008-regulators" to "regulators" > - Changed "qcom,min-dropout-voltage" to "regulator-min-dropout-voltage-microvolt" > > arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 103 +++++++++++++++++++++++++++++++ > 1 file changed, 103 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > index d623d71..493575b 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > @@ -309,6 +309,97 @@ > }; > }; > > +&i2c1 { > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; > + > + pm8008_chip: pm8008@8 {
If this is going to be copy/pasted wherever devices that use pm8008 live then it's probably better to make a new file like we do for other pmics. Maybe something like
&pm8008_i2c { <All the generic stuff in here like reg properties and address/size cells and compatible> };
and then have each board set the min/max voltages and min dropout properties. Then we can include the pm8008.dtsi file after defining which i2c bus it lives on.
pm8008_i2c: i2c5 { }; #include "pm8008.dtsi"
...
&pm8008_l1 { regulator-min-microvolt = <...>; ... };
> + compatible = "qcom,pm8008"; > + reg = <0x8>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&pm8008_active>; > + }; > + > + pm8008_ldo: pm8008@9 { > + compatible = "qcom,pm8008"; > + reg = <0x9>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + regulators { > + compatible = "qcom,pm8008-regulator"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + vdd_l1_l2-supply = <&vreg_s8b_1p2>; > + vdd_l3_l4-supply = <&vreg_s1b_1p8>; > + vdd_l5-supply = <&vreg_bob>; > + vdd_l6-supply = <&vreg_bob>; > + vdd_l7-supply = <&vreg_bob>; > + > + pm8008_l1: l1@4000 { > + reg = <0x4000>; > + regulator-name = "pm8008_l1"; > + regulator-min-microvolt = <950000>; > + regulator-max-microvolt = <1300000>; > + regulator-min-dropout-voltage-microvolt = <96000>; > + }; > + > + pm8008_l2: l2@4100 { > + reg = <0x4100>; > + regulator-name = "pm8008_l2"; > + regulator-min-microvolt = <950000>; > + regulator-max-microvolt = <1250000>; > + regulator-min-dropout-voltage-microvolt = <24000>; > + }; > + > + pm8008_l3: l3@4200 { > + reg = <0x4200>; > + regulator-name = "pm8008_l3"; > + regulator-min-microvolt = <1650000>; > + regulator-max-microvolt = <3000000>; > + regulator-min-dropout-voltage-microvolt = <224000>; > + }; > + > + pm8008_l4: l4@4300 { > + reg = <0x4300>; > + regulator-name = "pm8008_l4"; > + regulator-min-microvolt = <1504000>; > + regulator-max-microvolt = <1600000>; > + regulator-min-dropout-voltage-microvolt = <0>; > + }; > + > + pm8008_l5: l5@4400 { > + reg = <0x4400>; > + regulator-name = "pm8008_l5"; > + regulator-min-microvolt = <2600000>; > + regulator-max-microvolt = <3000000>; > + regulator-min-dropout-voltage-microvolt = <104000>; > + }; > + > + pm8008_l6: l6@4500 { > + reg = <0x4500>; > + regulator-name = "pm8008_l6"; > + regulator-min-microvolt = <2600000>; > + regulator-max-microvolt = <3000000>; > + regulator-min-dropout-voltage-microvolt = <112000>; > + }; > + > + pm8008_l7: l7@4600 { > + reg = <0x4600>; > + regulator-name = "pm8008_l7"; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3544000>; > + regulator-min-dropout-voltage-microvolt = <96000>; > + }; > + }; > + }; > +}; > + > &qfprom { > vcc-supply = <&vreg_l1c_1p8>; > }; > @@ -437,6 +528,18 @@ > }; > }; > > +&pm8350c_gpios { > + pm8008-reset {
Why is it a subnode of a subnode? Shouldn't it be pm8008-active directly underneath pm8350c_gpios?
> + pm8008_active: pm8008-active { > + pins = "gpio4"; > + function = "normal"; > + bias-disable; > + output-high; > + power-source = <0>; > + }; > + }; > +}; > + > &qspi_cs0 { > bias-disable; > };
| |