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

[PATCH v2 0/5] 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 with three patches that add a new tmds-enable-gpios
property to the rk3588-dw-hdmi-qp DT binding (1/5) and update the driver
accordingly (2/5 and 3/5). Those patches have been authored by Cristian
Ciocaltea as part of a larger series, I have incorporated them here
as-is.

Patch 4/5 then add a new compatible for the board to arm/rockchip.yaml.
The last patch (5/5) adds the device tree for the board.

Patches 2/5 and 3/5 could be merged separately through the DRM tree,
but patch 1/5 needs to be merged with the device tree in 5/5 to avoid
introducing DT validation warnings. I'd appreciate advice from the DT
maintainers regarding how to handle this, as I have been told before
that DT bindings and DT sources can't be merged through the same tree.

Cristian Ciocaltea (3):
  dt-bindings: display: rk3588-dw-hdmi-qp: Add tmds-enable-gpios
    property
  drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()
  drm/rockchip: dw_hdmi_qp: Fixup usage of enable_gpio member in main
    struct

Laurent Pinchart (2):
  dt-bindings: arm: rockchip: Add Orange Pi CM5 Base
  arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

 .../devicetree/bindings/arm/rockchip.yaml     |   6 +
 .../rockchip/rockchip,rk3588-dw-hdmi-qp.yaml  |  10 +
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../rockchip/rk3588s-orangepi-cm5-base.dts    | 346 +++++++++++++
 .../dts/rockchip/rk3588s-orangepi-cm5.dtsi    | 473 ++++++++++++++++++
 .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    |  70 ++-
 6 files changed, 864 insertions(+), 42 deletions(-)
 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: e5f0a698b34ed76002dc5cff3804a61c80233a7a
-- 
Regards,

Laurent Pinchart

[PATCH v2 1/5] dt-bindings: display: rk3588-dw-hdmi-qp: Add tmds-enable-gpios property

[Laurent Pinchart]

From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

Add an optional property to RK3588 HDMI TX Controller binding describing
a GPIO line to be asserted when operating in HDMI 1.4/2.0 TMDS mode and
deasserted for HDMI 2.1 FRL.

This is used to control an external level shifter for HDMI data lines.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml   | 10 ++++++++++
 1 file changed, 10 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..eec0d40c91ea 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
@@ -113,6 +113,13 @@ properties:
     description:
       Additional HDMI QP related data is accessed through VO GRF regs.
 
+  tmds-enable-gpios:
+    description:
+      Optional GPIO line to be asserted when operating in HDMI 1.4/2.0
+      TMDS mode and deasserted for HDMI 2.1 FRL. It can be used to control
+      an external level shifter for HDMI data lines.
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -132,8 +139,10 @@ unevaluatedProperties: false
 examples:
   - |
     #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+    #include <dt-bindings/gpio/gpio.h>
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/pinctrl/rockchip.h>
     #include <dt-bindings/power/rk3588-power.h>
     #include <dt-bindings/reset/rockchip,rk3588-cru.h>
 
@@ -164,6 +173,7 @@ examples:
         rockchip,grf = <&sys_grf>;
         rockchip,vo-grf = <&vo1_grf>;
         #sound-dai-cells = <0>;
+        tmds-enable-gpios = <&gpio4 RK_PB1 GPIO_ACTIVE_HIGH>;
 
         ports {
           #address-cells = <1>;
-- 
Regards,

Laurent Pinchart

[PATCH v2 2/5] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Laurent Pinchart]

From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
i.e. in some cases the error code is not included in the message, while
in some other cases there is no check for -EPROBE_DEFER.

Since this is part of the probe path, address the aforementioned issues
by switching to dev_err_probe(), which also reduces the code a bit.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    | 62 +++++++------------
 1 file changed, 24 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 7d531b6f4c09..4e7794aa2dde 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 		return -ENODEV;
 
 	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
-	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
-		dev_err(dev, "Missing platform ctrl ops\n");
-		return -ENODEV;
-	}
+	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
+		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
 
 	hdmi->ctrl_ops = cfg->ctrl_ops;
 	hdmi->dev = &pdev->dev;
@@ -473,10 +471,9 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 			break;
 		}
 	}
-	if (hdmi->port_id < 0) {
-		dev_err(hdmi->dev, "Failed to match HDMI port ID\n");
-		return hdmi->port_id;
-	}
+	if (hdmi->port_id < 0)
+		return dev_err_probe(hdmi->dev, hdmi->port_id,
+				     "Failed to match HDMI port ID\n");
 
 	plat_data.phy_ops = cfg->phy_ops;
 	plat_data.phy_data = hdmi;
@@ -497,39 +494,30 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 
 	hdmi->regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
 						       "rockchip,grf");
