Re: [PATCH v2 4/4] arm64: dts: qcom: Add IMDT QCS8550 SBC

[William Bright]

On Wed, May 06, 2026 at 09:53:01AM +0200, Krzysztof Kozlowski wrote:
> On Tue, May 05, 2026 at 09:09:54PM +0100, William Bright wrote:
> > + compatible = "regulator-fixed";
> > + regulator-name = "cam_3v3_reg";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + vin-supply = <&hr_cam_pwr>;
> > + };
> > +
> > + display_panel_pwr_en: regulator-display-panel-en {
>
> Again, unused
>

For the camera regulators, I intended these to be used within device tree
overlays (I.E a DTSO would be made for an AR1335 connected to CSI0).
I don't have any camera device tree overlays in this patch series
as the only camera I have tested is the OnSemi AR1335 which isn't supported
upstream. So I will drop these camera regulator nodes.

> > + compatible = "regulator-fixed";
> > + regulator-name = "dsi_5v_en";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&dsi_5v_en_default>;
> > +
> > + gpio = <&tlmm 140 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > +
> > + vin-supply = <&som_vph_pwr>;
> > +
> > + regulator-always-on;
> > + regulator-boot-on;
> > + };
> > +
> > + /* Enables 1V2, 1V8_CAM and 3V3_CAM */
> > + hr_cam_pwr: regulator-hr-cam-pwr {
>
> And this becames unused after dropping fake regulators. Why don't you
> have proper users of these controllable supplies?
>

I will drop the on-board regulators for panels. These were intended to
be used by a panel DTSO. So I will these back in when I 
submit patches for a panel DTSO for this board.

> > + per_1v8_reg: regulator-per-1v8 {
>
> Drop node
>

I have omitted that its used by the LAN7430. I will add this
usage in V3.

> > + compatible = "regulator-fixed";
> > + regulator-name = "per_1v8_reg";
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <1800000>;
> > + vin-supply = <&per_pwr>;
> > + };
> > +
> > + per_3v3_reg: regulator-per-3v3 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "per_3v3_reg";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + vin-supply = <&per_pwr>;
> > + };
> > +
> > + per_5v_reg: regulator-per-5v {
> > + compatible = "regulator-fixed";
> > + regulator-name = "per_5v_reg";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + vin-supply = <&per_pwr>;
> > + };
>
> Drop all these
>

per_3v3_reg: Used by the LAN7430 but omitted by accident so I will define 
  that it's used by the LAN7430 in V3.
per_5v_reg: Used by onboard audio but audio support is omitted within this 
  patch series so I will drop this regulator.

>
> Probably most of them are to be dropped :/
>

For V3, I will go through all of these and eliminate any redundant regulators
or add where they're used.
Most of them likely became unused when removing SDHC4 whilst
preparing this patch series.

> > +&lpass_rxmacro {
> > + status = "disabled";
> > +};
> > +
> > +&lpass_tlmm {
> > + status = "disabled";
> > +};
> > +
> > +&lpass_txmacro {
> > + status = "disabled";
> > +};
> > +
> > +&lpass_vamacro {
> > + status = "disabled";
> > +};
> > +
> > +&lpass_wsa2macro {
> > + status = "disabled";
> > +};
> > +
> > +&lpass_wsamacro {
> > + status = "disabled";
> > +};
>
> Why are all these LPASS codecs disabled?
>
> Best regards,
> Krzysztof
>

No audio driver support is included in this series, but disabling the LPASS
codec nodes is unnecessary so I'll drop these changes in V3.
Many thanks for your feedback.

Best regards,
William Bright