[PATCH 0/3] arm64: dts: rockchip: Add device tree for the Orange Pi CM5 Base board

[PATCH 0/3] arm64: dts: rockchip: Add device tree for the Orange Pi CM5 Base board

[Laurent Pinchart]

Hello,

This patch series adds a device tree for the Orange Pi CM5 Base board
from Xunlong. This is a combination of a compute module and a carrier
board, so the device tree is split in two files.

The work is based on a combination of upstream device trees for other
RK3588-based Orange Pi boards and the downstream device tree, all
checked against the available schematics for the carrier board. The
compute module schematics is unfortunately not available.

The series starts by adding a new compatible for the board to
arm/rockchip.yaml. The next patch documents a missing property in the
rk3588-dw-hdmi-qp bindings that the device tree needs. Finally, the last
patch adds the device tree.

Patch 2/3 may be slightly controversial as the new DT property could be
better named. It has been supported in the driver for a year now, so
there could be users in the wild. I have therefore decided to keep the
current name. I am open to alternative solutions.

Laurent Pinchart (3):
  dt-bindings: arm: rockchip: Add Orange Pi CM5 Base
  dt-bindings: display: rk3588-dw-hdmi-qp: Document enable-gpios
    property
  arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

 .../devicetree/bindings/arm/rockchip.yaml     |   6 +
 .../rockchip/rockchip,rk3588-dw-hdmi-qp.yaml  |   4 +
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../rockchip/rk3588s-orangepi-cm5-base.dts    | 342 +++++++++++++
 .../dts/rockchip/rk3588s-orangepi-cm5.dtsi    | 450 ++++++++++++++++++
 5 files changed, 803 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5-base.dts
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5.dtsi


base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
-- 
Regards,

Laurent Pinchart

[PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi CM5 Base

[Laurent Pinchart]

The Orange Pi CM5 Base board is a carrier board for the Orange Pi CM5
compute module. It has 3 ethernet ports, 2 USB ports, one HDMI output
and 4 CSI-2 inputs.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index 28db6bd6aa5b..8b08a57f96ce 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -1243,6 +1243,12 @@ properties:
               - xunlong,orangepi-5b
           - const: rockchip,rk3588s
 
+      - description: Xunlong Orange Pi CM5
+        items:
+          - const: xunlong,orangepi-cm5-base
+          - const: xunlong,orangepi-cm5
+          - const: rockchip,rk3588s
+
       - description: Zkmagic A95X Z2
         items:
           - const: zkmagic,a95x-z2
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi CM5 Base

[Conor Dooley]

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

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

[PATCH 2/3] dt-bindings: display: rk3588-dw-hdmi-qp: Document enable-gpios property

[Laurent Pinchart]

The RK3588 DW HDMI QP encoder only supports AC-coupled links as defined
by the HDMI 2.1 FRL mode. In order to be compliant with the DC common
mode requirements of HDMI 2.0 (and earlier) TMDS mode, the middle point
of the termination resistors needs to be connected to ground. Document a
property to toggle this connection using a GPIO.

The property could be better named, but it has been introduced in the
Linux kernel driver for this device a year ago in commit 128a9bf8ace2
("drm/rockchip: Add basic RK3588 HDMI output support"). While it is not
used by any in-tree device tree, it may be used in the wild.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
index 96b4b088eebe..7471c2f51c32 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
@@ -103,6 +103,10 @@ properties:
   "#sound-dai-cells":
     const: 0
 
+  enable-gpios:
+    description:
+      GPIO to switch between TMDS (when active) and FRS (when inactive) modes.
+
   rockchip,grf:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] dt-bindings: display: rk3588-dw-hdmi-qp: Document enable-gpios property

[Conor Dooley]

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

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

[PATCH 3/3] arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

[Laurent Pinchart]

The Orange Pi CM5 Base board is a carrier board for the Orange Pi CM5
compute module. It has 3 ethernet ports, 2 USB ports, one HDMI output
and 4 CSI-2 inputs.

The device tree is split in two files, a .dtsi for the compute module
and a .dts for the carrier board. All the devices present on the carrier
board are enabled and tested, with the exception of the IR receiver due
to missing support for input capture in the PWM device's DT binding (and
driver).