-	if (IS_ERR(hdmi->regmap)) {
-		dev_err(hdmi->dev, "Unable to get rockchip,grf\n");
-		return PTR_ERR(hdmi->regmap);
-	}
+	if (IS_ERR(hdmi->regmap))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->regmap),
+				     "Unable to get rockchip,grf\n");
 
 	hdmi->vo_regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
 							  "rockchip,vo-grf");
-	if (IS_ERR(hdmi->vo_regmap)) {
-		dev_err(hdmi->dev, "Unable to get rockchip,vo-grf\n");
-		return PTR_ERR(hdmi->vo_regmap);
-	}
+	if (IS_ERR(hdmi->vo_regmap))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->vo_regmap),
+				     "Unable to get rockchip,vo-grf\n");
 
 	ret = devm_clk_bulk_get_all_enabled(hdmi->dev, &clks);
-	if (ret < 0) {
-		dev_err(hdmi->dev, "Failed to get clocks: %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(hdmi->dev, ret, "Failed to get clocks\n");
 
 	hdmi->enable_gpio = devm_gpiod_get_optional(hdmi->dev, "enable",
 						    GPIOD_OUT_HIGH);
-	if (IS_ERR(hdmi->enable_gpio)) {
-		ret = PTR_ERR(hdmi->enable_gpio);
-		dev_err(hdmi->dev, "Failed to request enable GPIO: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(hdmi->enable_gpio))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->enable_gpio),
+				     "Failed to request enable GPIO\n");
 
 	hdmi->phy = devm_of_phy_get_by_index(dev, dev->of_node, 0);
-	if (IS_ERR(hdmi->phy)) {
-		ret = PTR_ERR(hdmi->phy);
-		if (ret != -EPROBE_DEFER)
-			dev_err(hdmi->dev, "failed to get phy: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(hdmi->phy))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->phy),
+				     "Failed to get phy\n");
 
 	cfg->ctrl_ops->io_init(hdmi);
 
@@ -558,17 +546,15 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 
 	hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);
 	if (IS_ERR(hdmi->hdmi)) {
-		ret = PTR_ERR(hdmi->hdmi);
 		drm_encoder_cleanup(encoder);
-		return ret;
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),
+				     "Failed to bind dw-hdmi-qp");
 	}
 
 	connector = drm_bridge_connector_init(drm, encoder);
-	if (IS_ERR(connector)) {
-		ret = PTR_ERR(connector);
-		dev_err(hdmi->dev, "failed to init bridge connector: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(connector))
+		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
+				     "Failed to init bridge connector\n");
 
 	return drm_connector_attach_encoder(connector, encoder);
 }
-- 
Regards,

Laurent Pinchart

[PATCH v2 3/5] drm/rockchip: dw_hdmi_qp: Fixup usage of enable_gpio member in main struct

[Laurent Pinchart]

From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

The name of the enable_gpio member in struct rockchip_hdmi_qp is too
generic, as initially "borrowed" from downstream BSP code.  Moreover,
this hasn't been really in use so far, since there is neither a DT
providing an "enable-gpios" property to any of the "hdmi" nodes, nor a
binding documenting it.

The actual purpose of this GPIO line is to control the level shifters
for the HDMI TX data lines when switching between TMDS and FRL operating
modes, i.e. asserted for the former and deasserted for the latter.

A previous patch introduced the "tmds-enable-gpios" property of the
RK3588 HDMI TX Controller binding, hence pass the updated string to
devm_gpiod_get_optional() and rename the struct member accordingly.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 4e7794aa2dde..329a69a548bd 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -91,7 +91,7 @@ struct rockchip_hdmi_qp {
 	struct rockchip_encoder encoder;
 	struct dw_hdmi_qp *hdmi;
 	struct phy *phy;
-	struct gpio_desc *enable_gpio;
+	struct gpio_desc *tmds_enable_gpio;
 	struct delayed_work hpd_work;
 	int port_id;
 	const struct rockchip_hdmi_qp_ctrl_ops *ctrl_ops;
@@ -117,7 +117,7 @@ static void dw_hdmi_qp_rockchip_encoder_enable(struct drm_encoder *encoder)
 	unsigned long long rate;
 
 	/* Unconditionally switch to TMDS as FRL is not yet supported */
-	gpiod_set_value(hdmi->enable_gpio, 1);
+	gpiod_set_value(hdmi->tmds_enable_gpio, 1);
 
 	if (crtc && crtc->state) {
 		rate = drm_hdmi_compute_mode_clock(&crtc->state->adjusted_mode,
@@ -508,11 +508,11 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	if (ret < 0)
 		return dev_err_probe(hdmi->dev, ret, "Failed to get clocks\n");
 
-	hdmi->enable_gpio = devm_gpiod_get_optional(hdmi->dev, "enable",
-						    GPIOD_OUT_HIGH);
-	if (IS_ERR(hdmi->enable_gpio))
-		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->enable_gpio),
-				     "Failed to request enable GPIO\n");
+	hdmi->tmds_enable_gpio = devm_gpiod_get_optional(hdmi->dev, "tmds-enable",
+							 GPIOD_OUT_HIGH);
+	if (IS_ERR(hdmi->tmds_enable_gpio))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->tmds_enable_gpio),
+				     "Failed to request TMDS enable GPIO\n");
 
 	hdmi->phy = devm_of_phy_get_by_index(dev, dev->of_node, 0);
 	if (IS_ERR(hdmi->phy))
