linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Add Radxa Rock 4D support
@ 2025-02-17 16:34 Detlev Casanova
  2025-02-17 16:34 ` [PATCH v4 1/2] dt-bindings: arm: rockchip: Add Radxa ROCK 4D board Detlev Casanova
  2025-02-17 16:34 ` [PATCH v4 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree Detlev Casanova
  0 siblings, 2 replies; 6+ messages in thread
From: Detlev Casanova @ 2025-02-17 16:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Jonas Karlman, Chris Morgan, Kever Yang, Dragan Simic, Tim Lunn,
	FUKAUMI Naoki, Michael Riesch, Detlev Casanova, Stephen Chen,
	Elon Zhang, Alexey Charkov, devicetree, linux-arm-kernel,
	linux-rockchip, kernel

Add the basic support for the board. (Not officially released yet)
It is based on the Rockchip rk3576 SoC, so I haven't added the
following devices yet:
 - VOP/HDMI
 - UFS
as the support for those has not been merged yet, but are close to be
and I already validated that they work.
It will come with another patch set.

The following devices are supported and working:
 - UART
 - SD Card
 - Ethernet
 - USB
 - RTC

Changes since v3:
 - Reorder nodes alphabetically
 - Add missing cpu supply

Changes since v2:
 - Move and rename snps,reset-* props to the PHY node
 - Rename phy@1 to phy-ethernet@1

Changes since v1:
 - Add missing dt bindings
 - Remove clock-frequency in rtc node
 - Add line break in pmic pinctrls

Detlev Casanova (1):
  dt-bindings: arm: rockchip: Add Radxa ROCK 4D board

Stephen Chen (1):
  arm64: dts: rockchip: Add Radxa ROCK 4D device tree

 .../devicetree/bindings/arm/rockchip.yaml     |   5 +
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3576-rock-4d.dts      | 678 ++++++++++++++++++
 3 files changed, 684 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts

-- 
2.48.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 1/2] dt-bindings: arm: rockchip: Add Radxa ROCK 4D board
  2025-02-17 16:34 [PATCH v4 0/2] Add Radxa Rock 4D support Detlev Casanova
