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

Peter Geis
2024-12-10T13:01:31Z

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 {
> >
>
>
>
>