-- 
Regards,

Laurent Pinchart

[PATCH v2 4/5] 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>
Acked-by: Conor Dooley <conor.dooley@microchip.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

[PATCH v2 5/5] 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>
---
Changes since v1:

- Rename enable-gpios to tmds-enable-gpios
- Add pinctrl entries for camera reset pins
- Move gpu and pd_gpu enablement to CM5
- Move mmc0 alias to CM5
- Drop tx_delay from gmac1
- Reorder properties in carrier board regulators
- Set tsadc pinctrl to gpio
---
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../rockchip/rk3588s-orangepi-cm5-base.dts    | 346 +++++++++++++
 .../dts/rockchip/rk3588s-orangepi-cm5.dtsi    | 473 ++++++++++++++++++
 3 files changed, 820 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 099520962ffb..824922e27e43 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -195,6 +195,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..105814680c2e
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5-base.dts
@@ -0,0 +1,346 @@
+// 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;
+		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>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vbus_5v0_en_pin>;
+		vin-supply = <&vcc5v0_sys>;
+	};
+
+	vcc_3v3: regulator-vcc-3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		enable-active-high;
+		gpio = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vcc_3v3_en_pin>;
+		startup-delay-us = <50000>;
+		vin-supply = <&vcc5v0_sys>;
+	};
+
+	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>;
+	status = "okay";
+};
+
+&hdmi0 {
+	tmds-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";
+};
+
+&pinctrl {
+	camera {
+		cam1_reset_pin: cam1-reset-pin {
+			rockchip,pins = <1 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		cam2_reset_pin: cam2-reset-pin {
+			rockchip,pins = <1 RK_PD1 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		cam3_reset_pin: cam3-reset-pin {
+			rockchip,pins = <1 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		cam4_reset_pin: cam4-reset-pin {
+			rockchip,pins = <1 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	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..616a4674cbfe
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-cm5.dtsi
@@ -0,0 +1,473 @@
+// 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"
+
+/ {
+	aliases {
+		mmc0 = &sdhci;
+	};
+
+	/* 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>;
+};
+
+&gpu {
+	mali-supply = <&vdd_gpu_s0>;
+	status = "okay";
+};
+
+&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;
+		};
+	};
+};
+
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
+&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 {
+	/*
+	 * The TSADC_SHUT pin is exposed to carrier boards as a signal named
+	 * PMIC_RESET_L, meant to be driven externally. Reference carrier
+	 * boards connect it to a reset button that pulls the signal to GND
+	 * through a 100Ω resistor. This is too weak to overcome even the
+	 * minimum drive strength of the TSADC_SHUT pin when driven in
+	 * push-pull mode. Configure it as a GPIO, reset will be generated
+	 * through the CRU.
+	 */
+	pinctrl-0 = <&tsadc_gpio_func>;
+	status = "okay";
+};
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/5] dt-bindings: display: rk3588-dw-hdmi-qp: Add tmds-enable-gpios property

[Dmitry Baryshkov]

On Mon, Oct 06, 2025 at 02:55:37AM +0300, Laurent Pinchart wrote:
> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> 
> Add an optional property to RK3588 HDMI TX Controller binding describing
> a GPIO line to be asserted when operating in HDMI 1.4/2.0 TMDS mode and
> deasserted for HDMI 2.1 FRL.
> 
> This is used to control an external level shifter for HDMI data lines.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml   | 10 ++++++++++
>  1 file changed, 10 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..eec0d40c91ea 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
> @@ -113,6 +113,13 @@ properties:
>      description:
>        Additional HDMI QP related data is accessed through VO GRF regs.
>  
> +  tmds-enable-gpios:
> +    description:
> +      Optional GPIO line to be asserted when operating in HDMI 1.4/2.0
> +      TMDS mode and deasserted for HDMI 2.1 FRL. It can be used to control
> +      an external level shifter for HDMI data lines.
> +    maxItems: 1

Just my 2c: since TMDS is assumed to be a default mode, shouldn't this
be a frl-enable-gpios instead?

> +
>  required:
>    - compatible
>    - reg

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/5] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Dmitry Baryshkov]

On Mon, Oct 06, 2025 at 02:55:38AM +0300, Laurent Pinchart wrote:
> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> 
> The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
> i.e. in some cases the error code is not included in the message, while
> in some other cases there is no check for -EPROBE_DEFER.
> 
> Since this is part of the probe path, address the aforementioned issues
> by switching to dev_err_probe(), which also reduces the code a bit.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    | 62 +++++++------------
>  1 file changed, 24 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 7d531b6f4c09..4e7794aa2dde 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  		return -ENODEV;
>  
>  	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
> -	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
> -		dev_err(dev, "Missing platform ctrl ops\n");
> -		return -ENODEV;
> -	}
> +	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
> +		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");

This only makes sense for the purpose of unification.

>  
>  	hdmi->ctrl_ops = cfg->ctrl_ops;
>  	hdmi->dev = &pdev->dev;
> @@ -473,10 +471,9 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  			break;
>  		}
>  	}
> -	if (hdmi->port_id < 0) {
> -		dev_err(hdmi->dev, "Failed to match HDMI port ID\n");
> -		return hdmi->port_id;
> -	}
> +	if (hdmi->port_id < 0)
> +		return dev_err_probe(hdmi->dev, hdmi->port_id,
> +				     "Failed to match HDMI port ID\n");

