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

Diederik de Haas
2024-12-10T11:31:13Z

[-- Attachment #1: Type: text/plain, Size: 3742 bytes --]

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 {

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]