@ 2025-02-17 16:34 ` Detlev Casanova
  2025-02-17 16:34 ` [PATCH v4 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree Detlev Casanova
  1 sibling, 0 replies; 6+ messages in thread
From: Detlev Casanova @ 2025-02-17 16:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Jonas Karlman, Chris Morgan, Kever Yang, Dragan Simic, Tim Lunn,
	FUKAUMI Naoki, Michael Riesch, Detlev Casanova, Stephen Chen,
	Elon Zhang, Alexey Charkov, devicetree, linux-arm-kernel,
	linux-rockchip, kernel, Krzysztof Kozlowski

The board is based on the Rockchip rk3576 SoC.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index 522a6f0450eae..9ddb20890627f 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -862,6 +862,11 @@ properties:
           - const: radxa,rock-4c-plus
           - const: rockchip,rk3399
 
+      - description: Radxa ROCK 4D
+        items:
+          - const: radxa,rock-4d
+          - const: rockchip,rk3576
+
       - description: Radxa ROCK 4SE
         items:
           - const: radxa,rock-4se
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree
  2025-02-17 16:34 [PATCH v4 0/2] Add Radxa Rock 4D support Detlev Casanova
  2025-02-17 16:34 ` [PATCH v4 1/2] dt-bindings: arm: rockchip: Add Radxa ROCK 4D board Detlev Casanova
@ 2025-02-17 16:34 ` Detlev Casanova
  2025-02-17 17:08   ` Jonas Karlman
  1 sibling, 1 reply; 6+ messages in thread
From: Detlev Casanova @ 2025-02-17 16:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Jonas Karlman, Chris Morgan, Kever Yang, Dragan Simic, Tim Lunn,
	FUKAUMI Naoki, Michael Riesch, Detlev Casanova, Stephen Chen,
	Elon Zhang, Alexey Charkov, devicetree, linux-arm-kernel,
	linux-rockchip, kernel

From: Stephen Chen <stephen@radxa.com>

The Radxa ROCK 4D board is based on the Rockchip rk3576 SoC.

The device tree adds support for basic devices:
 - UART
 - SD Card
 - Ethernet
 - USB
 - RTC

It has 4 USB ports but only 3 are usable as the top left one is used
for maskrom.

It has a USB-C port that is only used for powering the board.

Signed-off-by: Stephen Chen <stephen@radxa.com>
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3576-rock-4d.dts      | 678 ++++++++++++++++++
 2 files changed, 679 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index def1222c1907e..a112aeb37948a 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -132,6 +132,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-armsom-sige5.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-evb1-v10.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-rock-4d.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3582-radxa-e52c.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-sige7.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
new file mode 100644
index 0000000000000..b499effd50396
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
@@ -0,0 +1,678 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2024 Radxa Computer (Shenzhen) Co., Ltd.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/pwm/pwm.h>
+#include <dt-bindings/soc/rockchip,vop2.h>
+#include <dt-bindings/usb/pd.h>
+#include "rk3576.dtsi"
+
+/ {
+	model = "Radxa ROCK 4D";
+	compatible = "radxa,rock-4d", "rockchip,rk3576";
+
+	aliases {
+		ethernet0 = &gmac0;
+	};
+
+	chosen {
+		stdout-path = "serial0:1500000n8";
+	};
+
+	leds: leds {
+		compatible = "gpio-leds";
+
+		power-led {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_STATUS;
+			gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "default-on";
+		};
+
+		user-led {
+			color = <LED_COLOR_ID_BLUE>;
+			function = LED_FUNCTION_HEARTBEAT;
+			gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "heartbeat";
+		};
+	};
+
+	vcc_12v0_dcin: regulator-vcc-12v0-dcin {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-name = "vcc_12v0_dcin";
+	};
+
+	vcc_1v1_nldo_s3: regulator-vcc-1v1-nldo-s3 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1100000>;
+		regulator-max-microvolt = <1100000>;
+		regulator-name = "vcc_1v1_nldo_s3";
+		vin-supply = <&vcc_5v0_sys>;
+	};
+
+	vcc_1v2_ufs_vccq_s0: regulator-vcc-1v2-ufs-vccq-s0 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+		regulator-name = "vcc_1v2_ufs_vccq_s0";
+		vin-supply = <&vcc_5v0_sys>;
+	};
+
+	vcc_1v8_s0: regulator-vcc-1v8-s0 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-name = "vcc_1v8_s0";
+		vin-supply = <&vcc_1v8_s3>;
+	};
+
+	vcc_1v8_ufs_vccq2_s0: regulator-vcc1v8-ufs-vccq2-s0 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-name = "vcc_1v8_ufs_vccq2_s0";
+		vin-supply = <&vcc_1v8_s3>;
+	};
+
+	vcc_2v0_pldo_s3: regulator-vcc-2v0-pldo-s3 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <2000000>;
+		regulator-max-microvolt = <2000000>;
+		regulator-name = "vcc_2v0_pldo_s3";
+		vin-supply = <&vcc_5v0_sys>;
+	};
+
+	vcc_3v3_pcie: regulator-vcc-3v3-pcie {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PD3 GPIO_ACTIVE_HIGH>;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-name = "vcc_3v3_pcie";
+		startup-delay-us = <5000>;
+		vin-supply = <&vcc_5v0_sys>;
+	};
+
+	vcc_3v3_rtc_s5: regulator-vcc-3v3-rtc-s5 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-name = "vcc_3v3_rtc_s5";
+		vin-supply = <&vcc_5v0_sys>;
+	};
+
+	vcc_3v3_s0: regulator-vcc-3v3-s0 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-name = "vcc_3v3_s0";
+		vin-supply = <&vcc_3v3_s3>;
+	};
+
+	vcc_3v3_ufs_s0: regulator-vcc-ufs-s0 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-name = "vcc_3v3_ufs_s0";
+		vin-supply = <&vcc_5v0_sys>;
+	};
+
+	vcc_5v0_device: regulator-vcc-5v0-device {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-name = "vcc_5v0_device";
+		vin-supply = <&vcc_12v0_dcin>;
+	};
+
+	vcc_5v0_host: regulator-vcc-5v0-host {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb_host_pwren>;
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-name = "vcc5v0_host";
+		vin-supply = <&vcc_5v0_device>;
+	};
+
+	vcc_5v0_sys: regulator-vcc-5v0-sys {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-name = "vcc_5v0_sys";
+		vin-supply = <&vcc_12v0_dcin>;
+	};
+};
+
+&combphy1_psu {
+	status = "okay";
+};
+
+&cpu_b0 {
+	cpu-supply = <&vdd_cpu_big_s0>;
+};
+
+&cpu_b1 {
+	cpu-supply = <&vdd_cpu_big_s0>;
+};
+
+&cpu_b2 {
+	cpu-supply = <&vdd_cpu_big_s0>;
+};
+
+&cpu_b3 {
+	cpu-supply = <&vdd_cpu_big_s0>;
+};
+
+&cpu_l0 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_l1 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_l2 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_l3 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&gmac0 {
+	phy-mode = "rgmii-id";
+	clock_in_out = "output";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&eth0m0_miim
+		     &eth0m0_tx_bus2
+		     &eth0m0_rx_bus2
+		     &eth0m0_rgmii_clk
+		     &eth0m0_rgmii_bus
+		     &ethm0_clk0_25m_out>;
+
+	phy-handle = <&rgmii_phy0>;
+	status = "okay";
+};
+
+&gpu {
+	mali-supply = <&vdd_gpu_s0>;
+	status = "okay";
+};
+
+&i2c1 {
+	status = "okay";
+
+	pmic@23 {
+		compatible = "rockchip,rk806";
+		reg = <0x23>;
+
+		gpio-controller;
+
+		interrupt-parent = <&gpio0>;
+		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_pins
+			     &rk806_dvs1_null
+			     &rk806_dvs2_null
+			     &rk806_dvs3_null>;
+
+		system-power-controller;
+
+		vcc1-supply = <&vcc_5v0_sys>;
+		vcc2-supply = <&vcc_5v0_sys>;
+		vcc3-supply = <&vcc_5v0_sys>;
+		vcc4-supply = <&vcc_5v0_sys>;
+		vcc5-supply = <&vcc_5v0_sys>;
+		vcc6-supply = <&vcc_5v0_sys>;
+		vcc7-supply = <&vcc_5v0_sys>;
+		vcc8-supply = <&vcc_5v0_sys>;
+		vcc9-supply = <&vcc_5v0_sys>;
+		vcc10-supply = <&vcc_5v0_sys>;
+		vcc11-supply = <&vcc_2v0_pldo_s3>;
+		vcc12-supply = <&vcc_5v0_sys>;
+		vcc13-supply = <&vcc_1v1_nldo_s3>;
+		vcc14-supply = <&vcc_1v1_nldo_s3>;
+		vcca-supply = <&vcc_5v0_sys>;
+
+		#gpio-cells = <2>;
+
+		rk806_dvs1_null: dvs1-null-pins {
+			pins = "gpio_pwrctrl1";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs1_pwrdn: dvs1-pwrdn-pins {
+			pins = "gpio_pwrctrl1";
+			function = "pin_fun2";
+		};
+
+		rk806_dvs1_rst: dvs1-rst-pins {
+			pins = "gpio_pwrctrl1";
+			function = "pin_fun3";
+		};
+
+		rk806_dvs1_slp: dvs1-slp-pins {
+			pins = "gpio_pwrctrl1";
+			function = "pin_fun1";
+		};
+
+		rk806_dvs2_dvs: dvs2-dvs-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun4";
+		};
+
+		rk806_dvs2_gpio: dvs2-gpio-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun5";
+		};
+
+		rk806_dvs2_null: dvs2-null-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs2_pwrdn: dvs2-pwrdn-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun2";
+		};
+
+		rk806_dvs2_rst: dvs2-rst-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun3";
+		};
+
+		rk806_dvs2_slp: dvs2-slp-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun1";
+		};
+
+		rk806_dvs3_dvs: dvs3-dvs-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun4";
+		};
+
+		rk806_dvs3_gpio: dvs3-gpio-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun5";
+		};
+
+		rk806_dvs3_null: dvs3-null-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs3_pwrdn: dvs3-pwrdn-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun2";
+		};
+
+		rk806_dvs3_rst: dvs3-rst-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun3";
+		};
+
+		rk806_dvs3_slp: dvs3-slp-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun1";
+		};
+
+		regulators {
+			vdd_cpu_big_s0: dcdc-reg1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-enable-ramp-delay = <400>;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-name = "vdd_cpu_big_s0";
+				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_npu_s0: dcdc-reg2 {
+				regulator-boot-on;
+				regulator-enable-ramp-delay = <400>;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-name = "vdd_npu_s0";
+				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_cpu_lit_s0: dcdc-reg3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-name = "vdd_cpu_lit_s0";
+				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <750000>;
+				};
+			};
+
+			vcc_3v3_s3: dcdc-reg4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc_3v3_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vdd_gpu_s0: dcdc-reg5 {
+				regulator-boot-on;
+				regulator-enable-ramp-delay = <400>;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <900000>;
+				regulator-name = "vdd_gpu_s0";
+				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <850000>;
+				};
+			};
+
+			vddq_ddr_s0: dcdc-reg6 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-name = "vddq_ddr_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_logic_s0: dcdc-reg7 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <800000>;
+				regulator-name = "vdd_logic_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8_s3: dcdc-reg8 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc_1v8_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vdd2_ddr_s3: dcdc-reg9 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-name = "vdd2_ddr_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vdd_ddr_s0: dcdc-reg10 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-name = "vdd_ddr_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcca_1v8_s0: pldo-reg1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcca_1v8_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcca1v8_pldo2_s0: pldo-reg2 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcca1v8_pldo2_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda_1v2_s0: pldo-reg3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-name = "vdda_1v2_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcca_3v3_s0: pldo-reg4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcca_3v3_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vccio_sd_s0: pldo-reg5 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vccio_sd_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcca1v8_pldo6_s3: pldo-reg6 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcca1v8_pldo6_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vdd_0v75_s3: nldo-reg1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+				regulator-name = "vdd_0v75_s3";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <750000>;
+				};
+			};
+
+			vdda_ddr_pll_s0: nldo-reg2 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <850000>;
+				regulator-name = "vdda_ddr_pll_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda0v75_hdmi_s0: nldo-reg3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <837500>;
+				regulator-max-microvolt = <837500>;
+				regulator-name = "vdda0v75_hdmi_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda_0v85_s0: nldo-reg4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <850000>;
+				regulator-name = "vdda_0v85_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda_0v75_s0: nldo-reg5 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+				regulator-name = "vdda_0v75_s0";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+		};
+	};
+};
+
+&i2c2 {
+	status = "okay";
+
+	hym8563: rtc@51 {
+		compatible = "haoyu,hym8563";
+		reg = <0x51>;
+		clock-output-names = "hym8563";
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PB0 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hym8563_int>;
+		wakeup-source;
+		#clock-cells = <0>;
+	};
+};
+
+&mdio0 {
+	rgmii_phy0: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0x1>;
+		clocks = <&cru REFCLKO25M_GMAC0_OUT>;
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reset-gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&pinctrl {
+	hym8563 {
+		hym8563_int: hym8563-int {
+			rockchip,pins = <0 RK_PA0 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	leds {
+		led_rgb_g: led-green-en {
+			rockchip,pins = <4 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		led_rgb_r: led-red-en {
+			rockchip,pins = <4 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	usb {
+		usb_host_pwren: usb-host-pwren {
+			rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+};
+
+&sdmmc {
+	bus-width = <4>;
+	cap-mmc-highspeed;
+	cap-sd-highspeed;
+	disable-wp;
+	max-frequency = <200000000>;
+	no-sdio;
+	no-mmc;
+	sd-uhs-sdr104;
+	vmmc-supply = <&vcc_3v3_s3>;
+	vqmmc-supply = <&vccio_sd_s0>;
+	status = "okay";
+};
+
+&u2phy0 {
+	status = "okay";
+};
+
+&u2phy1 {
+	status = "okay";
+};
+
+&uart0 {
+	pinctrl-0 = <&uart0m0_xfer>;
+	status = "okay";
+};
+
+&usb_drd1_dwc3 {
+	dr_mode = "host";
+	status = "okay";
+};
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree
  2025-02-17 16:34 ` [PATCH v4 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree Detlev Casanova
@ 2025-02-17 17:08   ` Jonas Karlman
  2025-02-17 21:07     ` Detlev Casanova
  0 siblings, 1 reply; 6+ messages in thread
From: Jonas Karlman @ 2025-02-17 17:08 UTC (permalink / raw)
  To: Detlev Casanova
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Chris Morgan, Kever Yang, Dragan Simic, Tim Lunn, FUKAUMI Naoki,
	Michael Riesch, Stephen Chen, Elon Zhang, Alexey Charkov,
	devicetree, linux-arm-kernel, linux-rockchip, kernel,
	linux-kernel

Hi Detlev,

On 2025-02-17 17:34, Detlev Casanova wrote:
> From: Stephen Chen <stephen@radxa.com>
> 
> The Radxa ROCK 4D board is based on the Rockchip rk3576 SoC.
> 
> The device tree adds support for basic devices:
>  - UART
>  - SD Card
>  - Ethernet
>  - USB
>  - RTC
> 
> It has 4 USB ports but only 3 are usable as the top left one is used
> for maskrom.
> 
> It has a USB-C port that is only used for powering the board.
> 
> Signed-off-by: Stephen Chen <stephen@radxa.com>
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>  .../boot/dts/rockchip/rk3576-rock-4d.dts      | 678 ++++++++++++++++++
>  2 files changed, 679 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index def1222c1907e..a112aeb37948a 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -132,6 +132,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-armsom-sige5.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-evb1-v10.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-rock-4d.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3582-radxa-e52c.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-sige7.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> new file mode 100644
> index 0000000000000..b499effd50396
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> @@ -0,0 +1,678 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2024 Radxa Computer (Shenzhen) Co., Ltd.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/pwm/pwm.h>
> +#include <dt-bindings/soc/rockchip,vop2.h>
> +#include <dt-bindings/usb/pd.h>
> +#include "rk3576.dtsi"
> +
> +/ {
> +	model = "Radxa ROCK 4D";
> +	compatible = "radxa,rock-4d", "rockchip,rk3576";
> +
> +	aliases {
> +		ethernet0 = &gmac0;

Should mmc alias be added here?

> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:1500000n8";
> +	};
> +
> +	leds: leds {
> +		compatible = "gpio-leds";
> +
> +		power-led {
> +			color = <LED_COLOR_ID_GREEN>;
> +			function = LED_FUNCTION_STATUS;
> +			gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +
> +		user-led {
> +			color = <LED_COLOR_ID_BLUE>;
> +			function = LED_FUNCTION_HEARTBEAT;
> +			gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "heartbeat";
> +		};
> +	};

pinctrl should probably be added for the leds above.

> +
> +	vcc_12v0_dcin: regulator-vcc-12v0-dcin {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-name = "vcc_12v0_dcin";
> +	};
> +
> +	vcc_1v1_nldo_s3: regulator-vcc-1v1-nldo-s3 {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1100000>;
> +		regulator-max-microvolt = <1100000>;
> +		regulator-name = "vcc_1v1_nldo_s3";
> +		vin-supply = <&vcc_5v0_sys>;
> +	};
> +
> +	vcc_1v2_ufs_vccq_s0: regulator-vcc-1v2-ufs-vccq-s0 {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1200000>;
> +		regulator-max-microvolt = <1200000>;
> +		regulator-name = "vcc_1v2_ufs_vccq_s0";
> +		vin-supply = <&vcc_5v0_sys>;
> +	};
> +
> +	vcc_1v8_s0: regulator-vcc-1v8-s0 {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-name = "vcc_1v8_s0";
> +		vin-supply = <&vcc_1v8_s3>;
> +	};
> +
> +	vcc_1v8_ufs_vccq2_s0: regulator-vcc1v8-ufs-vccq2-s0 {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-name = "vcc_1v8_ufs_vccq2_s0";
> +		vin-supply = <&vcc_1v8_s3>;
> +	};
> +
> +	vcc_2v0_pldo_s3: regulator-vcc-2v0-pldo-s3 {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <2000000>;
> +		regulator-max-microvolt = <2000000>;
> +		regulator-name = "vcc_2v0_pldo_s3";
> +		vin-supply = <&vcc_5v0_sys>;
> +	};
> +
> +	vcc_3v3_pcie: regulator-vcc-3v3-pcie {
> +		compatible = "regulator-fixed";
> +		enable-active-high;
> +		gpio = <&gpio2 RK_PD3 GPIO_ACTIVE_HIGH>;

If I am not mistaken gpios is the preferred property name?
This should probably also have pinctrl props.

> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-name = "vcc_3v3_pcie";
> +		startup-delay-us = <5000>;
> +		vin-supply = <&vcc_5v0_sys>;
> +	};
> +
> +	vcc_3v3_rtc_s5: regulator-vcc-3v3-rtc-s5 {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-name = "vcc_3v3_rtc_s5";
> +		vin-supply = <&vcc_5v0_sys>;
> +	};
> +
> +	vcc_3v3_s0: regulator-vcc-3v3-s0 {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-name = "vcc_3v3_s0";
> +		vin-supply = <&vcc_3v3_s3>;
> +	};
> +
> +	vcc_3v3_ufs_s0: regulator-vcc-ufs-s0 {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-name = "vcc_3v3_ufs_s0";
> +		vin-supply = <&vcc_5v0_sys>;
> +	};
> +
> +	vcc_5v0_device: regulator-vcc-5v0-device {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-name = "vcc_5v0_device";
> +		vin-supply = <&vcc_12v0_dcin>;
> +	};
> +
> +	vcc_5v0_host: regulator-vcc-5v0-host {
> +		compatible = "regulator-fixed";
> +		enable-active-high;
> +		gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;

Same here.

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&usb_host_pwren>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-name = "vcc5v0_host";
> +		vin-supply = <&vcc_5v0_device>;
> +	};
> +
> +	vcc_5v0_sys: regulator-vcc-5v0-sys {
> +		compatible = "regulator-fixed";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-name = "vcc_5v0_sys";
> +		vin-supply = <&vcc_12v0_dcin>;
> +	};
> +};
> +
> +&combphy1_psu {
> +	status = "okay";
> +};
> +
> +&cpu_b0 {
> +	cpu-supply = <&vdd_cpu_big_s0>;
> +};
> +
> +&cpu_b1 {
> +	cpu-supply = <&vdd_cpu_big_s0>;
> +};
> +
> +&cpu_b2 {
> +	cpu-supply = <&vdd_cpu_big_s0>;
> +};
> +
> +&cpu_b3 {
> +	cpu-supply = <&vdd_cpu_big_s0>;
> +};
> +
> +&cpu_l0 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +};
> +
> +&cpu_l1 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +};
> +
> +&cpu_l2 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +};
> +
> +&cpu_l3 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +};
> +
> +&gmac0 {
> +	phy-mode = "rgmii-id";
> +	clock_in_out = "output";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&eth0m0_miim
> +		     &eth0m0_tx_bus2
> +		     &eth0m0_rx_bus2
> +		     &eth0m0_rgmii_clk
> +		     &eth0m0_rgmii_bus
> +		     &ethm0_clk0_25m_out>;
> +
> +	phy-handle = <&rgmii_phy0>;
> +	status = "okay";
> +};