The port_id can't become -EPROBE_DEFER, so it also unnecessary.

>  
>  	plat_data.phy_ops = cfg->phy_ops;
>  	plat_data.phy_data = hdmi;
> @@ -497,39 +494,30 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  
>  	hdmi->regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
>  						       "rockchip,grf");
> -	if (IS_ERR(hdmi->regmap)) {
> -		dev_err(hdmi->dev, "Unable to get rockchip,grf\n");
> -		return PTR_ERR(hdmi->regmap);
> -	}
> +	if (IS_ERR(hdmi->regmap))
> +		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->regmap),
> +				     "Unable to get rockchip,grf\n");
>  
>  	hdmi->vo_regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
>  							  "rockchip,vo-grf");
> -	if (IS_ERR(hdmi->vo_regmap)) {
> -		dev_err(hdmi->dev, "Unable to get rockchip,vo-grf\n");
> -		return PTR_ERR(hdmi->vo_regmap);
> -	}
> +	if (IS_ERR(hdmi->vo_regmap))
> +		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->vo_regmap),
> +				     "Unable to get rockchip,vo-grf\n");
>  
>  	ret = devm_clk_bulk_get_all_enabled(hdmi->dev, &clks);
> -	if (ret < 0) {
> -		dev_err(hdmi->dev, "Failed to get clocks: %d\n", ret);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		return dev_err_probe(hdmi->dev, ret, "Failed to get clocks\n");
>  
>  	hdmi->enable_gpio = devm_gpiod_get_optional(hdmi->dev, "enable",
>  						    GPIOD_OUT_HIGH);
> -	if (IS_ERR(hdmi->enable_gpio)) {
> -		ret = PTR_ERR(hdmi->enable_gpio);
> -		dev_err(hdmi->dev, "Failed to request enable GPIO: %d\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(hdmi->enable_gpio))
> +		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->enable_gpio),
> +				     "Failed to request enable GPIO\n");
>  
>  	hdmi->phy = devm_of_phy_get_by_index(dev, dev->of_node, 0);
> -	if (IS_ERR(hdmi->phy)) {
> -		ret = PTR_ERR(hdmi->phy);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(hdmi->dev, "failed to get phy: %d\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(hdmi->phy))
> +		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->phy),
> +				     "Failed to get phy\n");
>  
>  	cfg->ctrl_ops->io_init(hdmi);
>  
> @@ -558,17 +546,15 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  
>  	hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);
>  	if (IS_ERR(hdmi->hdmi)) {
> -		ret = PTR_ERR(hdmi->hdmi);
>  		drm_encoder_cleanup(encoder);
> -		return ret;
> +		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),
> +				     "Failed to bind dw-hdmi-qp");
>  	}
>  
>  	connector = drm_bridge_connector_init(drm, encoder);

This one is also not supposed to return -EPROBE_DEFER.

> -	if (IS_ERR(connector)) {
> -		ret = PTR_ERR(connector);
> -		dev_err(hdmi->dev, "failed to init bridge connector: %d\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(connector))
> +		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
> +				     "Failed to init bridge connector\n");
>  
>  	return drm_connector_attach_encoder(connector, encoder);
>  }
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/5] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Cristian Ciocaltea]

Hi Dmitry,

On 10/6/25 3:02 PM, Dmitry Baryshkov wrote:
> On Mon, Oct 06, 2025 at 02:55:38AM +0300, Laurent Pinchart wrote:
>> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>
>> The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
>> i.e. in some cases the error code is not included in the message, while
>> in some other cases there is no check for -EPROBE_DEFER.
>>
>> Since this is part of the probe path, address the aforementioned issues
>> by switching to dev_err_probe(), which also reduces the code a bit.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>  .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    | 62 +++++++------------
>>  1 file changed, 24 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> index 7d531b6f4c09..4e7794aa2dde 100644
>> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> @@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>>  		return -ENODEV;
>>  
>>  	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
>> -	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
>> -		dev_err(dev, "Missing platform ctrl ops\n");
>> -		return -ENODEV;
>> -	}
>> +	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
>> +		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
> 
> This only makes sense for the purpose of unification.

