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

[Johan Hovold]

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").


> +		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?

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

> +
> +	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.

> +};

Johan