[PATCH v2 6/6] arm64: dts: qcom: Add USB controller and phy nodes for IPQ5424

[Varadarajan Narayanan]

On Wed, Nov 13, 2024 at 01:15:15PM +0100, Johan Hovold wrote:
> On Wed, Nov 13, 2024 at 12:53:16PM +0530, Varadarajan Narayanan wrote:
>
> > --- a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
> > +++ b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
> > @@ -16,12 +16,71 @@ / {
> >  	aliases {
> >  		serial0 = &uart1;
> >  	};
> > +
> > +	regulator_fixed_3p3: s3300 {
>
> Fixed regulator nodes should have a "regulator-" prefix in their name.
>
> And please use a shorter label, look at the existing DTs for
> inspiration (e.g. "vreg_misc_3p3").

Ok.

> > +		compatible = "regulator-fixed";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-boot-on;
> > +		regulator-always-on;
> > +		regulator-name = "fixed_3p3";
> > +	};
>
> But is this really an accurate description of these regulators? Are
> they not part of some PMIC? Can they really not be turned off?

These supplies are coming from internal LDOs and are turned ON at
power up itself. Software (i.e. boot loaders or the kernel)
doesn't turn them ON. Checked with the board designers and they
too don't recommend turning these OFF as some other i/o lines
also depend on these voltages.

> Also please use the names from the schematics for these. I doubt they
> are named "fixed_3p3".

Ok.

> > +
> > +	regulator_fixed_1p8: s1800 {
> > +		compatible = "regulator-fixed";
> > +		regulator-min-microvolt = <1800000>;
> > +		regulator-max-microvolt = <1800000>;
> > +		regulator-boot-on;
> > +		regulator-always-on;
> > +		regulator-name = "fixed_1p8";
> > +	};
> > +
> > +	regulator_fixed_0p925: s0925 {
> > +		compatible = "regulator-fixed";
> > +		regulator-min-microvolt = <925000>;
> > +		regulator-max-microvolt = <925000>;
> > +		regulator-boot-on;
> > +		regulator-always-on;
> > +		regulator-name = "fixed_0p925";
> > +	};
> > +
>
> Stray newline.

Have addressed these and posted v3. Please review.

Thanks
Varada