Right, as mentioned in the commit description, the intention was to ensure
consistent error handling across the probe path rather than limiting the scope
to -EPROBE_DEFER exclusively.

Thanks,
Cristian

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

[Andrew Lunn]

> Changes since v1:

...

> - Drop tx_delay from gmac1

Thanks, that seems much more logical.

	Andrew

Re: [PATCH v2 1/5] dt-bindings: display: rk3588-dw-hdmi-qp: Add tmds-enable-gpios property

[Laurent Pinchart]

On Mon, Oct 06, 2025 at 02:19:24PM +0300, Dmitry Baryshkov wrote:
> On Mon, Oct 06, 2025 at 02:55:37AM +0300, Laurent Pinchart wrote:
> > From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > 
> > Add an optional property to RK3588 HDMI TX Controller binding describing
> > a GPIO line to be asserted when operating in HDMI 1.4/2.0 TMDS mode and
> > deasserted for HDMI 2.1 FRL.
> > 
> > This is used to control an external level shifter for HDMI data lines.
> > 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml   | 10 ++++++++++
> >  1 file changed, 10 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..eec0d40c91ea 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
> > @@ -113,6 +113,13 @@ properties:
> >      description:
> >        Additional HDMI QP related data is accessed through VO GRF regs.
> >  
> > +  tmds-enable-gpios:
> > +    description:
> > +      Optional GPIO line to be asserted when operating in HDMI 1.4/2.0
> > +      TMDS mode and deasserted for HDMI 2.1 FRL. It can be used to control
> > +      an external level shifter for HDMI data lines.
> > +    maxItems: 1
> 
> Just my 2c: since TMDS is assumed to be a default mode, shouldn't this
> be a frl-enable-gpios instead?

I don't mind either way. Cristian, any opinion ?

> > +
> >  required:
> >    - compatible
> >    - reg

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/5] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Laurent Pinchart]

On Mon, Oct 06, 2025 at 05:37:23PM +0300, Cristian Ciocaltea wrote:
> On 10/6/25 3:02 PM, Dmitry Baryshkov wrote:
> > On Mon, Oct 06, 2025 at 02:55:38AM +0300, Laurent Pinchart wrote:
> >> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >>
> >> The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
> >> i.e. in some cases the error code is not included in the message, while
> >> in some other cases there is no check for -EPROBE_DEFER.
> >>
> >> Since this is part of the probe path, address the aforementioned issues
> >> by switching to dev_err_probe(), which also reduces the code a bit.
> >>
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> ---
> >>  .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    | 62 +++++++------------
> >>  1 file changed, 24 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> >> index 7d531b6f4c09..4e7794aa2dde 100644
> >> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> >> @@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
> >>  		return -ENODEV;
> >>  
> >>  	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
> >> -	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
> >> -		dev_err(dev, "Missing platform ctrl ops\n");
> >> -		return -ENODEV;
> >> -	}
> >> +	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
> >> +		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
> > 
> > This only makes sense for the purpose of unification.
> 
> Right, as mentioned in the commit description, the intention was to ensure
> consistent error handling across the probe path rather than limiting the scope
> to -EPROBE_DEFER exclusively.

Should I revert this change in v3 or keep it ? I see value in
unification, but I don't mind either way. Dmitry, what's your preference
?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/5] dt-bindings: display: rk3588-dw-hdmi-qp: Add tmds-enable-gpios property

[Cristian Ciocaltea]

On 10/6/25 7:25 PM, Laurent Pinchart wrote:
> On Mon, Oct 06, 2025 at 02:19:24PM +0300, Dmitry Baryshkov wrote:
>> On Mon, Oct 06, 2025 at 02:55:37AM +0300, Laurent Pinchart wrote:
>>> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>
>>> Add an optional property to RK3588 HDMI TX Controller binding describing
>>> a GPIO line to be asserted when operating in HDMI 1.4/2.0 TMDS mode and
>>> deasserted for HDMI 2.1 FRL.
>>>
>>> This is used to control an external level shifter for HDMI data lines.
>>>
>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>>  .../display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml   | 10 ++++++++++
>>>  1 file changed, 10 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..eec0d40c91ea 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
>>> @@ -113,6 +113,13 @@ properties:
>>>      description:
>>>        Additional HDMI QP related data is accessed through VO GRF regs.
>>>  
>>> +  tmds-enable-gpios:
>>> +    description:
>>> +      Optional GPIO line to be asserted when operating in HDMI 1.4/2.0
>>> +      TMDS mode and deasserted for HDMI 2.1 FRL. It can be used to control
>>> +      an external level shifter for HDMI data lines.
>>> +    maxItems: 1
>>
>> Just my 2c: since TMDS is assumed to be a default mode, shouldn't this
>> be a frl-enable-gpios instead?
> 
> I don't mind either way. Cristian, any opinion ?

No strong opinion either; we could go for Dmitry's suggestion.

