| Message ID | 20211215003639.386460-1-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: sc7180: Add board regulators for MIPI camera trogdor boards | expand |
On Tue, Dec 14, 2021 at 04:36:38PM -0800, Stephen Boyd wrote: > Some trogdor boards have on-board regulators for the MIPI camera > components. Add nodes describing these regulators so boards with these > supplies can consume them. > > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > .../boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 16 +++ > .../dts/qcom/sc7180-trogdor-homestar.dtsi | 16 +++ > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 122 ++++++++++++++++++ > 3 files changed, 154 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi > index 14ed09f30a73..c81805ef2250 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi > @@ -142,6 +142,22 @@ skin-temp-thermistor@1 { > }; > }; > > +&pp1800_uf_cam { > + status = "okay"; > +}; > + > +&pp1800_wf_cam { > + status = "okay"; > +}; > + > +&pp2800_uf_cam { > + status = "okay"; > +}; > + > +&pp2800_wf_cam { > + status = "okay"; > +}; > + > &pp3300_dx_edp { > gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>; > }; > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi > index 4ab890b2a1d4..9110fed291c4 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi > @@ -149,6 +149,22 @@ skin-temp-thermistor@1 { > }; > }; > > +&pp1800_uf_cam { > + status = "okay"; > +}; > + > +&pp1800_wf_cam { > + status = "okay"; > +}; > + > +&pp2800_uf_cam { > + status = "okay"; > +}; > + > +&pp2800_wf_cam { > + status = "okay"; > +}; > + > &pp3300_dx_edp { > gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>; > }; > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > index d4f4441179fc..1dd8e35093a8 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > @@ -113,6 +113,40 @@ src_vph_pwr: src-vph-pwr-regulator { > vin-supply = <&ppvar_sys>; > }; > > + pp1800_uf_cam: pp1800-uf-cam-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "pp1800_uf_cam"; > + status = "disabled"; > + > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + > + gpio = <&tlmm 6 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + pinctrl-names = "default"; > + pinctrl-0 = <&uf_cam_en>; > + > + vin-supply = <&pp1800_ldo>; > + regulator-enable-ramp-delay = <1000>; > + }; > + > + pp1800_wf_cam: pp1800-wf-cam-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "pp1800_wf_cam"; > + status = "disabled"; > + > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + > + gpio = <&tlmm 7 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + pinctrl-names = "default"; > + pinctrl-0 = <&wf_cam_en>; > + > + vin-supply = <&pp1800_ldo>; > + regulator-enable-ramp-delay = <1000>; > + }; > + Shouldn't 'pp1800_ldo' be defined before these ("FIXED REGULATORS - parents above children")? I suggest to move these two below the top level regulators, i.e. somwhere after pp3300_a (probably pp3300_a and pp5000_a should be swapped, but that's beyond the scope of this patch). > pp5000_a: pp5000-a-regulator { > compatible = "regulator-fixed"; > regulator-name = "pp5000_a"; > @@ -144,6 +178,66 @@ pp3300_a: pp3300-a-regulator { > vin-supply = <&ppvar_sys>; > }; > > + pp1800_ec: > + pp1800_sensors: > + pp1800_ldo: pp1800-ldo-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "pp1800_ldo"; > + > + /* EC turns on with hibernate_l; always on for AP */ > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + > + /* > + * Actually should be pp1800_h1 but we don't have any need to > + * model that so we use the parent of pp1800_h1. > + */ > + vin-supply = <&pp3300_a>; > + }; > + > + pp2800_uf_cam: pp2800-uf-cam-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "pp2800_uf_cam"; > + status = "disabled"; > + > + regulator-min-microvolt = <2850000>; > + regulator-max-microvolt = <2850000>; > + > + gpio = <&tlmm 6 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + /* > + * The pinconf can only be referenced once so we put it on the > + * first regulator and comment it out here. > + * pinctrl-names = "default"; > + * pinctrl-0 = <&uf_cam_en>; > + */ > + > + vin-supply = <&pp3300_a>; > + }; > + > + pp2800_vcm_wf_cam: > + pp2800_wf_cam: pp2800-wf-cam-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "pp2800_wf_cam"; > + status = "disabled"; > + > + regulator-min-microvolt = <2850000>; > + regulator-max-microvolt = <2850000>; > + > + gpio = <&tlmm 7 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + /* > + * The pinconf can only be referenced once so we put it on the > + * first regulator and comment it out here. > + * pinctrl-names = "default"; > + * pinctrl-0 = <&wf_cam_en>; > + */ > + > + vin-supply = <&pp3300_a>; > + }; > + > pp3300_audio: > pp3300_codec: pp3300-codec-regulator { > compatible = "regulator-fixed"; > @@ -1517,4 +1611,32 @@ pinconf-sd-cd { > drive-strength = <2>; > }; > }; > + > + uf_cam_en: uf-cam-en { > + pinmux { > + pins = "gpio6"; > + function = "gpio"; > + }; > + > + pinconf { > + pins = "gpio6"; > + drive-strength = <2>; > + /* External pull down */ Is there actually an external pull down? > + bias-disable; > + }; > + }; > + > + wf_cam_en: wf-cam-en { > + pinmux { > + pins = "gpio7"; > + function = "gpio"; > + }; > + > + pinconf { > + pins = "gpio7"; > + drive-strength = <2>; > + /* External pull down */ ditto
Quoting Matthias Kaehlcke (2021-12-15 18:31:41) > On Tue, Dec 14, 2021 at 04:36:38PM -0800, Stephen Boyd wrote: > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > index d4f4441179fc..1dd8e35093a8 100644 > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > @@ -113,6 +113,40 @@ src_vph_pwr: src-vph-pwr-regulator { > > vin-supply = <&ppvar_sys>; > > }; > > > > + pp1800_uf_cam: pp1800-uf-cam-regulator { > > + compatible = "regulator-fixed"; > > + regulator-name = "pp1800_uf_cam"; > > + status = "disabled"; > > + > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + > > + gpio = <&tlmm 6 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&uf_cam_en>; > > + > > + vin-supply = <&pp1800_ldo>; > > + regulator-enable-ramp-delay = <1000>; > > + }; > > + > > + pp1800_wf_cam: pp1800-wf-cam-regulator { > > + compatible = "regulator-fixed"; > > + regulator-name = "pp1800_wf_cam"; > > + status = "disabled"; > > + > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + > > + gpio = <&tlmm 7 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&wf_cam_en>; > > + > > + vin-supply = <&pp1800_ldo>; > > + regulator-enable-ramp-delay = <1000>; > > + }; > > + > > Shouldn't 'pp1800_ldo' be defined before these ("FIXED REGULATORS > - parents above children")? Good catch! Fixed it. > > I suggest to move these two below the top level regulators, i.e. > somwhere after pp3300_a (probably pp3300_a and pp5000_a should be > swapped, but that's beyond the scope of this patch). > > > pp5000_a: pp5000-a-regulator { > > compatible = "regulator-fixed"; > > regulator-name = "pp5000_a"; > > @@ -1517,4 +1611,32 @@ pinconf-sd-cd { > > drive-strength = <2>; > > }; > > }; > > + > > + uf_cam_en: uf-cam-en { > > + pinmux { > > + pins = "gpio6"; > > + function = "gpio"; > > + }; > > + > > + pinconf { > > + pins = "gpio6"; > > + drive-strength = <2>; > > + /* External pull down */ > > Is there actually an external pull down? My understanding is that there's an internal pull down in the LDO so while it isn't exactly "external" in the sense there's a pull down on the net via a resistor, there's still a pull down that you can't see in the schematic in the IC.
On Wed, Dec 15, 2021 at 08:43:21PM -0800, Stephen Boyd wrote: > Quoting Matthias Kaehlcke (2021-12-15 18:31:41) > > On Tue, Dec 14, 2021 at 04:36:38PM -0800, Stephen Boyd wrote: > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > > index d4f4441179fc..1dd8e35093a8 100644 > > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi > > > @@ -113,6 +113,40 @@ src_vph_pwr: src-vph-pwr-regulator { > > > vin-supply = <&ppvar_sys>; > > > }; > > > > > > + pp1800_uf_cam: pp1800-uf-cam-regulator { > > > + compatible = "regulator-fixed"; > > > + regulator-name = "pp1800_uf_cam"; > > > + status = "disabled"; > > > + > > > + regulator-min-microvolt = <1800000>; > > > + regulator-max-microvolt = <1800000>; > > > + > > > + gpio = <&tlmm 6 GPIO_ACTIVE_HIGH>; > > > + enable-active-high; > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&uf_cam_en>; > > > + > > > + vin-supply = <&pp1800_ldo>; > > > + regulator-enable-ramp-delay = <1000>; > > > + }; > > > + > > > + pp1800_wf_cam: pp1800-wf-cam-regulator { > > > + compatible = "regulator-fixed"; > > > + regulator-name = "pp1800_wf_cam"; > > > + status = "disabled"; > > > + > > > + regulator-min-microvolt = <1800000>; > > > + regulator-max-microvolt = <1800000>; > > > + > > > + gpio = <&tlmm 7 GPIO_ACTIVE_HIGH>; > > > + enable-active-high; > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&wf_cam_en>; > > > + > > > + vin-supply = <&pp1800_ldo>; > > > + regulator-enable-ramp-delay = <1000>; > > > + }; > > > + > > > > Shouldn't 'pp1800_ldo' be defined before these ("FIXED REGULATORS > > - parents above children")? > > Good catch! Fixed it. > > > > > I suggest to move these two below the top level regulators, i.e. > > somwhere after pp3300_a (probably pp3300_a and pp5000_a should be > > swapped, but that's beyond the scope of this patch). > > > > > pp5000_a: pp5000-a-regulator { > > > compatible = "regulator-fixed"; > > > regulator-name = "pp5000_a"; > > > @@ -1517,4 +1611,32 @@ pinconf-sd-cd { > > > drive-strength = <2>; > > > }; > > > }; > > > + > > > + uf_cam_en: uf-cam-en { > > > + pinmux { > > > + pins = "gpio6"; > > > + function = "gpio"; > > > + }; > > > + > > > + pinconf { > > > + pins = "gpio6"; > > > + drive-strength = <2>; > > > + /* External pull down */ > > > > Is there actually an external pull down? > > My understanding is that there's an internal pull down in the LDO so > while it isn't exactly "external" in the sense there's a pull down on > the net via a resistor, there's still a pull down that you can't see in > the schematic in the IC. Thanks for the clarification!
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi index 14ed09f30a73..c81805ef2250 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi @@ -142,6 +142,22 @@ skin-temp-thermistor@1 { }; }; +&pp1800_uf_cam { + status = "okay"; +}; + +&pp1800_wf_cam { + status = "okay"; +}; + +&pp2800_uf_cam { + status = "okay"; +}; + +&pp2800_wf_cam { + status = "okay"; +}; + &pp3300_dx_edp { gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>; }; diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi index 4ab890b2a1d4..9110fed291c4 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi @@ -149,6 +149,22 @@ skin-temp-thermistor@1 { }; }; +&pp1800_uf_cam { + status = "okay"; +}; + +&pp1800_wf_cam { + status = "okay"; +}; + +&pp2800_uf_cam { + status = "okay"; +}; + +&pp2800_wf_cam { + status = "okay"; +}; + &pp3300_dx_edp { gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>; }; diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi index d4f4441179fc..1dd8e35093a8 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi @@ -113,6 +113,40 @@ src_vph_pwr: src-vph-pwr-regulator { vin-supply = <&ppvar_sys>; }; + pp1800_uf_cam: pp1800-uf-cam-regulator { + compatible = "regulator-fixed"; + regulator-name = "pp1800_uf_cam"; + status = "disabled"; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + gpio = <&tlmm 6 GPIO_ACTIVE_HIGH>; + enable-active-high; + pinctrl-names = "default"; + pinctrl-0 = <&uf_cam_en>; + + vin-supply = <&pp1800_ldo>; + regulator-enable-ramp-delay = <1000>; + }; + + pp1800_wf_cam: pp1800-wf-cam-regulator { + compatible = "regulator-fixed"; + regulator-name = "pp1800_wf_cam"; + status = "disabled"; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + gpio = <&tlmm 7 GPIO_ACTIVE_HIGH>; + enable-active-high; + pinctrl-names = "default"; + pinctrl-0 = <&wf_cam_en>; + + vin-supply = <&pp1800_ldo>; + regulator-enable-ramp-delay = <1000>; + }; + pp5000_a: pp5000-a-regulator { compatible = "regulator-fixed"; regulator-name = "pp5000_a"; @@ -144,6 +178,66 @@ pp3300_a: pp3300-a-regulator { vin-supply = <&ppvar_sys>; }; + pp1800_ec: + pp1800_sensors: + pp1800_ldo: pp1800-ldo-regulator { + compatible = "regulator-fixed"; + regulator-name = "pp1800_ldo"; + + /* EC turns on with hibernate_l; always on for AP */ + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + + /* + * Actually should be pp1800_h1 but we don't have any need to + * model that so we use the parent of pp1800_h1. + */ + vin-supply = <&pp3300_a>; + }; + + pp2800_uf_cam: pp2800-uf-cam-regulator { + compatible = "regulator-fixed"; + regulator-name = "pp2800_uf_cam"; + status = "disabled"; + + regulator-min-microvolt = <2850000>; + regulator-max-microvolt = <2850000>; + + gpio = <&tlmm 6 GPIO_ACTIVE_HIGH>; + enable-active-high; + /* + * The pinconf can only be referenced once so we put it on the + * first regulator and comment it out here. + * pinctrl-names = "default"; + * pinctrl-0 = <&uf_cam_en>; + */ + + vin-supply = <&pp3300_a>; + }; + + pp2800_vcm_wf_cam: + pp2800_wf_cam: pp2800-wf-cam-regulator { + compatible = "regulator-fixed"; + regulator-name = "pp2800_wf_cam"; + status = "disabled"; + + regulator-min-microvolt = <2850000>; + regulator-max-microvolt = <2850000>; + + gpio = <&tlmm 7 GPIO_ACTIVE_HIGH>; + enable-active-high; + /* + * The pinconf can only be referenced once so we put it on the + * first regulator and comment it out here. + * pinctrl-names = "default"; + * pinctrl-0 = <&wf_cam_en>; + */ + + vin-supply = <&pp3300_a>; + }; + pp3300_audio: pp3300_codec: pp3300-codec-regulator { compatible = "regulator-fixed"; @@ -1517,4 +1611,32 @@ pinconf-sd-cd { drive-strength = <2>; }; }; + + uf_cam_en: uf-cam-en { + pinmux { + pins = "gpio6"; + function = "gpio"; + }; + + pinconf { + pins = "gpio6"; + drive-strength = <2>; + /* External pull down */ + bias-disable; + }; + }; + + wf_cam_en: wf-cam-en { + pinmux { + pins = "gpio7"; + function = "gpio"; + }; + + pinconf { + pins = "gpio7"; + drive-strength = <2>; + /* External pull down */ + bias-disable; + }; + }; };
Some trogdor boards have on-board regulators for the MIPI camera components. Add nodes describing these regulators so boards with these supplies can consume them. Cc: Douglas Anderson <dianders@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- .../boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 16 +++ .../dts/qcom/sc7180-trogdor-homestar.dtsi | 16 +++ arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 122 ++++++++++++++++++ 3 files changed, 154 insertions(+) base-commit: 136057256686de39cc3a07c2e39ef6bc43003ff6