This work is based on a combination of the Orange Pi 5 device tree from
the upstream kernel and the Orange Pi CM5 device tree from the BSP
kernel. All nodes and properties have been carefully checked to the best
of my abilities against the schematics of the carrier board. The
schematics of the compute module is not available publicly, so the
configuration of the PMIC hasn't been double-checked.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../rockchip/rk3588s-orangepi-cm5-base.dts    | 342 +++++++++++++
 .../dts/rockchip/rk3588s-orangepi-cm5.dtsi    | 450 ++++++++++++++++++
 3 files changed, 793 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5-base.dts
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5.dtsi

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index 40ef0ad71816..079b17ad532f 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -197,6 +197,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-nanopi-r6c.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-odroid-m2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5b.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-cm5-base.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-roc-pc.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5a.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5c.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5-base.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5-base.dts
new file mode 100644
index 000000000000..9dbb7724e7ae
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5-base.dts
@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.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 "rk3588s-orangepi-cm5.dtsi"
+
+/ {
+	model = "Xunlong Orange Pi CM5 Base";
+	compatible = "xunlong,orangepi-cm5-base", "xunlong,orangepi-cm5", "rockchip,rk3588s";
+
+	aliases {
+		ethernet0 = &gmac1;
+		mmc0 = &sdhci;
+		mmc1 = &sdmmc;
+	};
+
+	chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+		pinctrl-names = "default";
+		pinctrl-0 = <&key1_pin>;
+
+		button {
+			gpios = <&gpio3 RK_PC6 GPIO_ACTIVE_LOW>;
+			label = "USERKEY";
+			linux,code = <BTN_MISC>;
+			debounce-interval = <50>;
+			wakeup-source;
+		};
+	};
+
+	hdmi0-con {
+		compatible = "hdmi-connector";
+		type = "a";
+
+		port {
+			hdmi0_con_in: endpoint {
+				remote-endpoint = <&hdmi0_out_con>;
+			};
+		};
+	};
+
+	pwm-leds {
+		compatible = "pwm-leds";
+
+		led-1 {
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_STATUS;
+			linux,default-trigger = "heartbeat";
+			max-brightness = <255>;
+			pwms = <&pwm2 0 25000 0>;
+		};
+
+		led-2 {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_WAN;
+			max-brightness = <255>;
+			pwms = <&pwm4 0 25000 PWM_POLARITY_INVERTED>;
+		};
+
+		led-3 {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_LAN;
+			function-enumerator = <0>;
+			max-brightness = <255>;
+			pwms = <&pwm5 0 25000 PWM_POLARITY_INVERTED>;
+		};
+
+		led-4 {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_LAN;
+			function-enumerator = <1>;
+			max-brightness = <255>;
+			pwms = <&pwm6 0 25000 0>;
+		};
+	};
+
+	vbus_5v0: vbus-5v0 {
+		compatible = "regulator-fixed";
+		regulator-name = "vbus_5v0";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&vcc5v0_sys>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vbus_5v0_en_pin>;
+	};
+
+	vcc_3v3: regulator-vcc-3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		startup-delay-us = <50000>;
+		enable-active-high;
+		gpio = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&vcc5v0_sys>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vcc_3v3_en_pin>;
+	};
+
+	vcc5v0_sys: regulator-vcc-5v0 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc5v0_sys";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+};
+
+&combphy0_ps {
+	status = "okay";
+};
+
+&combphy2_psu {
+	status = "okay";
+};
+
+&gmac1 {
+	clock_in_out = "output";
+	phy-handle = <&rgmii_phy>;
+	phy-mode = "rgmii-id";
+	phy-supply = <&vcc_3v3>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac1_miim
+		     &gmac1_rx_bus2
+		     &gmac1_tx_bus2
+		     &gmac1_rgmii_clk
+		     &gmac1_rgmii_bus>;
+	tx_delay = <0x42>;
+	status = "okay";
+};
+
+&gpu {
+	mali-supply = <&vdd_gpu_s0>;
+	status = "okay";
+};
+
+&hdmi0 {
+	enable-gpios = <&gpio4 RK_PB5 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+};
+
+&hdmi0_in {
+	hdmi0_in_vp0: endpoint {
+		remote-endpoint = <&vp0_out_hdmi0>;
+	};
+};
+
+&hdmi0_out {
+	hdmi0_out_con: endpoint {
+		remote-endpoint = <&hdmi0_con_in>;
+	};
+};
+
+&hdmi0_sound {
+	status = "okay";
+};
+
+&hdptxphy0 {
+	status = "okay";
+};
+
+&i2c1 {
+	pinctrl-0 = <&i2c1m2_xfer>;
+	status = "okay";
+
+	rtc@51 {
+		compatible = "haoyu,hym8563";
+		reg = <0x51>;
+		#clock-cells = <0>;
+		clock-output-names = "hym8563";
+		pinctrl-names = "default";
+		pinctrl-0 = <&rtc_int_pin>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PB0 IRQ_TYPE_LEVEL_LOW>;
+		wakeup-source;
+	};
+};
+
+&i2s5_8ch {
+	status = "okay";
+};
+
+&mdio1 {
+	rgmii_phy: ethernet-phy@1 {
+		/* YT8531C */
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0x1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&rgmii_phy_pin>;
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reset-gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&pcie2x1l1 {
+	reset-gpios = <&gpio4 RK_PA2 GPIO_ACTIVE_HIGH>;
+	vpcie3v3-supply = <&vcc_3v3>;
+	status = "okay";
+};
+
+&pcie2x1l2 {
+	reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
+	vpcie3v3-supply = <&vcc_3v3>;
+	status = "okay";
+};
+
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
+&pinctrl {
+	ethernet {
+		rgmii_phy_pin: rgmii-phy-pin {
+			rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	gpio-key {
+		key1_pin: key1-pin {
+			rockchip,pins = <3 RK_PC6 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	power {
+		vcc_3v3_en_pin: vcc-3v3-en-pin {
+			rockchip,pins = <4 RK_PA3 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+
+	rtc {
+		rtc_int_pin: rtc-int-pin {
+			rockchip,pins = <0 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	usb {
+		vbus_5v0_en_pin: vbus-5v0-en-pin {
+			rockchip,pins = <0 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+};
+
+&pwm2 {
+	status = "okay";
+};
+
+&pwm4 {
+	status = "okay";
+};
+
+&pwm5 {
+	pinctrl-0 = <&pwm5m1_pins>;
+	status = "okay";
+};
+
+&pwm6 {
+	status = "okay";
+};
+
+&sdmmc {
+	bus-width = <4>;
+	cap-sd-highspeed;
+	disable-wp;
+	max-frequency = <150000000>;
+	no-mmc;
+	no-sdio;
+	sd-uhs-sdr104;
+	vmmc-supply = <&vcc_3v3_s3>;
+	vqmmc-supply = <&vccio_sd_s0>;
+	status = "okay";
+};
+
+&u2phy0 {
+	status = "okay";
+};
+
+&u2phy0_otg {
+	phy-supply = <&vbus_5v0>;
+	status = "okay";
+};
+
+&u2phy2 {
+	status = "okay";
+};
+
+&u2phy2_host {
+	phy-supply = <&vbus_5v0>;
+	status = "okay";
+};
+
+&uart2 {
+	pinctrl-0 = <&uart2m0_xfer>;
+	status = "okay";
+};
+
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};
+
+&usb_host0_xhci {
+	dr_mode = "host";
+	status = "okay";
+};
+
+&usbdp_phy0 {
+	status = "okay";
+};
+
+&vop {
+	status = "okay";
+};
+
+&vop_mmu {
+	status = "okay";
+};
+
+&vp0 {
+	vp0_out_hdmi0: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
+		reg = <ROCKCHIP_VOP2_EP_HDMI0>;
+		remote-endpoint = <&hdmi0_in_vp0>;
+	};
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5.dtsi
new file mode 100644
index 000000000000..aa3407fceba9
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5.dtsi
@@ -0,0 +1,450 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+
+#include <dt-bindings/clock/rockchip,rk3588-cru.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+#include "rk3588s.dtsi"
+#include "rk8xx.h"
+
+/ {
+	/* Can't be verified due to missing schematics for the CM5. */
+	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_1v1_nldo_s3";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1100000>;
+		regulator-max-microvolt = <1100000>;
+		vin-supply = <&vcc5v0_sys>;
+	};
+};
+
+&cpu_b0 {
+	cpu-supply = <&vdd_cpu_big0_s0>;
+};
+
+&cpu_b1 {
+	cpu-supply = <&vdd_cpu_big0_s0>;
+};
+
+&cpu_b2 {
+	cpu-supply = <&vdd_cpu_big1_s0>;
+};
+
+&cpu_b3 {
+	cpu-supply = <&vdd_cpu_big1_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>;
+};
+
+&i2c0 {
+	pinctrl-0 = <&i2c0m2_xfer>;
+	status = "okay";
+
+	vdd_cpu_big0_s0: regulator@42 {
+		compatible = "rockchip,rk8602";
+		reg = <0x42>;
+		fcs,suspend-voltage-selector = <1>;
+		regulator-name = "vdd_cpu_big0_s0";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <550000>;
+		regulator-max-microvolt = <1050000>;
+		regulator-ramp-delay = <2300>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+
+	vdd_cpu_big1_s0: regulator@43 {
+		compatible = "rockchip,rk8603", "rockchip,rk8602";
+		reg = <0x43>;
+		fcs,suspend-voltage-selector = <1>;
+		regulator-name = "vdd_cpu_big1_s0";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <550000>;
+		regulator-max-microvolt = <1050000>;
+		regulator-ramp-delay = <2300>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+};
+
+&i2c2 {
+	status = "okay";
+
+	vdd_npu_s0: regulator@42 {
+		compatible = "rockchip,rk8602";
+		reg = <0x42>;
+		fcs,suspend-voltage-selector = <1>;
+		regulator-name = "vdd_npu_s0";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <550000>;
+		regulator-max-microvolt = <950000>;
+		regulator-ramp-delay = <2300>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+};
+
+&sdhci {
+	bus-width = <8>;
+	no-sdio;
+	no-sd;
+	non-removable;
+	max-frequency = <200000000>;
+	mmc-hs400-1_8v;
+	mmc-hs400-enhanced-strobe;
+	status = "okay";
+};
+
+&spi2 {
+	assigned-clocks = <&cru CLK_SPI2>;
+	assigned-clock-rates = <200000000>;
+	num-cs = <1>;
+	pinctrl-0 = <&spi2m2_cs0 &spi2m2_pins>;
+	status = "okay";
+
+	pmic@0 {
+		compatible = "rockchip,rk806";
+		reg = <0x0>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
+			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
+		spi-max-frequency = <1000000>;
+		system-power-controller;
+		rockchip,reset-mode = <RK806_RESET>;
+
+		vcc1-supply = <&vcc5v0_sys>;
+		vcc2-supply = <&vcc5v0_sys>;
+		vcc3-supply = <&vcc5v0_sys>;
+		vcc4-supply = <&vcc5v0_sys>;
+		vcc5-supply = <&vcc5v0_sys>;
+		vcc6-supply = <&vcc5v0_sys>;
+		vcc7-supply = <&vcc5v0_sys>;
+		vcc8-supply = <&vcc5v0_sys>;
+		vcc9-supply = <&vcc5v0_sys>;
+		vcc10-supply = <&vcc5v0_sys>;
+		vcc11-supply = <&vcc_2v0_pldo_s3>;
+		vcc12-supply = <&vcc5v0_sys>;
+		vcc13-supply = <&vcc_1v1_nldo_s3>;
+		vcc14-supply = <&vcc_1v1_nldo_s3>;
+		vcca-supply = <&vcc5v0_sys>;
+
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		rk806_dvs1_null: dvs1-null-pins {
+			pins = "gpio_pwrctrl1";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs2_null: dvs2-null-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs3_null: dvs3-null-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun0";
+		};
+
+		regulators {
+			vdd_gpu_s0: dcdc-reg1 {
+				regulator-name = "vdd_gpu_s0";
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <400>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_cpu_lit_s0: dcdc-reg2 {
+				regulator-name = "vdd_cpu_lit_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_log_s0: dcdc-reg3 {
+				regulator-name = "vdd_log_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <675000>;
+				regulator-max-microvolt = <750000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <750000>;
+				};
+			};
+
+			vdd_vdenc_s0: dcdc-reg4 {
+				regulator-name = "vdd_vdenc_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_ddr_s0: dcdc-reg5 {
+				regulator-name = "vdd_ddr_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <675000>;
+				regulator-max-microvolt = <900000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <850000>;
+				};
+			};
+
+			vdd2_ddr_s3: dcdc-reg6 {
+				regulator-name = "vdd2_ddr_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-max-microvolt = <1100000>;
+				regulator-min-microvolt = <1100000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vcc_2v0_pldo_s3: dcdc-reg7 {
+				regulator-name = "vdd_2v0_pldo_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <2000000>;
+				regulator-max-microvolt = <2000000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <2000000>;
+				};
+			};
+
+			vcc_3v3_s3: dcdc-reg8 {
+				regulator-name = "vcc_3v3_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vddq_ddr_s0: dcdc-reg9 {
+				regulator-name = "vddq_ddr_s0";
+				regulator-always-on;
+				regulator-boot-on;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8_s3: dcdc-reg10 {
+				regulator-name = "vcc_1v8_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			avcc_1v8_s0: pldo-reg1 {
+				regulator-name = "avcc_1v8_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8_s0: pldo-reg2 {
+				regulator-name = "vcc_1v8_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			avdd_1v2_s0: pldo-reg3 {
+				regulator-name = "avdd_1v2_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_3v3_s0: pldo-reg4 {
+				regulator-name = "vcc_3v3_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vccio_sd_s0: pldo-reg5 {
+				regulator-name = "vccio_sd_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			pldo6_s3: pldo-reg6 {
+				regulator-name = "pldo6_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vdd_0v75_s3: nldo-reg1 {
+				regulator-name = "vdd_0v75_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <750000>;
+				};
+			};
+
+			vdd_ddr_pll_s0: nldo-reg2 {
+				regulator-name = "vdd_ddr_pll_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <850000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <850000>;
+				};
+			};
+
+			avdd_0v75_s0: nldo-reg3 {
+				regulator-name = "avdd_0v75_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_0v85_s0: nldo-reg4 {
+				regulator-name = "vdd_0v85_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <850000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_0v75_s0: nldo-reg5 {
+				regulator-name = "vdd_0v75_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+		};
+	};
+};
+
+&tsadc {
+	status = "okay";
+};
-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

[Jimmy Hon]

A few nitpicks below

[ snip ]
> +
> +#include "rk3588s-orangepi-cm5.dtsi"
> +
> +/ {
> +       model = "Xunlong Orange Pi CM5 Base";
> +       compatible = "xunlong,orangepi-cm5-base", "xunlong,orangepi-cm5", "rockchip,rk3588s";
> +
> +       aliases {
> +               ethernet0 = &gmac1;
> +               mmc0 = &sdhci;
Since sdhci is enabled in the SoM.dtsi, this alias should probably go
there instead.

> +               mmc1 = &sdmmc;
> +       };
> +

[ snip ]

> +
> +       vbus_5v0: vbus-5v0 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vbus_5v0";
> +               regulator-min-microvolt = <5000000>;
> +               regulator-max-microvolt = <5000000>;
> +               enable-active-high;
> +               gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;
> +               vin-supply = <&vcc5v0_sys>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&vbus_5v0_en_pin>;
The property names in these regulators are not as organized as the
regulators for the CPU/NPU.

> +       };
> +
> +       vcc_3v3: regulator-vcc-3v3 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vcc_3v3";
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +               startup-delay-us = <50000>;
> +               enable-active-high;
> +               gpio = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
> +               vin-supply = <&vcc5v0_sys>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&vcc_3v3_en_pin>;
> +       };
> +
> +       vcc5v0_sys: regulator-vcc-5v0 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vcc5v0_sys";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-min-microvolt = <5000000>;
> +               regulator-max-microvolt = <5000000>;
> +       };
> +};

[ snip ]

> +
> +&gmac1 {
> +       clock_in_out = "output";
> +       phy-handle = <&rgmii_phy>;
> +       phy-mode = "rgmii-id";
> +       phy-supply = <&vcc_3v3>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&gmac1_miim
> +                    &gmac1_rx_bus2
> +                    &gmac1_tx_bus2
> +                    &gmac1_rgmii_clk
> +                    &gmac1_rgmii_bus>;
> +       tx_delay = <0x42>;
When using "rgmii-id", tx_delay will be ignored. Does the ethernet
work without this property?

See the comment by Jonas in another review.
https://lore.kernel.org/linux-rockchip/da752790-da17-4d26-b9b2-8240b38b3276@kwiboo.se/

> +       status = "okay";
> +};
> +
> +&gpu {
> +       mali-supply = <&vdd_gpu_s0>;
> +       status = "okay";
> +};
This is a feature in the SoC itself, so it's not board specific and
can be put into the SoM.dtsi.

[ snip ]

> +
> +&pd_gpu {
> +       domain-supply = <&vdd_gpu_s0>;
> +};
Same comment regarding moving to the SoM.dtsi

> --
> Regards,
>
> Laurent Pinchart
>
Jimmy Hon

Re: [PATCH 3/3] arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

[Laurent Pinchart]

Hi Jimmy,

On Thu, Oct 02, 2025 at 07:01:53PM -0500, Jimmy Hon wrote:
> A few nitpicks below
> 
> [ snip ]
> > +
> > +#include "rk3588s-orangepi-cm5.dtsi"
> > +
> > +/ {
> > +       model = "Xunlong Orange Pi CM5 Base";
> > +       compatible = "xunlong,orangepi-cm5-base", "xunlong,orangepi-cm5", "rockchip,rk3588s";
> > +
> > +       aliases {
> > +               ethernet0 = &gmac1;
> > +               mmc0 = &sdhci;
> 
> Since sdhci is enabled in the SoM.dtsi, this alias should probably go
> there instead.

Good point, I'll do that.

> > +               mmc1 = &sdmmc;
> > +       };
> > +
> 
> [ snip ]
> 
> > +
> > +       vbus_5v0: vbus-5v0 {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "vbus_5v0";
> > +               regulator-min-microvolt = <5000000>;
> > +               regulator-max-microvolt = <5000000>;
> > +               enable-active-high;
> > +               gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;
> > +               vin-supply = <&vcc5v0_sys>;
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&vbus_5v0_en_pin>;
> 
> The property names in these regulators are not as organized as the
> regulators for the CPU/NPU.

Which properties in particular ? There are more properties in these
regulators, but otherwise the order seem to match.

> > +       };
> > +
> > +       vcc_3v3: regulator-vcc-3v3 {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "vcc_3v3";
> > +               regulator-min-microvolt = <3300000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               startup-delay-us = <50000>;
> > +               enable-active-high;
> > +               gpio = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
> > +               vin-supply = <&vcc5v0_sys>;
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&vcc_3v3_en_pin>;
> > +       };
> > +
> > +       vcc5v0_sys: regulator-vcc-5v0 {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "vcc5v0_sys";
> > +               regulator-always-on;
> > +               regulator-boot-on;
> > +               regulator-min-microvolt = <5000000>;
> > +               regulator-max-microvolt = <5000000>;
> > +       };
> > +};
> 
> [ snip ]
> 
> > +
> > +&gmac1 {
> > +       clock_in_out = "output";
> > +       phy-handle = <&rgmii_phy>;
> > +       phy-mode = "rgmii-id";
> > +       phy-supply = <&vcc_3v3>;
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&gmac1_miim
> > +                    &gmac1_rx_bus2
> > +                    &gmac1_tx_bus2
> > +                    &gmac1_rgmii_clk
> > +                    &gmac1_rgmii_bus>;
> > +       tx_delay = <0x42>;
> 
> When using "rgmii-id", tx_delay will be ignored. Does the ethernet
> work without this property?

I have to confess this was blindly copied from the BSP :-/ I'll drop the
property and test.

> See the comment by Jonas in another review.
> https://lore.kernel.org/linux-rockchip/da752790-da17-4d26-b9b2-8240b38b3276@kwiboo.se/
> 
> > +       status = "okay";
> > +};
> > +
> > +&gpu {
> > +       mali-supply = <&vdd_gpu_s0>;
> > +       status = "okay";
> > +};
> 
> This is a feature in the SoC itself, so it's not board specific and
> can be put into the SoM.dtsi.

I'm a bit in two minds here. If a carrier board doesn't have a display
output, the GPU isn't very useful (although in theory the GPU can be
used without a display). That's why I decided to enable it in the
carrier board. I suppose it doesn't hurt to enable it in the SoM, worst
case it won't be used and so won't be powered up. I'll move it to the
SoM.

> [ snip ]
> 
> > +
> > +&pd_gpu {
> > +       domain-supply = <&vdd_gpu_s0>;
> > +};
> 
> Same comment regarding moving to the SoM.dtsi

OK.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

[Jimmy Hon]

On Thu, Oct 2, 2025 at 9:40 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Jimmy,
>
> On Thu, Oct 02, 2025 at 07:01:53PM -0500, Jimmy Hon wrote:
> > A few nitpicks below
> >
> > [ snip ]
> > > +
> > > +#include "rk3588s-orangepi-cm5.dtsi"
> > > +
> > > +/ {
> > > +       model = "Xunlong Orange Pi CM5 Base";
> > > +       compatible = "xunlong,orangepi-cm5-base", "xunlong,orangepi-cm5", "rockchip,rk3588s";
> > > +
> > > +       aliases {
> > > +               ethernet0 = &gmac1;
> > > +               mmc0 = &sdhci;
> >
> > Since sdhci is enabled in the SoM.dtsi, this alias should probably go
> > there instead.
>
> Good point, I'll do that.
>
> > > +               mmc1 = &sdmmc;
> > > +       };
> > > +
> >
> > [ snip ]
> >
> > > +
> > > +       vbus_5v0: vbus-5v0 {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "vbus_5v0";
> > > +               regulator-min-microvolt = <5000000>;
> > > +               regulator-max-microvolt = <5000000>;
> > > +               enable-active-high;
> > > +               gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;
> > > +               vin-supply = <&vcc5v0_sys>;
> > > +               pinctrl-names = "default";
> > > +               pinctrl-0 = <&vbus_5v0_en_pin>;
> >
> > The property names in these regulators are not as organized as the
> > regulators for the CPU/NPU.
>
> Which properties in particular ? There are more properties in these
> regulators, but otherwise the order seem to match.
>
> > > +       };
> > > +
> > > +       vcc_3v3: regulator-vcc-3v3 {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "vcc_3v3";
> > > +               regulator-min-microvolt = <3300000>;
> > > +               regulator-max-microvolt = <3300000>;
> > > +               startup-delay-us = <50000>;
> > > +               enable-active-high;
> > > +               gpio = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
> > > +               vin-supply = <&vcc5v0_sys>;
> > > +               pinctrl-names = "default";
> > > +               pinctrl-0 = <&vcc_3v3_en_pin>;
> > > +       };
The majority of the properties should be in alphabetical order. So the
startup-delay-us and vin-supply are out of place.

> > > +
> > > +       vcc5v0_sys: regulator-vcc-5v0 {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "vcc5v0_sys";
> > > +               regulator-always-on;
> > > +               regulator-boot-on;
> > > +               regulator-min-microvolt = <5000000>;
> > > +               regulator-max-microvolt = <5000000>;
> > > +       };
> > > +};
> >
> > [ snip ]
> >
> > > +
> > > +&gmac1 {
> > > +       clock_in_out = "output";
> > > +       phy-handle = <&rgmii_phy>;
> > > +       phy-mode = "rgmii-id";
> > > +       phy-supply = <&vcc_3v3>;
> > > +       pinctrl-names = "default";
> > > +       pinctrl-0 = <&gmac1_miim
> > > +                    &gmac1_rx_bus2
> > > +                    &gmac1_tx_bus2
> > > +                    &gmac1_rgmii_clk
> > > +                    &gmac1_rgmii_bus>;
> > > +       tx_delay = <0x42>;
> >
> > When using "rgmii-id", tx_delay will be ignored. Does the ethernet
> > work without this property?
>
> I have to confess this was blindly copied from the BSP :-/ I'll drop the
> property and test.
>
> > See the comment by Jonas in another review.
> > https://lore.kernel.org/linux-rockchip/da752790-da17-4d26-b9b2-8240b38b3276@kwiboo.se/
> >
> > > +       status = "okay";
> > > +};
> > > +
> > > +&gpu {
> > > +       mali-supply = <&vdd_gpu_s0>;
> > > +       status = "okay";
> > > +};
> >
> > This is a feature in the SoC itself, so it's not board specific and
> > can be put into the SoM.dtsi.
>
> I'm a bit in two minds here. If a carrier board doesn't have a display
> output, the GPU isn't very useful (although in theory the GPU can be
> used without a display). That's why I decided to enable it in the
> carrier board. I suppose it doesn't hurt to enable it in the SoM, worst
> case it won't be used and so won't be powered up. I'll move it to the
> SoM.
The nice thing about the G610 GPU is that OpenCL support via Mesa's
RustICL was added earlier this year. So even in a headless cluster,
the GPU can still be useful.

>
> > [ snip ]
> >
> > > +
> > > +&pd_gpu {
> > > +       domain-supply = <&vdd_gpu_s0>;
> > > +};
> >
> > Same comment regarding moving to the SoM.dtsi
>
> OK.
>
> --
> Regards,
>
> Laurent Pinchart

Jimmy

Re: [PATCH 3/3] arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

[Laurent Pinchart]

On Fri, Oct 03, 2025 at 09:47:40PM -0500, Jimmy Hon wrote:
> On Thu, Oct 2, 2025 at 9:40 PM Laurent Pinchart wrote:
> > On Thu, Oct 02, 2025 at 07:01:53PM -0500, Jimmy Hon wrote:
> > > A few nitpicks below
> > >
> > > [ snip ]
> > > > +
> > > > +#include "rk3588s-orangepi-cm5.dtsi"
> > > > +
> > > > +/ {
> > > > +       model = "Xunlong Orange Pi CM5 Base";
> > > > +       compatible = "xunlong,orangepi-cm5-base", "xunlong,orangepi-cm5", "rockchip,rk3588s";
> > > > +
> > > > +       aliases {
> > > > +               ethernet0 = &gmac1;
> > > > +               mmc0 = &sdhci;
> > >
> > > Since sdhci is enabled in the SoM.dtsi, this alias should probably go
> > > there instead.
> >
> > Good point, I'll do that.
> >
> > > > +               mmc1 = &sdmmc;
> > > > +       };
> > > > +
> > >
> > > [ snip ]
> > >
> > > > +
> > > > +       vbus_5v0: vbus-5v0 {
> > > > +               compatible = "regulator-fixed";
> > > > +               regulator-name = "vbus_5v0";
> > > > +               regulator-min-microvolt = <5000000>;
> > > > +               regulator-max-microvolt = <5000000>;
> > > > +               enable-active-high;
> > > > +               gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;
> > > > +               vin-supply = <&vcc5v0_sys>;
> > > > +               pinctrl-names = "default";
> > > > +               pinctrl-0 = <&vbus_5v0_en_pin>;
> > >
> > > The property names in these regulators are not as organized as the
> > > regulators for the CPU/NPU.
> >
> > Which properties in particular ? There are more properties in these
> > regulators, but otherwise the order seem to match.
> >
> > > > +       };
> > > > +
> > > > +       vcc_3v3: regulator-vcc-3v3 {
> > > > +               compatible = "regulator-fixed";
> > > > +               regulator-name = "vcc_3v3";
> > > > +               regulator-min-microvolt = <3300000>;
> > > > +               regulator-max-microvolt = <3300000>;
> > > > +               startup-delay-us = <50000>;
> > > > +               enable-active-high;
> > > > +               gpio = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
> > > > +               vin-supply = <&vcc5v0_sys>;
> > > > +               pinctrl-names = "default";
> > > > +               pinctrl-0 = <&vcc_3v3_en_pin>;
> > > > +       };
>
> The majority of the properties should be in alphabetical order. So the
> startup-delay-us and vin-supply are out of place.

OK I'll move those.

> > > > +
> > > > +       vcc5v0_sys: regulator-vcc-5v0 {
> > > > +               compatible = "regulator-fixed";
> > > > +               regulator-name = "vcc5v0_sys";
> > > > +               regulator-always-on;
> > > > +               regulator-boot-on;
> > > > +               regulator-min-microvolt = <5000000>;
> > > > +               regulator-max-microvolt = <5000000>;
> > > > +       };
> > > > +};
> > >
> > > [ snip ]
> > >
> > > > +
> > > > +&gmac1 {
> > > > +       clock_in_out = "output";
> > > > +       phy-handle = <&rgmii_phy>;
> > > > +       phy-mode = "rgmii-id";
> > > > +       phy-supply = <&vcc_3v3>;
> > > > +       pinctrl-names = "default";
> > > > +       pinctrl-0 = <&gmac1_miim
> > > > +                    &gmac1_rx_bus2
> > > > +                    &gmac1_tx_bus2
> > > > +                    &gmac1_rgmii_clk
> > > > +                    &gmac1_rgmii_bus>;
> > > > +       tx_delay = <0x42>;
> > >
> > > When using "rgmii-id", tx_delay will be ignored. Does the ethernet
> > > work without this property?
> >
> > I have to confess this was blindly copied from the BSP :-/ I'll drop the
> > property and test.
> >
> > > See the comment by Jonas in another review.
> > > https://lore.kernel.org/linux-rockchip/da752790-da17-4d26-b9b2-8240b38b3276@kwiboo.se/
> > >
> > > > +       status = "okay";
> > > > +};
> > > > +
> > > > +&gpu {
> > > > +       mali-supply = <&vdd_gpu_s0>;
> > > > +       status = "okay";
> > > > +};
> > >
> > > This is a feature in the SoC itself, so it's not board specific and
> > > can be put into the SoM.dtsi.
> >
> > I'm a bit in two minds here. If a carrier board doesn't have a display
> > output, the GPU isn't very useful (although in theory the GPU can be
> > used without a display). That's why I decided to enable it in the
> > carrier board. I suppose it doesn't hurt to enable it in the SoM, worst
> > case it won't be used and so won't be powered up. I'll move it to the
> > SoM.
>
> The nice thing about the G610 GPU is that OpenCL support via Mesa's
> RustICL was added earlier this year. So even in a headless cluster,
> the GPU can still be useful.

Yes, that's why I decided to move it to the SoM in the end.

> > > [ snip ]
> > >
> > > > +
> > > > +&pd_gpu {
> > > > +       domain-supply = <&vdd_gpu_s0>;
> > > > +};
> > >
> > > Same comment regarding moving to the SoM.dtsi
> >
> > OK.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

[Andrew Lunn]

> +&gmac1 {
> +	clock_in_out = "output";
> +	phy-handle = <&rgmii_phy>;
> +	phy-mode = "rgmii-id";
> +	phy-supply = <&vcc_3v3>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac1_miim
> +		     &gmac1_rx_bus2
> +		     &gmac1_tx_bus2
> +		     &gmac1_rgmii_clk
> +		     &gmac1_rgmii_bus>;
> +	tx_delay = <0x42>;

Nice to see phy-mode 'rgmii-id', but 0x42 is pretty high in comparison
to other boards correctly using 'rgmii-id', and is asymmetric with no
rx_delay value.

I cannot say this is wrong, it is just odd, not what i would expect.

  Andrew

Re: [PATCH 3/3] arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

[Laurent Pinchart]

On Mon, Oct 06, 2025 at 06:06:21PM +0200, Andrew Lunn wrote:
> > +&gmac1 {
> > +	clock_in_out = "output";
> > +	phy-handle = <&rgmii_phy>;
> > +	phy-mode = "rgmii-id";
> > +	phy-supply = <&vcc_3v3>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&gmac1_miim
> > +		     &gmac1_rx_bus2
> > +		     &gmac1_tx_bus2
> > +		     &gmac1_rgmii_clk
> > +		     &gmac1_rgmii_bus>;
> > +	tx_delay = <0x42>;
> 
> Nice to see phy-mode 'rgmii-id', but 0x42 is pretty high in comparison
> to other boards correctly using 'rgmii-id', and is asymmetric with no
> rx_delay value.
> 
> I cannot say this is wrong, it is just odd, not what i would expect.

I copied it blindly from the BSP. Dropping the property, I can still
ping the board, so I removed it in v2. There's more that will need to be
investigated there, as clock_in_out should likely be "input", but I
couldn't get that to work. I'd like to leave this to someone else as I'm
lacking time to fix it (and ethernet is also not my strongest
expertise). I could drop the gmac1 completely, but I think the starting
point is valuable.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/3] arm64: dts: rockchip: Add device tree for the Orange Pi CM5 Base board

[Cristian Ciocaltea]

Hi Laurent,

On 10/2/25 6:47 AM, Laurent Pinchart wrote:
> Hello,
> 
> This patch series adds a device tree for the Orange Pi CM5 Base board
> from Xunlong. This is a combination of a compute module and a carrier
> board, so the device tree is split in two files.
> 
> The work is based on a combination of upstream device trees for other
> RK3588-based Orange Pi boards and the downstream device tree, all
> checked against the available schematics for the carrier board. The
> compute module schematics is unfortunately not available.
> 
> The series starts by adding a new compatible for the board to
> arm/rockchip.yaml. The next patch documents a missing property in the
> rk3588-dw-hdmi-qp bindings that the device tree needs. Finally, the last
> patch adds the device tree.
> 
> Patch 2/3 may be slightly controversial as the new DT property could be
> better named. 

Indeed :-). As a matter of fact I've already planned to introduce it as
"tmds-enable-gpios" [1], during my recent FRL related work.

> It has been supported in the driver for a year now, so
> there could be users in the wild. I have therefore decided to keep the
> current name. I am open to alternative solutions.

As mentioned in [2], this was "inherited" from downstream and hasn't been
really in use so far.  I intended to do some more testing before attempting
to send those patches upstream, as for now I couldn't notice any (obvious)
change in behavior when switching between TMDS and FRL, with or without
making use of it in DT.

Regards,
Cristian

[1] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/5a32a84ee3e801daaeb974f967f6e213983feeb4
[2] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/df4d94fdb94463dd4175338bf7044b40c5e559e4

Re: [PATCH 0/3] arm64: dts: rockchip: Add device tree for the Orange Pi CM5 Base board

[Laurent Pinchart]

Hi Cristian,

On Thu, Oct 02, 2025 at 10:24:59PM +0300, Cristian Ciocaltea wrote:
> On 10/2/25 6:47 AM, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series adds a device tree for the Orange Pi CM5 Base board
> > from Xunlong. This is a combination of a compute module and a carrier
> > board, so the device tree is split in two files.
> > 
> > The work is based on a combination of upstream device trees for other
> > RK3588-based Orange Pi boards and the downstream device tree, all
> > checked against the available schematics for the carrier board. The
> > compute module schematics is unfortunately not available.
> > 
> > The series starts by adding a new compatible for the board to
> > arm/rockchip.yaml. The next patch documents a missing property in the
> > rk3588-dw-hdmi-qp bindings that the device tree needs. Finally, the last
> > patch adds the device tree.
> > 
> > Patch 2/3 may be slightly controversial as the new DT property could be
> > better named. 
> 
> Indeed :-). As a matter of fact I've already planned to introduce it as
> "tmds-enable-gpios" [1], during my recent FRL related work.
> 
> > It has been supported in the driver for a year now, so
> > there could be users in the wild. I have therefore decided to keep the
> > current name. I am open to alternative solutions.
> 
> As mentioned in [2], this was "inherited" from downstream and hasn't been
> really in use so far.  I intended to do some more testing before attempting
> to send those patches upstream, as for now I couldn't notice any (obvious)
> change in behavior when switching between TMDS and FRL, with or without
> making use of it in DT.

Thanks for the links. The new property name is better. We may need to
preserve backward compatibility though, how confident are you that the
enabled-gpios property isn't used in the wild ?

I'd like to get the device tree for the Orange Pi CM5 merged sooner than
later. Would you post those two patches, or should I take them in the v2
of my series ?

> [1] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/5a32a84ee3e801daaeb974f967f6e213983feeb4
> [2] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/df4d94fdb94463dd4175338bf7044b40c5e559e4

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/3] arm64: dts: rockchip: Add device tree for the Orange Pi CM5 Base board

[Cristian Ciocaltea]

On 10/2/25 11:52 PM, Laurent Pinchart wrote:
> Hi Cristian,
> 
> On Thu, Oct 02, 2025 at 10:24:59PM +0300, Cristian Ciocaltea wrote:
>> On 10/2/25 6:47 AM, Laurent Pinchart wrote:
>>> Hello,
>>>
>>> This patch series adds a device tree for the Orange Pi CM5 Base board
>>> from Xunlong. This is a combination of a compute module and a carrier
>>> board, so the device tree is split in two files.
>>>
>>> The work is based on a combination of upstream device trees for other
>>> RK3588-based Orange Pi boards and the downstream device tree, all
>>> checked against the available schematics for the carrier board. The
>>> compute module schematics is unfortunately not available.
>>>
>>> The series starts by adding a new compatible for the board to
>>> arm/rockchip.yaml. The next patch documents a missing property in the
>>> rk3588-dw-hdmi-qp bindings that the device tree needs. Finally, the last
>>> patch adds the device tree.
>>>
>>> Patch 2/3 may be slightly controversial as the new DT property could be
>>> better named. 
>>
>> Indeed :-). As a matter of fact I've already planned to introduce it as
>> "tmds-enable-gpios" [1], during my recent FRL related work.
>>
>>> It has been supported in the driver for a year now, so
>>> there could be users in the wild. I have therefore decided to keep the
>>> current name. I am open to alternative solutions.
>>
>> As mentioned in [2], this was "inherited" from downstream and hasn't been
>> really in use so far.  I intended to do some more testing before attempting
>> to send those patches upstream, as for now I couldn't notice any (obvious)
>> change in behavior when switching between TMDS and FRL, with or without
>> making use of it in DT.
> 
> Thanks for the links. The new property name is better. We may need to
> preserve backward compatibility though, how confident are you that the
> enabled-gpios property isn't used in the wild ?

I think we should be fine, also considering this doesn't seem to have a
negative impact on the functionality provided so far.

> I'd like to get the device tree for the Orange Pi CM5 merged sooner than
> later. Would you post those two patches, or should I take them in the v2
> of my series ?

Sure, feel free to take them if you wish to speed things up a bit.

>> [1] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/5a32a84ee3e801daaeb974f967f6e213983feeb4
>> [2] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/df4d94fdb94463dd4175338bf7044b40c5e559e4