>>> +
>>>  required:
>>>    - compatible
>>>    - reg
> 

Re: [PATCH v2 1/5] dt-bindings: display: rk3588-dw-hdmi-qp: Add tmds-enable-gpios property

[Laurent Pinchart]

On Mon, Oct 06, 2025 at 07:42:56PM +0300, Cristian Ciocaltea wrote:
> On 10/6/25 7:25 PM, Laurent Pinchart wrote:
> > On Mon, Oct 06, 2025 at 02:19:24PM +0300, Dmitry Baryshkov wrote:
> >> On Mon, Oct 06, 2025 at 02:55:37AM +0300, Laurent Pinchart wrote:
> >>> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >>>
> >>> Add an optional property to RK3588 HDMI TX Controller binding describing
> >>> a GPIO line to be asserted when operating in HDMI 1.4/2.0 TMDS mode and
> >>> deasserted for HDMI 2.1 FRL.
> >>>
> >>> This is used to control an external level shifter for HDMI data lines.
> >>>
> >>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>> ---
> >>>  .../display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml   | 10 ++++++++++
> >>>  1 file changed, 10 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..eec0d40c91ea 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
> >>> @@ -113,6 +113,13 @@ properties:
> >>>      description:
> >>>        Additional HDMI QP related data is accessed through VO GRF regs.
> >>>  
> >>> +  tmds-enable-gpios:
> >>> +    description:
> >>> +      Optional GPIO line to be asserted when operating in HDMI 1.4/2.0
> >>> +      TMDS mode and deasserted for HDMI 2.1 FRL. It can be used to control
> >>> +      an external level shifter for HDMI data lines.
> >>> +    maxItems: 1
> >>
> >> Just my 2c: since TMDS is assumed to be a default mode, shouldn't this
> >> be a frl-enable-gpios instead?
> > 
> > I don't mind either way. Cristian, any opinion ?
> 
> No strong opinion either; we could go for Dmitry's suggestion.

OK, I'll flip it.

> >>> +
> >>>  required:
> >>>    - compatible
> >>>    - reg

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/5] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Cristian Ciocaltea]

On 10/6/25 7:26 PM, Laurent Pinchart wrote:
> On Mon, Oct 06, 2025 at 05:37:23PM +0300, Cristian Ciocaltea wrote:
>> On 10/6/25 3:02 PM, Dmitry Baryshkov wrote:
>>> On Mon, Oct 06, 2025 at 02:55:38AM +0300, Laurent Pinchart wrote:
>>>> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>>
>>>> The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
>>>> i.e. in some cases the error code is not included in the message, while
>>>> in some other cases there is no check for -EPROBE_DEFER.
>>>>
>>>> Since this is part of the probe path, address the aforementioned issues
>>>> by switching to dev_err_probe(), which also reduces the code a bit.
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> ---
>>>>  .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    | 62 +++++++------------
>>>>  1 file changed, 24 insertions(+), 38 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>> index 7d531b6f4c09..4e7794aa2dde 100644
>>>> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>> @@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>>>>  		return -ENODEV;
>>>>  
>>>>  	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
>>>> -	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
>>>> -		dev_err(dev, "Missing platform ctrl ops\n");
>>>> -		return -ENODEV;
>>>> -	}
>>>> +	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
>>>> +		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
>>>
>>> This only makes sense for the purpose of unification.
>>
>> Right, as mentioned in the commit description, the intention was to ensure
>> consistent error handling across the probe path rather than limiting the scope
>> to -EPROBE_DEFER exclusively.
> 
> Should I revert this change in v3 or keep it ? I see value in
> unification, but I don't mind either way. Dmitry, what's your preference
> ?

I missed to point out this patch has been also sent a while ago as part of
another series [1] which should be ready for merging.  It'd be great if there's
no need to revert any changes, otherwise we need to keep those in sync.

Regardless, I'll let you know if that gets applied first, allowing us to drop
this one after rebasing.

[1] https://lore.kernel.org/all/20250903-rk3588-hdmi-cec-v4-3-fa25163c4b08@collabora.com/

Regards,
Cristian

Re: [PATCH v2 2/5] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Laurent Pinchart]

