Re: [PATCH RESEND] ARM: dts: sun8i: r40: open the regulator aldo1

[Andre Przywara <andre.przywara@xxxxxxx>]

On Wed, 20 Dec 2023 16:18:43 +0800
fuyao <fuyao1697@xxxxxxx> wrote:

Hi,

> the aldo1 is connect regulator pin which power the TV.

What do you mean with that? That ALDO1 is connected to VCC-TVOUT and/or
VCC-TVIN on the R40 SoC?

> The USB core use TV ref as reference Voltage.

The USB core in the SoC? So pin VCC-USB, which requires 3.3V, the same
voltage as the TV pins?
Which means this doesn't really have much to do with TV, it's just that
USB and also "TV" are supplied by ALDO1?

> Signed-off-by: fuyao <fuyao1697@xxxxxxx>
> ---
>  arch/arm/boot/dts/allwinner/sun8i-r40-feta40i.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/allwinner/sun8i-r40-feta40i.dtsi b/arch/arm/boot/dts/allwinner/sun8i-r40-feta40i.dtsi
> index 9f39b5a2bb35..8906170461df 100644
> --- a/arch/arm/boot/dts/allwinner/sun8i-r40-feta40i.dtsi
> +++ b/arch/arm/boot/dts/allwinner/sun8i-r40-feta40i.dtsi
> @@ -42,6 +42,13 @@ &pio {
>  	vcc-pg-supply = <&reg_dldo1>;
>  };
>  
> +&reg_aldo1 {
> +	regulator-always-on;

So did USB never work before, with the DT as in mainline?

For always-on regulators it would be good to see some rationale why this
cannot be referenced by its consumer. If it is really supplying the USB
core, that would be a reason, because we don't have a good way of
describing this.

> +	regulator-min-microvolt = <3300000>;
> +	regulator-max-microvolt = <3300000>;
> +	regulator-name = "vcc-aldo1";

Regulators should be named after their users, so use something like:
	regulator-name = "vcc-3v3-tv-usb";

That then also serves as documentation of why this is always on.

Cheers,
Andre

> +};
> +
>  &reg_aldo2 {
>  	regulator-always-on;
>  	regulator-min-microvolt = <1800000>;