[PATCH v2 2/2] arm64: dts: rockchip: Enable regulators for Radxa E20C

[Jonas Karlman]

Hi Chukun,

On 2025-03-18 13:00, Chukun Pan wrote:
> Enable pwm and fixed regulators for Radxa E20C. The pwm regulator is
> used to power the CPU and GPU. Note that the LPDDR4 voltage is 1.1V.
> 
> Signed-off-by: Chukun Pan <amadeus at jmu.edu.cn>
> ---
>  .../boot/dts/rockchip/rk3528-radxa-e20c.dts   | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> index 57a446b5cbd6..159b55aa970d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> @@ -100,6 +100,16 @@ vcc_3v3: regulator-3v3-vcc {
>  		vin-supply = <&vcc5v0_sys>;
>  	};
>  
> +	vcc_ddr: regulator-vcc-ddr {

Since we know the voltage for this regulator please name the node
regulator-1v1-vcc-ddr and place it above the regulator-1v8 to keep node
order by node name (not label name).

> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_ddr";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1100000>;
> +		regulator-max-microvolt = <1100000>;
> +		vin-supply = <&vcc5v0_sys>;
> +	};
> +
>  	vcc5v0_sys: regulator-5v0-vcc-sys {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vcc5v0_sys";
> @@ -108,6 +118,56 @@ vcc5v0_sys: regulator-5v0-vcc-sys {
>  		regulator-min-microvolt = <5000000>;
>  		regulator-max-microvolt = <5000000>;
>  	};
> +
> +	vdd_0v9: regulator-0v9-vdd {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vdd_0v9";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <900000>;
> +		regulator-max-microvolt = <900000>;
> +		vin-supply = <&vcc5v0_sys>;
> +	};

This node is out of node name order, please place it before the
regulator-1v1-vcc-ddr node.

> +
> +	vdd_arm: regulator-vdd-arm {
> +		compatible = "pwm-regulator";

Suggest we add the pinctrl props here instead:

 		pinctrl-names = "default";
 		pinctrl-0 = <&pwm1m0_pins>;

> +		pwms = <&pwm1 0 5000 1>;

This should use the PWM_POLARITY_INVERTED flag not 1.

Will require include of dt-bindings/pwm/pwm.h.

> +		pwm-supply = <&vcc5v0_sys>;
> +		regulator-name = "vdd_arm";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <746000>;
> +		regulator-max-microvolt = <1201000>;
> +		regulator-settling-time-up-us = <250>;
> +	};
> +
> +	vdd_logic: regulator-vdd-logic {
> +		compatible = "pwm-regulator";

Same here, suggest we add the pinctrl props here instead:

 		pinctrl-names = "default";
 		pinctrl-0 = <&pwm2m0_pins>;

> +		pwms = <&pwm2 0 5000 1>;

Same here, this should use the PWM_POLARITY_INVERTED flag not 1.

> +		pwm-supply = <&vcc5v0_sys>;
> +		regulator-name = "vdd_logic";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <705000>;
> +		regulator-max-microvolt = <1006000>;
> +		regulator-settling-time-up-us = <250>;
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu1 {
> +	cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu2 {
> +	cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu3 {
> +	cpu-supply = <&vdd_arm>;
>  };
>  
>  &pinctrl {
> @@ -132,6 +192,18 @@ wan_led_g: wan-led-g {
>  	};
>  };
>  
> +&pwm1 {
> +	pinctrl-0 = <&pwm1m0_pins>;
> +	pinctrl-names = "default";

For consistency please put the pinctrl-names before the pinctrl-X props.
And as stated above, I suggest we move this to the regulator node.

> +	status = "okay";
> +};
> +
> +&pwm2 {
> +	pinctrl-0 = <&pwm2m0_pins>;
> +	pinctrl-names = "default";

Same here.

Regards,
Jonas

> +	status = "okay";
> +};
> +
>  &saradc {
>  	vref-supply = <&vcc_1v8>;
>  	status = "okay";