On Thu, Oct 09, 2025 at 10:24:25AM +0300, Cristian Ciocaltea wrote:
> On 10/6/25 7:26 PM, Laurent Pinchart wrote:
> > On Mon, Oct 06, 2025 at 05:37:23PM +0300, Cristian Ciocaltea wrote:
> >> On 10/6/25 3:02 PM, Dmitry Baryshkov wrote:
> >>> On Mon, Oct 06, 2025 at 02:55:38AM +0300, Laurent Pinchart wrote:
> >>>> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >>>>
> >>>> The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
> >>>> i.e. in some cases the error code is not included in the message, while
> >>>> in some other cases there is no check for -EPROBE_DEFER.
> >>>>
> >>>> Since this is part of the probe path, address the aforementioned issues
> >>>> by switching to dev_err_probe(), which also reduces the code a bit.
> >>>>
> >>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> ---
> >>>>  .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    | 62 +++++++------------
> >>>>  1 file changed, 24 insertions(+), 38 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> >>>> index 7d531b6f4c09..4e7794aa2dde 100644
> >>>> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> >>>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> >>>> @@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
> >>>>  		return -ENODEV;
> >>>>  
> >>>>  	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
> >>>> -	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
> >>>> -		dev_err(dev, "Missing platform ctrl ops\n");
> >>>> -		return -ENODEV;
> >>>> -	}
> >>>> +	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
> >>>> +		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
> >>>
> >>> This only makes sense for the purpose of unification.
> >>
> >> Right, as mentioned in the commit description, the intention was to ensure
> >> consistent error handling across the probe path rather than limiting the scope
> >> to -EPROBE_DEFER exclusively.
> > 
> > Should I revert this change in v3 or keep it ? I see value in
> > unification, but I don't mind either way. Dmitry, what's your preference
> > ?
> 
> I missed to point out this patch has been also sent a while ago as part of
> another series [1] which should be ready for merging.  It'd be great if there's
> no need to revert any changes, otherwise we need to keep those in sync.
> 
> Regardless, I'll let you know if that gets applied first, allowing us to drop
> this one after rebasing.

Thanks for the information. I'm happy to merge 3/5 on top of that
series. The DT binding change in 1/5 can be merged separately.

> [1] https://lore.kernel.org/all/20250903-rk3588-hdmi-cec-v4-3-fa25163c4b08@collabora.com/

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/5] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Cristian Ciocaltea]

On 10/9/25 12:41 PM, Laurent Pinchart wrote:
> On Thu, Oct 09, 2025 at 10:24:25AM +0300, Cristian Ciocaltea wrote:
>> On 10/6/25 7:26 PM, Laurent Pinchart wrote:
>>> On Mon, Oct 06, 2025 at 05:37:23PM +0300, Cristian Ciocaltea wrote:
>>>> On 10/6/25 3:02 PM, Dmitry Baryshkov wrote:
>>>>> On Mon, Oct 06, 2025 at 02:55:38AM +0300, Laurent Pinchart wrote:
>>>>>> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>>>>
>>>>>> The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
>>>>>> i.e. in some cases the error code is not included in the message, while
>>>>>> in some other cases there is no check for -EPROBE_DEFER.
>>>>>>
>>>>>> Since this is part of the probe path, address the aforementioned issues
>>>>>> by switching to dev_err_probe(), which also reduces the code a bit.
>>>>>>
>>>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>> ---
>>>>>>  .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    | 62 +++++++------------
>>>>>>  1 file changed, 24 insertions(+), 38 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>>>> index 7d531b6f4c09..4e7794aa2dde 100644
>>>>>> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>>>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>>>> @@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>>>>>>  		return -ENODEV;
>>>>>>  
>>>>>>  	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
>>>>>> -	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
>>>>>> -		dev_err(dev, "Missing platform ctrl ops\n");
>>>>>> -		return -ENODEV;
>>>>>> -	}
>>>>>> +	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
>>>>>> +		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
>>>>>
>>>>> This only makes sense for the purpose of unification.
>>>>
>>>> Right, as mentioned in the commit description, the intention was to ensure
>>>> consistent error handling across the probe path rather than limiting the scope
>>>> to -EPROBE_DEFER exclusively.
>>>
>>> Should I revert this change in v3 or keep it ? I see value in
>>> unification, but I don't mind either way. Dmitry, what's your preference
>>> ?
>>
>> I missed to point out this patch has been also sent a while ago as part of
>> another series [1] which should be ready for merging.  It'd be great if there's
>> no need to revert any changes, otherwise we need to keep those in sync.
>>
>> Regardless, I'll let you know if that gets applied first, allowing us to drop
>> this one after rebasing.
> 
> Thanks for the information. I'm happy to merge 3/5 on top of that
> series. The DT binding change in 1/5 can be merged separately.

That series has been applied onto drm-misc-next.

>> [1] https://lore.kernel.org/all/20250903-rk3588-hdmi-cec-v4-3-fa25163c4b08@collabora.com/
> 

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

[Heiko Stuebner]

Hi Laurent,

Am Montag, 6. Oktober 2025, 01:55:36 Mitteleuropäische Sommerzeit schrieb 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 with three patches that add a new tmds-enable-gpios
> property to the rk3588-dw-hdmi-qp DT binding (1/5) and update the driver
> accordingly (2/5 and 3/5). Those patches have been authored by Cristian
> Ciocaltea as part of a larger series, I have incorporated them here
> as-is.
> 
> Patch 4/5 then add a new compatible for the board to arm/rockchip.yaml.
> The last patch (5/5) adds the device tree for the board.
> 
> Patches 2/5 and 3/5 could be merged separately through the DRM tree,
> but patch 1/5 needs to be merged with the device tree in 5/5 to avoid
> introducing DT validation warnings. I'd appreciate advice from the DT
> maintainers regarding how to handle this, as I have been told before
> that DT bindings and DT sources can't be merged through the same tree.