The extra blank lines is probably not needed in above node?
Props is also not sorted in this node, phy-handle and phy-mode should
probably be placed next to each other.

> +
> +&gpu {
> +	mali-supply = <&vdd_gpu_s0>;
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	status = "okay";
> +
> +	pmic@23 {
> +		compatible = "rockchip,rk806";
> +		reg = <0x23>;
> +
> +		gpio-controller;
> +
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_pins
> +			     &rk806_dvs1_null
> +			     &rk806_dvs2_null
> +			     &rk806_dvs3_null>;
> +
> +		system-power-controller;
> +
> +		vcc1-supply = <&vcc_5v0_sys>;
> +		vcc2-supply = <&vcc_5v0_sys>;
> +		vcc3-supply = <&vcc_5v0_sys>;
> +		vcc4-supply = <&vcc_5v0_sys>;
> +		vcc5-supply = <&vcc_5v0_sys>;
> +		vcc6-supply = <&vcc_5v0_sys>;
> +		vcc7-supply = <&vcc_5v0_sys>;
> +		vcc8-supply = <&vcc_5v0_sys>;
> +		vcc9-supply = <&vcc_5v0_sys>;
> +		vcc10-supply = <&vcc_5v0_sys>;
> +		vcc11-supply = <&vcc_2v0_pldo_s3>;
> +		vcc12-supply = <&vcc_5v0_sys>;
> +		vcc13-supply = <&vcc_1v1_nldo_s3>;
> +		vcc14-supply = <&vcc_1v1_nldo_s3>;
> +		vcca-supply = <&vcc_5v0_sys>;
> +
> +		#gpio-cells = <2>;

This should probably be sorted next to gpio-controller.

> +
> +		rk806_dvs1_null: dvs1-null-pins {
> +			pins = "gpio_pwrctrl1";
> +			function = "pin_fun0";
> +		};
> +
> +		rk806_dvs1_pwrdn: dvs1-pwrdn-pins {
> +			pins = "gpio_pwrctrl1";
> +			function = "pin_fun2";
> +		};
> +
> +		rk806_dvs1_rst: dvs1-rst-pins {
> +			pins = "gpio_pwrctrl1";
> +			function = "pin_fun3";
> +		};
> +
> +		rk806_dvs1_slp: dvs1-slp-pins {
> +			pins = "gpio_pwrctrl1";
> +			function = "pin_fun1";
> +		};
> +
> +		rk806_dvs2_dvs: dvs2-dvs-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun4";
> +		};
> +
> +		rk806_dvs2_gpio: dvs2-gpio-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun5";
> +		};
> +
> +		rk806_dvs2_null: dvs2-null-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun0";
> +		};
> +
> +		rk806_dvs2_pwrdn: dvs2-pwrdn-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun2";
> +		};
> +
> +		rk806_dvs2_rst: dvs2-rst-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun3";
> +		};
> +
> +		rk806_dvs2_slp: dvs2-slp-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun1";
> +		};
> +
> +		rk806_dvs3_dvs: dvs3-dvs-pins {
> +			pins = "gpio_pwrctrl3";
> +			function = "pin_fun4";
> +		};
> +
> +		rk806_dvs3_gpio: dvs3-gpio-pins {
> +			pins = "gpio_pwrctrl3";
> +			function = "pin_fun5";
> +		};
> +
> +		rk806_dvs3_null: dvs3-null-pins {
> +			pins = "gpio_pwrctrl3";
> +			function = "pin_fun0";
> +		};
> +
> +		rk806_dvs3_pwrdn: dvs3-pwrdn-pins {
> +			pins = "gpio_pwrctrl3";
> +			function = "pin_fun2";
> +		};
> +
> +		rk806_dvs3_rst: dvs3-rst-pins {
> +			pins = "gpio_pwrctrl3";
> +			function = "pin_fun3";
> +		};
> +
> +		rk806_dvs3_slp: dvs3-slp-pins {
> +			pins = "gpio_pwrctrl3";
> +			function = "pin_fun1";
> +		};
> +
> +		regulators {
> +			vdd_cpu_big_s0: dcdc-reg1 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-enable-ramp-delay = <400>;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <950000>;
> +				regulator-name = "vdd_cpu_big_s0";
> +				regulator-ramp-delay = <12500>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_npu_s0: dcdc-reg2 {
> +				regulator-boot-on;
> +				regulator-enable-ramp-delay = <400>;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <950000>;
> +				regulator-name = "vdd_npu_s0";
> +				regulator-ramp-delay = <12500>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_cpu_lit_s0: dcdc-reg3 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <950000>;
> +				regulator-name = "vdd_cpu_lit_s0";
> +				regulator-ramp-delay = <12500>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +					regulator-suspend-microvolt = <750000>;
> +				};
> +			};
> +
> +			vcc_3v3_s3: dcdc-reg4 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc_3v3_s3";
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <3300000>;
> +				};
> +			};
> +
> +			vdd_gpu_s0: dcdc-reg5 {
> +				regulator-boot-on;
> +				regulator-enable-ramp-delay = <400>;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <900000>;
> +				regulator-name = "vdd_gpu_s0";
> +				regulator-ramp-delay = <12500>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +					regulator-suspend-microvolt = <850000>;
> +				};
> +			};
> +
> +			vddq_ddr_s0: dcdc-reg6 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-name = "vddq_ddr_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_logic_s0: dcdc-reg7 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <800000>;
> +				regulator-name = "vdd_logic_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_1v8_s3: dcdc-reg8 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc_1v8_s3";
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			vdd2_ddr_s3: dcdc-reg9 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-name = "vdd2_ddr_s3";
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +				};
> +			};
> +
> +			vdd_ddr_s0: dcdc-reg10 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-name = "vdd_ddr_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcca_1v8_s0: pldo-reg1 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcca_1v8_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcca1v8_pldo2_s0: pldo-reg2 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcca1v8_pldo2_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda_1v2_s0: pldo-reg3 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-name = "vdda_1v2_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcca_3v3_s0: pldo-reg4 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcca_3v3_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vccio_sd_s0: pldo-reg5 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vccio_sd_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcca1v8_pldo6_s3: pldo-reg6 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcca1v8_pldo6_s3";
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			vdd_0v75_s3: nldo-reg1 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <750000>;
> +				regulator-max-microvolt = <750000>;
> +				regulator-name = "vdd_0v75_s3";
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <750000>;
> +				};
> +			};
> +
> +			vdda_ddr_pll_s0: nldo-reg2 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <850000>;
> +				regulator-max-microvolt = <850000>;
> +				regulator-name = "vdda_ddr_pll_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda0v75_hdmi_s0: nldo-reg3 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <837500>;
> +				regulator-max-microvolt = <837500>;
> +				regulator-name = "vdda0v75_hdmi_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda_0v85_s0: nldo-reg4 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <850000>;
> +				regulator-max-microvolt = <850000>;
> +				regulator-name = "vdda_0v85_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda_0v75_s0: nldo-reg5 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <750000>;
> +				regulator-max-microvolt = <750000>;
> +				regulator-name = "vdda_0v75_s0";
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&i2c2 {
> +	status = "okay";
> +
> +	hym8563: rtc@51 {
> +		compatible = "haoyu,hym8563";
> +		reg = <0x51>;
> +		clock-output-names = "hym8563";
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <RK_PB0 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hym8563_int>;
> +		wakeup-source;
> +		#clock-cells = <0>;

This should probably be sorted next to clock-output-names.

> +	};
> +};
> +
> +&mdio0 {
> +	rgmii_phy0: ethernet-phy@1 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0x1>;
> +		clocks = <&cru REFCLKO25M_GMAC0_OUT>;
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reset-gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_LOW>;

Pinctrl should probably be added here.

> +	};
> +};
> +
> +&pinctrl {
> +	hym8563 {
> +		hym8563_int: hym8563-int {
> +			rockchip,pins = <0 RK_PA0 RK_FUNC_GPIO &pcfg_pull_up>;

This does not match (A0 vs B0) the pin used in rtc@51.

> +		};
> +	};
> +
> +	leds {
> +		led_rgb_g: led-green-en {
> +			rockchip,pins = <4 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +		led_rgb_r: led-red-en {
> +			rockchip,pins = <4 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};

These are unreferenced and does not match the pin used by gpio leds.

> +	};
> +
> +	usb {
> +		usb_host_pwren: usb-host-pwren {
> +			rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>;

This does not match the pin used in the regulator.

Regards,
Jonas

> +		};
> +	};
> +};
> +
> +&sdmmc {
> +	bus-width = <4>;
> +	cap-mmc-highspeed;
> +	cap-sd-highspeed;
> +	disable-wp;
> +	max-frequency = <200000000>;
> +	no-sdio;
> +	no-mmc;
> +	sd-uhs-sdr104;
> +	vmmc-supply = <&vcc_3v3_s3>;
> +	vqmmc-supply = <&vccio_sd_s0>;
> +	status = "okay";
> +};
> +
> +&u2phy0 {
> +	status = "okay";
> +};
> +
> +&u2phy1 {
> +	status = "okay";
> +};
> +
> +&uart0 {
> +	pinctrl-0 = <&uart0m0_xfer>;
> +	status = "okay";
> +};
> +
> +&usb_drd1_dwc3 {
> +	dr_mode = "host";
> +	status = "okay";
> +};



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree
  2025-02-17 17:08   ` Jonas Karlman
@ 2025-02-17 21:07     ` Detlev Casanova
  2025-02-17 21:30       ` Heiko Stübner
  0 siblings, 1 reply; 6+ messages in thread
From: Detlev Casanova @ 2025-02-17 21:07 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Chris Morgan, Kever Yang, Dragan Simic, Tim Lunn, FUKAUMI Naoki,
	Michael Riesch, Stephen Chen, Elon Zhang, Alexey Charkov,
	devicetree, linux-arm-kernel, linux-rockchip, kernel,
	linux-kernel

Hi Jonas,

On Monday, 17 February 2025 12:08:47 EST Jonas Karlman wrote:
> Hi Detlev,
> 
> On 2025-02-17 17:34, Detlev Casanova wrote:
> > From: Stephen Chen <stephen@radxa.com>
> > 
> > The Radxa ROCK 4D board is based on the Rockchip rk3576 SoC.
> > 
> > The device tree adds support for basic devices:
> >  - UART
> >  - SD Card
> >  - Ethernet
> >  - USB
> >  - RTC
> > 
> > It has 4 USB ports but only 3 are usable as the top left one is used
> > for maskrom.
> > 
> > It has a USB-C port that is only used for powering the board.
> > 
> > Signed-off-by: Stephen Chen <stephen@radxa.com>
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> > 
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3576-rock-4d.dts      | 678 ++++++++++++++++++
> >  2 files changed, 679 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile
> > b/arch/arm64/boot/dts/rockchip/Makefile index
> > def1222c1907e..a112aeb37948a 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -132,6 +132,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) +=
> > rk3568-wolfvision-pf5-display-vz.dtbo> 
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-armsom-sige5.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-evb1-v10.dtb
> > 
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-rock-4d.dtb
> > 
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3582-radxa-e52c.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-sige7.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-armsom-w3.dtb
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> > b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts new file mode 100644
> > index 0000000000000..b499effd50396
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> > @@ -0,0 +1,678 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2024 Radxa Computer (Shenzhen) Co., Ltd.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include <dt-bindings/pwm/pwm.h>
> > +#include <dt-bindings/soc/rockchip,vop2.h>
> > +#include <dt-bindings/usb/pd.h>
> > +#include "rk3576.dtsi"
> > +
> > +/ {
> > +	model = "Radxa ROCK 4D";
> > +	compatible = "radxa,rock-4d", "rockchip,rk3576";
> > +
> > +	aliases {
> > +		ethernet0 = &gmac0;
> 
> Should mmc alias be added here?
>
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:1500000n8";
> > +	};
> > +
> > +	leds: leds {
> > +		compatible = "gpio-leds";
> > +
> > +		power-led {
> > +			color = <LED_COLOR_ID_GREEN>;
> > +			function = LED_FUNCTION_STATUS;
> > +			gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> > +			linux,default-trigger = "default-on";
> > +		};
> > +
> > +		user-led {
> > +			color = <LED_COLOR_ID_BLUE>;
> > +			function = LED_FUNCTION_HEARTBEAT;
> > +			gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_LOW>;
> > +			linux,default-trigger = "heartbeat";
> > +		};
> > +	};
> 
> pinctrl should probably be added for the leds above.
> 
> > +
> > +	vcc_12v0_dcin: regulator-vcc-12v0-dcin {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +		regulator-name = "vcc_12v0_dcin";
> > +	};
> > +
> > +	vcc_1v1_nldo_s3: regulator-vcc-1v1-nldo-s3 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <1100000>;
> > +		regulator-max-microvolt = <1100000>;
> > +		regulator-name = "vcc_1v1_nldo_s3";
> > +		vin-supply = <&vcc_5v0_sys>;
> > +	};
> > +
> > +	vcc_1v2_ufs_vccq_s0: regulator-vcc-1v2-ufs-vccq-s0 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <1200000>;
> > +		regulator-max-microvolt = <1200000>;
> > +		regulator-name = "vcc_1v2_ufs_vccq_s0";
> > +		vin-supply = <&vcc_5v0_sys>;
> > +	};
> > +
> > +	vcc_1v8_s0: regulator-vcc-1v8-s0 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <1800000>;
> > +		regulator-max-microvolt = <1800000>;
> > +		regulator-name = "vcc_1v8_s0";
> > +		vin-supply = <&vcc_1v8_s3>;
> > +	};
> > +
> > +	vcc_1v8_ufs_vccq2_s0: regulator-vcc1v8-ufs-vccq2-s0 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <1800000>;
> > +		regulator-max-microvolt = <1800000>;
> > +		regulator-name = "vcc_1v8_ufs_vccq2_s0";
> > +		vin-supply = <&vcc_1v8_s3>;
> > +	};
> > +
> > +	vcc_2v0_pldo_s3: regulator-vcc-2v0-pldo-s3 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <2000000>;
> > +		regulator-max-microvolt = <2000000>;
> > +		regulator-name = "vcc_2v0_pldo_s3";
> > +		vin-supply = <&vcc_5v0_sys>;
> > +	};
> > +
> > +	vcc_3v3_pcie: regulator-vcc-3v3-pcie {
> > +		compatible = "regulator-fixed";
> > +		enable-active-high;
> > +		gpio = <&gpio2 RK_PD3 GPIO_ACTIVE_HIGH>;
> 
> If I am not mistaken gpios is the preferred property name?

Hehe, looking at the rk3588-rock-5b dts, both "gpio" and "gpios" are used, 
I'll use the second one.

> This should probably also have pinctrl props.

Yes, I'll fix all those pinctrl issues.

> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-name = "vcc_3v3_pcie";
> > +		startup-delay-us = <5000>;
> > +		vin-supply = <&vcc_5v0_sys>;
> > +	};
> > +
> > +	vcc_3v3_rtc_s5: regulator-vcc-3v3-rtc-s5 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-name = "vcc_3v3_rtc_s5";
> > +		vin-supply = <&vcc_5v0_sys>;
> > +	};
> > +
> > +	vcc_3v3_s0: regulator-vcc-3v3-s0 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-name = "vcc_3v3_s0";
> > +		vin-supply = <&vcc_3v3_s3>;
> > +	};
> > +
> > +	vcc_3v3_ufs_s0: regulator-vcc-ufs-s0 {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-name = "vcc_3v3_ufs_s0";
> > +		vin-supply = <&vcc_5v0_sys>;
> > +	};
> > +
> > +	vcc_5v0_device: regulator-vcc-5v0-device {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <5000000>;
> > +		regulator-max-microvolt = <5000000>;
> > +		regulator-name = "vcc_5v0_device";
> > +		vin-supply = <&vcc_12v0_dcin>;
> > +	};
> > +
> > +	vcc_5v0_host: regulator-vcc-5v0-host {
> > +		compatible = "regulator-fixed";
> > +		enable-active-high;
> > +		gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;
> 
> Same here.
> 
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&usb_host_pwren>;
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <5000000>;
> > +		regulator-max-microvolt = <5000000>;
> > +		regulator-name = "vcc5v0_host";
> > +		vin-supply = <&vcc_5v0_device>;
> > +	};
> > +
> > +	vcc_5v0_sys: regulator-vcc-5v0-sys {
> > +		compatible = "regulator-fixed";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <5000000>;
> > +		regulator-max-microvolt = <5000000>;
> > +		regulator-name = "vcc_5v0_sys";
> > +		vin-supply = <&vcc_12v0_dcin>;
> > +	};
> > +};
> > +
> > +&combphy1_psu {
> > +	status = "okay";
> > +};
> > +
> > +&cpu_b0 {
> > +	cpu-supply = <&vdd_cpu_big_s0>;
> > +};
> > +
> > +&cpu_b1 {
> > +	cpu-supply = <&vdd_cpu_big_s0>;
> > +};
> > +
> > +&cpu_b2 {
> > +	cpu-supply = <&vdd_cpu_big_s0>;
> > +};
> > +
> > +&cpu_b3 {
> > +	cpu-supply = <&vdd_cpu_big_s0>;
> > +};
> > +
> > +&cpu_l0 {
> > +	cpu-supply = <&vdd_cpu_lit_s0>;
> > +};
> > +
> > +&cpu_l1 {
> > +	cpu-supply = <&vdd_cpu_lit_s0>;
> > +};
> > +
> > +&cpu_l2 {
> > +	cpu-supply = <&vdd_cpu_lit_s0>;
> > +};
> > +
> > +&cpu_l3 {
> > +	cpu-supply = <&vdd_cpu_lit_s0>;
> > +};
> > +
> > +&gmac0 {
> > +	phy-mode = "rgmii-id";
> > +	clock_in_out = "output";
> > +
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&eth0m0_miim
> > +		     &eth0m0_tx_bus2
> > +		     &eth0m0_rx_bus2
> > +		     &eth0m0_rgmii_clk
> > +		     &eth0m0_rgmii_bus
> > +		     &ethm0_clk0_25m_out>;
> > +
> > +	phy-handle = <&rgmii_phy0>;
> > +	status = "okay";
> > +};
> 
> The extra blank lines is probably not needed in above node?
> Props is also not sorted in this node, phy-handle and phy-mode should
> probably be placed next to each other.
> 
> > +
> > +&gpu {
> > +	mali-supply = <&vdd_gpu_s0>;
> > +	status = "okay";
> > +};
> > +
> > +&i2c1 {
> > +	status = "okay";
> > +
> > +	pmic@23 {
> > +		compatible = "rockchip,rk806";
> > +		reg = <0x23>;
> > +
> > +		gpio-controller;
> > +
> > +		interrupt-parent = <&gpio0>;
> > +		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pmic_pins
> > +			     &rk806_dvs1_null
> > +			     &rk806_dvs2_null
> > +			     &rk806_dvs3_null>;
> > +
> > +		system-power-controller;
> > +
> > +		vcc1-supply = <&vcc_5v0_sys>;
> > +		vcc2-supply = <&vcc_5v0_sys>;
> > +		vcc3-supply = <&vcc_5v0_sys>;
> > +		vcc4-supply = <&vcc_5v0_sys>;
> > +		vcc5-supply = <&vcc_5v0_sys>;
> > +		vcc6-supply = <&vcc_5v0_sys>;
> > +		vcc7-supply = <&vcc_5v0_sys>;
> > +		vcc8-supply = <&vcc_5v0_sys>;
> > +		vcc9-supply = <&vcc_5v0_sys>;
> > +		vcc10-supply = <&vcc_5v0_sys>;
> > +		vcc11-supply = <&vcc_2v0_pldo_s3>;
> > +		vcc12-supply = <&vcc_5v0_sys>;
> > +		vcc13-supply = <&vcc_1v1_nldo_s3>;
> > +		vcc14-supply = <&vcc_1v1_nldo_s3>;
> > +		vcca-supply = <&vcc_5v0_sys>;
> > +
> > +		#gpio-cells = <2>;
> 
> This should probably be sorted next to gpio-controller.

It's not unusual to put # props at the end. but I can move it up if it is 
preferred.

> > +
> > +		rk806_dvs1_null: dvs1-null-pins {
> > +			pins = "gpio_pwrctrl1";
> > +			function = "pin_fun0";
> > +		};
> > +
> > +		rk806_dvs1_pwrdn: dvs1-pwrdn-pins {
> > +			pins = "gpio_pwrctrl1";
> > +			function = "pin_fun2";
> > +		};
> > +
> > +		rk806_dvs1_rst: dvs1-rst-pins {
> > +			pins = "gpio_pwrctrl1";
> > +			function = "pin_fun3";
> > +		};
> > +
> > +		rk806_dvs1_slp: dvs1-slp-pins {
> > +			pins = "gpio_pwrctrl1";
> > +			function = "pin_fun1";
> > +		};
> > +
> > +		rk806_dvs2_dvs: dvs2-dvs-pins {
> > +			pins = "gpio_pwrctrl2";
> > +			function = "pin_fun4";
> > +		};
> > +
> > +		rk806_dvs2_gpio: dvs2-gpio-pins {
> > +			pins = "gpio_pwrctrl2";
> > +			function = "pin_fun5";
> > +		};
> > +
> > +		rk806_dvs2_null: dvs2-null-pins {
> > +			pins = "gpio_pwrctrl2";
> > +			function = "pin_fun0";
> > +		};
> > +
> > +		rk806_dvs2_pwrdn: dvs2-pwrdn-pins {
> > +			pins = "gpio_pwrctrl2";
> > +			function = "pin_fun2";
> > +		};
> > +
> > +		rk806_dvs2_rst: dvs2-rst-pins {
> > +			pins = "gpio_pwrctrl2";
> > +			function = "pin_fun3";
> > +		};
> > +
> > +		rk806_dvs2_slp: dvs2-slp-pins {
> > +			pins = "gpio_pwrctrl2";
> > +			function = "pin_fun1";
> > +		};
> > +
> > +		rk806_dvs3_dvs: dvs3-dvs-pins {
> > +			pins = "gpio_pwrctrl3";
> > +			function = "pin_fun4";
> > +		};
> > +
> > +		rk806_dvs3_gpio: dvs3-gpio-pins {
> > +			pins = "gpio_pwrctrl3";
> > +			function = "pin_fun5";
> > +		};
> > +
> > +		rk806_dvs3_null: dvs3-null-pins {
> > +			pins = "gpio_pwrctrl3";
> > +			function = "pin_fun0";
> > +		};
> > +
> > +		rk806_dvs3_pwrdn: dvs3-pwrdn-pins {
> > +			pins = "gpio_pwrctrl3";
> > +			function = "pin_fun2";
> > +		};
> > +
> > +		rk806_dvs3_rst: dvs3-rst-pins {
> > +			pins = "gpio_pwrctrl3";
> > +			function = "pin_fun3";
> > +		};
> > +
> > +		rk806_dvs3_slp: dvs3-slp-pins {
> > +			pins = "gpio_pwrctrl3";
> > +			function = "pin_fun1";
> > +		};
> > +
> > +		regulators {
> > +			vdd_cpu_big_s0: dcdc-reg1 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-enable-ramp-delay = <400>;
> > +				regulator-min-microvolt = <550000>;
> > +				regulator-max-microvolt = <950000>;
> > +				regulator-name = "vdd_cpu_big_s0";
> > +				regulator-ramp-delay = <12500>;
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_npu_s0: dcdc-reg2 {
> > +				regulator-boot-on;
> > +				regulator-enable-ramp-delay = <400>;
> > +				regulator-min-microvolt = <550000>;
> > +				regulator-max-microvolt = <950000>;
> > +				regulator-name = "vdd_npu_s0";
> > +				regulator-ramp-delay = <12500>;
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_cpu_lit_s0: dcdc-reg3 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <550000>;
> > +				regulator-max-microvolt = <950000>;
> > +				regulator-name = "vdd_cpu_lit_s0";
> > +				regulator-ramp-delay = <12500>;
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +					regulator-suspend-microvolt = 
<750000>;
> > +				};
> > +			};
> > +
> > +			vcc_3v3_s3: dcdc-reg4 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcc_3v3_s3";
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = 
<3300000>;
> > +				};
> > +			};
> > +
> > +			vdd_gpu_s0: dcdc-reg5 {
> > +				regulator-boot-on;
> > +				regulator-enable-ramp-delay = <400>;
> > +				regulator-min-microvolt = <550000>;
> > +				regulator-max-microvolt = <900000>;
> > +				regulator-name = "vdd_gpu_s0";
> > +				regulator-ramp-delay = <12500>;
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +					regulator-suspend-microvolt = 
<850000>;
> > +				};
> > +			};
> > +
> > +			vddq_ddr_s0: dcdc-reg6 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-name = "vddq_ddr_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_logic_s0: dcdc-reg7 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <550000>;
> > +				regulator-max-microvolt = <800000>;
> > +				regulator-name = "vdd_logic_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_1v8_s3: dcdc-reg8 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcc_1v8_s3";
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = 
<1800000>;
> > +				};
> > +			};
> > +
> > +			vdd2_ddr_s3: dcdc-reg9 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-name = "vdd2_ddr_s3";
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_ddr_s0: dcdc-reg10 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <550000>;
> > +				regulator-max-microvolt = <1200000>;
> > +				regulator-name = "vdd_ddr_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcca_1v8_s0: pldo-reg1 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcca_1v8_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcca1v8_pldo2_s0: pldo-reg2 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcca1v8_pldo2_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda_1v2_s0: pldo-reg3 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1200000>;
> > +				regulator-max-microvolt = <1200000>;
> > +				regulator-name = "vdda_1v2_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcca_3v3_s0: pldo-reg4 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcca_3v3_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vccio_sd_s0: pldo-reg5 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vccio_sd_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcca1v8_pldo6_s3: pldo-reg6 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcca1v8_pldo6_s3";
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = 
<1800000>;
> > +				};
> > +			};
> > +
> > +			vdd_0v75_s3: nldo-reg1 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <750000>;
> > +				regulator-max-microvolt = <750000>;
> > +				regulator-name = "vdd_0v75_s3";
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = 
<750000>;
> > +				};
> > +			};
> > +
> > +			vdda_ddr_pll_s0: nldo-reg2 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <850000>;
> > +				regulator-max-microvolt = <850000>;
> > +				regulator-name = "vdda_ddr_pll_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda0v75_hdmi_s0: nldo-reg3 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <837500>;
> > +				regulator-max-microvolt = <837500>;
> > +				regulator-name = "vdda0v75_hdmi_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda_0v85_s0: nldo-reg4 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <850000>;
> > +				regulator-max-microvolt = <850000>;
> > +				regulator-name = "vdda_0v85_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda_0v75_s0: nldo-reg5 {
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <750000>;
> > +				regulator-max-microvolt = <750000>;
> > +				regulator-name = "vdda_0v75_s0";
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&i2c2 {
> > +	status = "okay";
> > +
> > +	hym8563: rtc@51 {
> > +		compatible = "haoyu,hym8563";
> > +		reg = <0x51>;
> > +		clock-output-names = "hym8563";
> > +		interrupt-parent = <&gpio0>;
> > +		interrupts = <RK_PB0 IRQ_TYPE_LEVEL_LOW>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&hym8563_int>;
> > +		wakeup-source;
> > +		#clock-cells = <0>;
> 
> This should probably be sorted next to clock-output-names.

Same for # props.

> > +	};
> > +};
> > +
> > +&mdio0 {
> > +	rgmii_phy0: ethernet-phy@1 {
> > +		compatible = "ethernet-phy-ieee802.3-c22";
> > +		reg = <0x1>;
> > +		clocks = <&cru REFCLKO25M_GMAC0_OUT>;
> > +		reset-assert-us = <20000>;
> > +		reset-deassert-us = <100000>;
> > +		reset-gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_LOW>;
> 
> Pinctrl should probably be added here.
> 
> > +	};
> > +};
> > +
> > +&pinctrl {
> > +	hym8563 {
> > +		hym8563_int: hym8563-int {
> > +			rockchip,pins = <0 RK_PA0 RK_FUNC_GPIO 
&pcfg_pull_up>;
> 
> This does not match (A0 vs B0) the pin used in rtc@51.

This probably explains the errors I've seen with the rtc.

> > +		};
> > +	};
> > +
> > +	leds {
> > +		led_rgb_g: led-green-en {
> > +			rockchip,pins = <4 RK_PB2 RK_FUNC_GPIO 
&pcfg_pull_none>;
> > +		};
> > +		led_rgb_r: led-red-en {
> > +			rockchip,pins = <4 RK_PB1 RK_FUNC_GPIO 
&pcfg_pull_none>;
> > +		};
> 
> These are unreferenced and does not match the pin used by gpio leds.
> 
> > +	};
> > +
> > +	usb {
> > +		usb_host_pwren: usb-host-pwren {
> > +			rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO 
&pcfg_pull_none>;
> 
> This does not match the pin used in the regulator.

I'll fix the pinctrl issues, I had to improve my understanding of those a bit 
:)

let me know for those # props.

Regards,
Detlev.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree
  2025-02-17 21:07     ` Detlev Casanova
@ 2025-02-17 21:30       ` Heiko Stübner
  0 siblings, 0 replies; 6+ messages in thread
From: Heiko Stübner @ 2025-02-17 21:30 UTC (permalink / raw)
  To: Jonas Karlman, Detlev Casanova
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chris Morgan,
	Kever Yang, Dragan Simic, Tim Lunn, FUKAUMI Naoki, Michael Riesch,
	Stephen Chen, Elon Zhang, Alexey Charkov, devicetree,
	linux-arm-kernel, linux-rockchip, kernel, linux-kernel

Am Montag, 17. Februar 2025, 22:07:06 MEZ schrieb Detlev Casanova:
> On Monday, 17 February 2025 12:08:47 EST Jonas Karlman wrote:
> > On 2025-02-17 17:34, Detlev Casanova wrote:
> > > +	pmic@23 {
> > > +		compatible = "rockchip,rk806";
> > > +		reg = <0x23>;
> > > +
> > > +		gpio-controller;
> > > +
> > > +		interrupt-parent = <&gpio0>;
> > > +		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
> > > +
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&pmic_pins
> > > +			     &rk806_dvs1_null
> > > +			     &rk806_dvs2_null
> > > +			     &rk806_dvs3_null>;
> > > +
> > > +		system-power-controller;
> > > +
> > > +		vcc1-supply = <&vcc_5v0_sys>;
> > > +		vcc2-supply = <&vcc_5v0_sys>;
> > > +		vcc3-supply = <&vcc_5v0_sys>;
> > > +		vcc4-supply = <&vcc_5v0_sys>;
> > > +		vcc5-supply = <&vcc_5v0_sys>;
> > > +		vcc6-supply = <&vcc_5v0_sys>;
> > > +		vcc7-supply = <&vcc_5v0_sys>;
> > > +		vcc8-supply = <&vcc_5v0_sys>;
> > > +		vcc9-supply = <&vcc_5v0_sys>;
> > > +		vcc10-supply = <&vcc_5v0_sys>;
> > > +		vcc11-supply = <&vcc_2v0_pldo_s3>;
> > > +		vcc12-supply = <&vcc_5v0_sys>;
> > > +		vcc13-supply = <&vcc_1v1_nldo_s3>;
> > > +		vcc14-supply = <&vcc_1v1_nldo_s3>;
> > > +		vcca-supply = <&vcc_5v0_sys>;
> > > +
> > > +		#gpio-cells = <2>;
> > 
> > This should probably be sorted next to gpio-controller.
> 
> It's not unusual to put # props at the end. but I can move it up if it is 
> preferred.

That is actually a multi-colored bikeshed ;-)

When sorting alphabetically, do you
- just ignore the "#", this would move #gpio-cells to gpio*  but also
  split up #address-cells and #size-cells
- count "#" as special character and move them to the bottom, but this
  would split #gpio-cells from gpio-controller

--- TL;DR
So far I've not managed to come with a 1-size-fits-all opinion, but for
#gpio* and #clock* properties, readability gets better when they are
together with other gpio* / clock* properties .
----

Also I think you could do away with all the empty lines between properties
above (pinctrl <-> system-power-controller, etc), but of course keep the
empty between the subnodes below :-)


Heiko




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-02-17 21:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-17 16:34 [PATCH v4 0/2] Add Radxa Rock 4D support Detlev Casanova
2025-02-17 16:34 ` [PATCH v4 1/2] dt-bindings: arm: rockchip: Add Radxa ROCK 4D board Detlev Casanova
2025-02-17 16:34 ` [PATCH v4 2/2] arm64: dts: rockchip: Add Radxa ROCK 4D device tree Detlev Casanova
2025-02-17 17:08   ` Jonas Karlman
2025-02-17 21:07     ` Detlev Casanova
2025-02-17 21:30       ` Heiko Stübner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).