diff mbox series

[5/6] arm64: dts: rockchip: correct rk3328-roc regulator map

Message ID 20241210013010.81257-6-pgwipeout@gmail.com (mailing list archive)
State New
Headers show
Series rockchip: rk3328 fixes in preparation for usb3-phy | expand

Commit Message

Peter Geis Dec. 10, 2024, 1:30 a.m. UTC
The rk3328-roc-cc input power is sourced from a micro-usb port, while
the rk3328-roc-pc input power is sourced from a usb-c port. Both inputs
are 5vdc only. Remove the 12v input from the device tree.

While we are at it, add missing voltages and supply to vcc_phy, missing
voltages to vcc_host1_5v, and standardize the order of regulator
properties among the fixed regulators.

Fixes: 2171f4fdac06 ("arm64: dts: rockchip: add roc-rk3328-cc board")
Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---

 arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 23 +++++++++++++-------
 1 file changed, 15 insertions(+), 8 deletions(-)

Comments

Heiko Stübner Dec. 10, 2024, 10:54 a.m. UTC | #1
Am Dienstag, 10. Dezember 2024, 02:30:09 CET schrieb Peter Geis:
> The rk3328-roc-cc input power is sourced from a micro-usb port, while
> the rk3328-roc-pc input power is sourced from a usb-c port. Both inputs
> are 5vdc only. Remove the 12v input from the device tree.

full stop. Please don't do "While we are at it" commits.

> While we are at it, add missing voltages and supply to vcc_phy, missing
> voltages to vcc_host1_5v, and standardize the order of regulator
> properties among the fixed regulators.

This second part wants to be its own commit :-) .

Thanks
Heiko