I think we generally only care about binding warnings happening
in linux-next.

I.e. in most cases, the binding is merged with the driver and
the dts then into a separate tree - sort of ignoring the local
binding error, because everything will be fine once stuff comes
together in linux-next.


Speaking of bindings, does your board _need_ the frl-gpio to produce
any hdmi output? Like could we merge the board without the (optional)
gpio and then add it later, once the binding+driver have made it in?

Just me trying to grab the big chunks earlier in the cycle.


Thanks
Heiko

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

[Laurent Pinchart]

On Mon, Oct 20, 2025 at 04:44:21PM +0200, Heiko Stuebner wrote:
> Am Montag, 6. Oktober 2025, 01:55:36 Mitteleuropäische Sommerzeit schrieb 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 with three patches that add a new tmds-enable-gpios
> > property to the rk3588-dw-hdmi-qp DT binding (1/5) and update the driver
> > accordingly (2/5 and 3/5). Those patches have been authored by Cristian
> > Ciocaltea as part of a larger series, I have incorporated them here
> > as-is.
> > 
> > Patch 4/5 then add a new compatible for the board to arm/rockchip.yaml.
> > The last patch (5/5) adds the device tree for the board.
> > 
> > Patches 2/5 and 3/5 could be merged separately through the DRM tree,
> > but patch 1/5 needs to be merged with the device tree in 5/5 to avoid
> > introducing DT validation warnings. I'd appreciate advice from the DT
> > maintainers regarding how to handle this, as I have been told before
> > that DT bindings and DT sources can't be merged through the same tree.
> 
> I think we generally only care about binding warnings happening
> in linux-next.
> 
> I.e. in most cases, the binding is merged with the driver and
> the dts then into a separate tree - sort of ignoring the local
> binding error, because everything will be fine once stuff comes
> together in linux-next.

Assuming they get merged together in linux-next. If there's a delay,
linux-next will exhibit warnings for some time. I don't know what the
rule is regarding that.

> Speaking of bindings, does your board _need_ the frl-gpio to produce
> any hdmi output? Like could we merge the board without the (optional)
> gpio and then add it later, once the binding+driver have made it in?

With a suitable pull-up I think so. I can submit a v3 with that.

> Just me trying to grab the big chunks earlier in the cycle.

-- 
Regards,

Laurent Pinchart

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

[Heiko Stuebner]

Am Montag, 20. Oktober 2025, 17:46:25 Mitteleuropäische Sommerzeit schrieb Laurent Pinchart:
> On Mon, Oct 20, 2025 at 04:44:21PM +0200, Heiko Stuebner wrote:
> > Am Montag, 6. Oktober 2025, 01:55:36 Mitteleuropäische Sommerzeit schrieb 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 with three patches that add a new tmds-enable-gpios
> > > property to the rk3588-dw-hdmi-qp DT binding (1/5) and update the driver
> > > accordingly (2/5 and 3/5). Those patches have been authored by Cristian
> > > Ciocaltea as part of a larger series, I have incorporated them here
> > > as-is.
> > > 
> > > Patch 4/5 then add a new compatible for the board to arm/rockchip.yaml.
> > > The last patch (5/5) adds the device tree for the board.
> > > 
> > > Patches 2/5 and 3/5 could be merged separately through the DRM tree,
> > > but patch 1/5 needs to be merged with the device tree in 5/5 to avoid
> > > introducing DT validation warnings. I'd appreciate advice from the DT
> > > maintainers regarding how to handle this, as I have been told before
> > > that DT bindings and DT sources can't be merged through the same tree.
> > 
> > I think we generally only care about binding warnings happening
> > in linux-next.
> > 
> > I.e. in most cases, the binding is merged with the driver and
> > the dts then into a separate tree - sort of ignoring the local
> > binding error, because everything will be fine once stuff comes
> > together in linux-next.
> 
> Assuming they get merged together in linux-next. If there's a delay,
> linux-next will exhibit warnings for some time. I don't know what the
> rule is regarding that.

Normally once the driver-maintainer has applied binding + driver
change, I also pick up the devicetree shortly change after that.

And in most cases the driver change will be in -next the next day,
and the dts change as well or 1-2 days later.

If a CI bot complains, one then goes looking if something went wrong :-) .
But in 99.9% of cases, things just work out.


> > Speaking of bindings, does your board _need_ the frl-gpio to produce
> > any hdmi output? Like could we merge the board without the (optional)
> > gpio and then add it later, once the binding+driver have made it in?
> 
> With a suitable pull-up I think so. I can submit a v3 with that.

I guess that might be a nice way to go, as then the main board dts
doesn't has to wait for the drm changes to be finalized :-)


Heiko