> Fixes: 2171f4fdac06 ("arm64: dts: rockchip: add roc-rk3328-cc board")
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
> 
>  arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 23 +++++++++++++-------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> index f782c8220dd3..6984387ff8b3 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> @@ -24,22 +24,23 @@ gmac_clkin: external-gmac-clock {
>  		#clock-cells = <0>;
>  	};
>  
> -	dc_12v: regulator-dc-12v {
> +	/* fed from passive usb input connector */
> +	dc_5v: regulator-dc-5v {
>  		compatible = "regulator-fixed";
> -		regulator-name = "dc_12v";
> +		regulator-name = "dc_5v";
>  		regulator-always-on;
>  		regulator-boot-on;
> -		regulator-min-microvolt = <12000000>;
> -		regulator-max-microvolt = <12000000>;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
>  	};
>  
>  	vcc_sd: regulator-sdmmc {
>  		compatible = "regulator-fixed";
> +		regulator-name = "vcc_sd";
>  		gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&sdmmc0m1_pin>;
>  		regulator-boot-on;
> -		regulator-name = "vcc_sd";
>  		regulator-min-microvolt = <3300000>;
>  		regulator-max-microvolt = <3300000>;
>  		vin-supply = <&vcc_io>;
> @@ -50,22 +51,25 @@ vcc_sdio: regulator-sdmmcio {
>  		states = <1800000 0x1>, <3300000 0x0>;
>  		regulator-name = "vcc_sdio";
>  		regulator-type = "voltage";
> +		regulator-always-on;
>  		regulator-min-microvolt = <1800000>;
>  		regulator-max-microvolt = <3300000>;
> -		regulator-always-on;
>  		vin-supply = <&vcc_sys>;
>  	};
>  
>  	vcc_host1_5v: vcc_otg_5v: regulator-vcc-host1-5v {
>  		compatible = "regulator-fixed";
> +		regulator-name = "vcc_host1_5v";
>  		enable-active-high;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&usb20_host_drv>;
> -		regulator-name = "vcc_host1_5v";
>  		regulator-always-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
>  		vin-supply = <&vcc_sys>;
>  	};
>  
> +	/* sourced from usb input through 3A fuse */
>  	vcc_sys: regulator-vcc-sys {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vcc_sys";
> @@ -73,7 +77,7 @@ vcc_sys: regulator-vcc-sys {
>  		regulator-boot-on;
>  		regulator-min-microvolt = <5000000>;
>  		regulator-max-microvolt = <5000000>;
> -		vin-supply = <&dc_12v>;
> +		vin-supply = <&dc_5v>;
>  	};
>  
>  	vcc_phy: regulator-vcc-phy {
> @@ -81,6 +85,9 @@ vcc_phy: regulator-vcc-phy {
>  		regulator-name = "vcc_phy";
>  		regulator-always-on;
>  		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&vcc_io>;
>  	};
>  
>  	leds {
>
Diederik de Haas Dec. 10, 2024, 11:31 a.m. UTC | #2
Hi Peter,

Thanks for this series, I already saw some familiar error msgs
mentioned, so will try this series out soon (tm).

On Tue Dec 10, 2024 at 2:30 AM CET, Peter Geis wrote:
> The rk3328-roc-cc input power is sourced from a micro-usb port, while
> the rk3328-roc-pc input power is sourced from a usb-c port. Both inputs
> are 5vdc only. Remove the 12v input from the device tree.
>
> While we are at it, add missing voltages and supply to vcc_phy, missing
> voltages to vcc_host1_5v, and standardize the order of regulator
> properties among the fixed regulators.

Big fan of standardization :-) ...

>
> Fixes: 2171f4fdac06 ("arm64: dts: rockchip: add roc-rk3328-cc board")
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>
>  arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 23 +++++++++++++-------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> index f782c8220dd3..6984387ff8b3 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> @@ -24,22 +24,23 @@ gmac_clkin: external-gmac-clock {
>  		#clock-cells = <0>;
>  	};
>  
> -	dc_12v: regulator-dc-12v {
> +	/* fed from passive usb input connector */
> +	dc_5v: regulator-dc-5v {
>  		compatible = "regulator-fixed";
> -		regulator-name = "dc_12v";
> +		regulator-name = "dc_5v";
>  		regulator-always-on;
>  		regulator-boot-on;
> -		regulator-min-microvolt = <12000000>;
> -		regulator-max-microvolt = <12000000>;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
>  	};
>  
>  	vcc_sd: regulator-sdmmc {
>  		compatible = "regulator-fixed";
> +		regulator-name = "vcc_sd";
>  		gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&sdmmc0m1_pin>;
>  		regulator-boot-on;
> -		regulator-name = "vcc_sd";
>  		regulator-min-microvolt = <3300000>;
>  		regulator-max-microvolt = <3300000>;
>  		vin-supply = <&vcc_io>;

... but why not put regulator-name as the first of the regulator
properties as is done in the rk3328-rock64.dts ...

> @@ -50,22 +51,25 @@ vcc_sdio: regulator-sdmmcio {
>  		states = <1800000 0x1>, <3300000 0x0>;
>  		regulator-name = "vcc_sdio";
>  		regulator-type = "voltage";
> +		regulator-always-on;
>  		regulator-min-microvolt = <1800000>;
>  		regulator-max-microvolt = <3300000>;
> -		regulator-always-on;
>  		vin-supply = <&vcc_sys>;
>  	};
>  
>  	vcc_host1_5v: vcc_otg_5v: regulator-vcc-host1-5v {
>  		compatible = "regulator-fixed";
> +		regulator-name = "vcc_host1_5v";
>  		enable-active-high;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&usb20_host_drv>;
> -		regulator-name = "vcc_host1_5v";
>  		regulator-always-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
>  		vin-supply = <&vcc_sys>;
>  	};

... and was the case here?

Cheers,
  Diederik

>  
> +	/* sourced from usb input through 3A fuse */
>  	vcc_sys: regulator-vcc-sys {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vcc_sys";
> @@ -73,7 +77,7 @@ vcc_sys: regulator-vcc-sys {
>  		regulator-boot-on;
>  		regulator-min-microvolt = <5000000>;
>  		regulator-max-microvolt = <5000000>;
> -		vin-supply = <&dc_12v>;
> +		vin-supply = <&dc_5v>;
>  	};
>  
>  	vcc_phy: regulator-vcc-phy {
> @@ -81,6 +85,9 @@ vcc_phy: regulator-vcc-phy {
>  		regulator-name = "vcc_phy";
>  		regulator-always-on;
>  		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&vcc_io>;
>  	};
>  
>  	leds {
Peter Geis Dec. 10, 2024, 1:01 p.m. UTC | #3
On Tue, Dec 10, 2024 at 5:54 AM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Dienstag, 10. Dezember 2024, 02:30:09 CET schrieb Peter Geis:
> > The rk3328-roc-cc input power is sourced from a micro-usb port, while
> > the rk3328-roc-pc input power is sourced from a usb-c port. Both inputs
> > are 5vdc only. Remove the 12v input from the device tree.
>
> full stop. Please don't do "While we are at it" commits.
>
> > While we are at it, add missing voltages and supply to vcc_phy, missing
> > voltages to vcc_host1_5v, and standardize the order of regulator
> > properties among the fixed regulators.
>
> This second part wants to be its own commit :-) .

Thank you, you're right I was torn between doing not enough and doing
too much and ended up doing both. Thinking about it now this should be
at least three patches:
- Power input
- Drop the phy regulator (it's directly tied to vcc_io, not a separate device)
- Cosmetic changes.

Thanks again!
Peter

>
> Thanks
> Heiko
>
> > Fixes: 2171f4fdac06 ("arm64: dts: rockchip: add roc-rk3328-cc board")
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >
> >  arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 23 +++++++++++++-------
> >  1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > index f782c8220dd3..6984387ff8b3 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > @@ -24,22 +24,23 @@ gmac_clkin: external-gmac-clock {
> >               #clock-cells = <0>;
> >       };
> >
> > -     dc_12v: regulator-dc-12v {
> > +     /* fed from passive usb input connector */
> > +     dc_5v: regulator-dc-5v {
> >               compatible = "regulator-fixed";
> > -             regulator-name = "dc_12v";
> > +             regulator-name = "dc_5v";
> >               regulator-always-on;
> >               regulator-boot-on;
> > -             regulator-min-microvolt = <12000000>;
> > -             regulator-max-microvolt = <12000000>;
> > +             regulator-min-microvolt = <5000000>;
> > +             regulator-max-microvolt = <5000000>;
> >       };
> >
> >       vcc_sd: regulator-sdmmc {
> >               compatible = "regulator-fixed";
> > +             regulator-name = "vcc_sd";
> >               gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
> >               pinctrl-names = "default";
> >               pinctrl-0 = <&sdmmc0m1_pin>;
> >               regulator-boot-on;
> > -             regulator-name = "vcc_sd";
> >               regulator-min-microvolt = <3300000>;
> >               regulator-max-microvolt = <3300000>;
> >               vin-supply = <&vcc_io>;
> > @@ -50,22 +51,25 @@ vcc_sdio: regulator-sdmmcio {
> >               states = <1800000 0x1>, <3300000 0x0>;
> >               regulator-name = "vcc_sdio";
> >               regulator-type = "voltage";
> > +             regulator-always-on;
> >               regulator-min-microvolt = <1800000>;
> >               regulator-max-microvolt = <3300000>;
> > -             regulator-always-on;
> >               vin-supply = <&vcc_sys>;
> >       };
> >
> >       vcc_host1_5v: vcc_otg_5v: regulator-vcc-host1-5v {
> >               compatible = "regulator-fixed";
> > +             regulator-name = "vcc_host1_5v";
> >               enable-active-high;
> >               pinctrl-names = "default";
> >               pinctrl-0 = <&usb20_host_drv>;
> > -             regulator-name = "vcc_host1_5v";
> >               regulator-always-on;
> > +             regulator-min-microvolt = <5000000>;
> > +             regulator-max-microvolt = <5000000>;
> >               vin-supply = <&vcc_sys>;
> >       };
> >
> > +     /* sourced from usb input through 3A fuse */
> >       vcc_sys: regulator-vcc-sys {
> >               compatible = "regulator-fixed";
> >               regulator-name = "vcc_sys";
> > @@ -73,7 +77,7 @@ vcc_sys: regulator-vcc-sys {
> >               regulator-boot-on;
> >               regulator-min-microvolt = <5000000>;
> >               regulator-max-microvolt = <5000000>;
> > -             vin-supply = <&dc_12v>;
> > +             vin-supply = <&dc_5v>;
> >       };
> >
> >       vcc_phy: regulator-vcc-phy {
> > @@ -81,6 +85,9 @@ vcc_phy: regulator-vcc-phy {
> >               regulator-name = "vcc_phy";
> >               regulator-always-on;
> >               regulator-boot-on;
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             vin-supply = <&vcc_io>;
> >       };
> >
> >       leds {
> >
>
>
>
>
Peter Geis Dec. 10, 2024, 1:04 p.m. UTC | #4
On Tue, Dec 10, 2024 at 6:31 AM Diederik de Haas <didi.debian@cknow.org> wrote:
>
> Hi Peter,
>
> Thanks for this series, I already saw some familiar error msgs
> mentioned, so will try this series out soon (tm).
>
> On Tue Dec 10, 2024 at 2:30 AM CET, Peter Geis wrote:
> > The rk3328-roc-cc input power is sourced from a micro-usb port, while
> > the rk3328-roc-pc input power is sourced from a usb-c port. Both inputs
> > are 5vdc only. Remove the 12v input from the device tree.
> >
> > While we are at it, add missing voltages and supply to vcc_phy, missing
> > voltages to vcc_host1_5v, and standardize the order of regulator
> > properties among the fixed regulators.
>
> Big fan of standardization :-) ...
>
> >
> > Fixes: 2171f4fdac06 ("arm64: dts: rockchip: add roc-rk3328-cc board")
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >
> >  arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 23 +++++++++++++-------
> >  1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > index f782c8220dd3..6984387ff8b3 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > @@ -24,22 +24,23 @@ gmac_clkin: external-gmac-clock {
> >               #clock-cells = <0>;
> >       };
> >
> > -     dc_12v: regulator-dc-12v {
> > +     /* fed from passive usb input connector */
> > +     dc_5v: regulator-dc-5v {
> >               compatible = "regulator-fixed";
> > -             regulator-name = "dc_12v";
> > +             regulator-name = "dc_5v";
> >               regulator-always-on;
> >               regulator-boot-on;
> > -             regulator-min-microvolt = <12000000>;
> > -             regulator-max-microvolt = <12000000>;
> > +             regulator-min-microvolt = <5000000>;
> > +             regulator-max-microvolt = <5000000>;
> >       };
> >
> >       vcc_sd: regulator-sdmmc {
> >               compatible = "regulator-fixed";
> > +             regulator-name = "vcc_sd";
> >               gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
> >               pinctrl-names = "default";
> >               pinctrl-0 = <&sdmmc0m1_pin>;
> >               regulator-boot-on;
> > -             regulator-name = "vcc_sd";
> >               regulator-min-microvolt = <3300000>;
> >               regulator-max-microvolt = <3300000>;
> >               vin-supply = <&vcc_io>;
>
> ... but why not put regulator-name as the first of the regulator
> properties as is done in the rk3328-rock64.dts ...
>
> > @@ -50,22 +51,25 @@ vcc_sdio: regulator-sdmmcio {
> >               states = <1800000 0x1>, <3300000 0x0>;
> >               regulator-name = "vcc_sdio";
> >               regulator-type = "voltage";
> > +             regulator-always-on;
> >               regulator-min-microvolt = <1800000>;
> >               regulator-max-microvolt = <3300000>;
> > -             regulator-always-on;
> >               vin-supply = <&vcc_sys>;
> >       };
> >
> >       vcc_host1_5v: vcc_otg_5v: regulator-vcc-host1-5v {
> >               compatible = "regulator-fixed";
> > +             regulator-name = "vcc_host1_5v";
> >               enable-active-high;
> >               pinctrl-names = "default";
> >               pinctrl-0 = <&usb20_host_drv>;
> > -             regulator-name = "vcc_host1_5v";
> >               regulator-always-on;
> > +             regulator-min-microvolt = <5000000>;
> > +             regulator-max-microvolt = <5000000>;
> >               vin-supply = <&vcc_sys>;
> >       };
>
> ... and was the case here?

That's fair, thank you. I like the alphabetical approach, I'll go that
way when I split this out.

>
> Cheers,
>   Diederik
>
> >
> > +     /* sourced from usb input through 3A fuse */
> >       vcc_sys: regulator-vcc-sys {
> >               compatible = "regulator-fixed";
> >               regulator-name = "vcc_sys";
> > @@ -73,7 +77,7 @@ vcc_sys: regulator-vcc-sys {
> >               regulator-boot-on;
> >               regulator-min-microvolt = <5000000>;
> >               regulator-max-microvolt = <5000000>;
> > -             vin-supply = <&dc_12v>;
> > +             vin-supply = <&dc_5v>;
> >       };
> >
> >       vcc_phy: regulator-vcc-phy {
> > @@ -81,6 +85,9 @@ vcc_phy: regulator-vcc-phy {
> >               regulator-name = "vcc_phy";
> >               regulator-always-on;
> >               regulator-boot-on;
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             vin-supply = <&vcc_io>;
> >       };
> >
> >       leds {
>
Diederik de Haas Dec. 10, 2024, 2:08 p.m. UTC | #5
On Tue Dec 10, 2024 at 2:04 PM CET, Peter Geis wrote:
> On Tue, Dec 10, 2024 at 6:31 AM Diederik de Haas <didi.debian@cknow.org> wrote:
> > On Tue Dec 10, 2024 at 2:30 AM CET, Peter Geis wrote:
> > > voltages to vcc_host1_5v, and standardize the order of regulator
> > > properties among the fixed regulators.
> >
> > Big fan of standardization :-) ...
> >
> > >
> > > Fixes: 2171f4fdac06 ("arm64: dts: rockchip: add roc-rk3328-cc board")
> > > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > > ---
> > >
> > >  arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 23 +++++++++++++-------
> > >  1 file changed, 15 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > > index f782c8220dd3..6984387ff8b3 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
> > > @@ -24,22 +24,23 @@ gmac_clkin: external-gmac-clock {
> > >               #clock-cells = <0>;
> > >       };
> > >
> > > -     dc_12v: regulator-dc-12v {
> > > +     /* fed from passive usb input connector */
> > > +     dc_5v: regulator-dc-5v {
> > >               compatible = "regulator-fixed";
> > > -             regulator-name = "dc_12v";
> > > +             regulator-name = "dc_5v";
> > >               regulator-always-on;
> > >               regulator-boot-on;
> > > -             regulator-min-microvolt = <12000000>;
> > > -             regulator-max-microvolt = <12000000>;
> > > +             regulator-min-microvolt = <5000000>;
> > > +             regulator-max-microvolt = <5000000>;
> > >       };
> > >
> > >       vcc_sd: regulator-sdmmc {
> > >               compatible = "regulator-fixed";
> > > +             regulator-name = "vcc_sd";
> > >               gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
> > >               pinctrl-names = "default";
> > >               pinctrl-0 = <&sdmmc0m1_pin>;
> > >               regulator-boot-on;
> > > -             regulator-name = "vcc_sd";
> > >               regulator-min-microvolt = <3300000>;
> > >               regulator-max-microvolt = <3300000>;
> > >               vin-supply = <&vcc_io>;
> >
> > ... but why not put regulator-name as the first of the regulator
> > properties as is done in the rk3328-rock64.dts ...
> >
> > > @@ -50,22 +51,25 @@ vcc_sdio: regulator-sdmmcio {
> > >               states = <1800000 0x1>, <3300000 0x0>;
> > >               regulator-name = "vcc_sdio";
> > >               regulator-type = "voltage";
> > > +             regulator-always-on;
> > >               regulator-min-microvolt = <1800000>;
> > >               regulator-max-microvolt = <3300000>;
> > > -             regulator-always-on;
> > >               vin-supply = <&vcc_sys>;
> > >       };
> > >
> > >       vcc_host1_5v: vcc_otg_5v: regulator-vcc-host1-5v {
> > >               compatible = "regulator-fixed";
> > > +             regulator-name = "vcc_host1_5v";
> > >               enable-active-high;
> > >               pinctrl-names = "default";
> > >               pinctrl-0 = <&usb20_host_drv>;
> > > -             regulator-name = "vcc_host1_5v";
> > >               regulator-always-on;
> > > +             regulator-min-microvolt = <5000000>;
> > > +             regulator-max-microvolt = <5000000>;
> > >               vin-supply = <&vcc_sys>;
> > >       };
> >
> > ... and was the case here?
>
> That's fair, thank you. I like the alphabetical approach, I'll go that
> way when I split this out.

FWIW, I'm fine when regulator-name and regulator-type would be put on
the top of the regulator-* properties, also because that's currently
the case for the rk3328-rock64.dts.
And a strict alphabetical order would look weird anyway as then
-max-microvolt should be ordered above -min-microvolt.

Cheers,
  Diederik
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
index f782c8220dd3..6984387ff8b3 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi
@@ -24,22 +24,23 @@  gmac_clkin: external-gmac-clock {
 		#clock-cells = <0>;
 	};
 
-	dc_12v: regulator-dc-12v {
+	/* fed from passive usb input connector */
+	dc_5v: regulator-dc-5v {
 		compatible = "regulator-fixed";
-		regulator-name = "dc_12v";
+		regulator-name = "dc_5v";
 		regulator-always-on;
 		regulator-boot-on;
-		regulator-min-microvolt = <12000000>;
-		regulator-max-microvolt = <12000000>;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
 	};
 
 	vcc_sd: regulator-sdmmc {
 		compatible = "regulator-fixed";
+		regulator-name = "vcc_sd";
 		gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdmmc0m1_pin>;
 		regulator-boot-on;
-		regulator-name = "vcc_sd";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		vin-supply = <&vcc_io>;
@@ -50,22 +51,25 @@  vcc_sdio: regulator-sdmmcio {
 		states = <1800000 0x1>, <3300000 0x0>;
 		regulator-name = "vcc_sdio";
 		regulator-type = "voltage";
+		regulator-always-on;
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <3300000>;
-		regulator-always-on;
 		vin-supply = <&vcc_sys>;
 	};
 
 	vcc_host1_5v: vcc_otg_5v: regulator-vcc-host1-5v {
 		compatible = "regulator-fixed";
+		regulator-name = "vcc_host1_5v";
 		enable-active-high;
 		pinctrl-names = "default";
 		pinctrl-0 = <&usb20_host_drv>;
-		regulator-name = "vcc_host1_5v";
 		regulator-always-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
 		vin-supply = <&vcc_sys>;
 	};
 
+	/* sourced from usb input through 3A fuse */
 	vcc_sys: regulator-vcc-sys {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc_sys";
@@ -73,7 +77,7 @@  vcc_sys: regulator-vcc-sys {
 		regulator-boot-on;
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
-		vin-supply = <&dc_12v>;
+		vin-supply = <&dc_5v>;
 	};
 
 	vcc_phy: regulator-vcc-phy {
@@ -81,6 +85,9 @@  vcc_phy: regulator-vcc-phy {
 		regulator-name = "vcc_phy";
 		regulator-always-on;
 		regulator-boot-on;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc_io>;
 	};
 
 	leds {