Re: [PATCH v2 2/3] arm64: dts: rockchip: Use reference PCIe clock generator for BPI-R2-Pro

On 03/03/2026 18:38, Charalampos Mitrodimas wrote:

David Heidelberg via B4 Relay [off-list ref] writes:

quoted

From: David Heidelberg <david@ixit.cz>

Describe properly PCIe clock, which allows us correct the
toplogy (removing the vcc3v3-{minipcie,ngff} dependency on pi6c as
supply) and adding the clock dependency in the PCIe nodes.

Suggested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Martin Filla <redacted>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
  arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts | 45 +++++++++++++++-------
  1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
index d02b82c5f979a..a071cb67579c4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts

@@ -120,18 +120,13 @@ pcie_refclk_gen: pcie-refclk-gen-clock {
  		clock-frequency = <100000000>;
  	};
  
-	vcc3v3_pi6c_05: regulator-vcc3v3-pi6c-05 {
-		compatible = "regulator-fixed";
-		regulator-name = "vcc3v3_pcie";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-		enable-active-high;
-		gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
-		startup-delay-us = <200000>;
-		vin-supply = <&vcc5v0_sys>;
+	pcie_refclk: pcie-refclk-clock {
+		compatible = "gpio-gate-clock";
+		clocks = <&pcie_refclk_gen>;
+		#clock-cells = <0>;
+		enable-gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
  	};
  
-	/* actually fed by vcc3v3_sys, dependent on pi6c clock generator */
  	vcc3v3_minipcie: regulator-vcc3v3-minipcie {
  		compatible = "regulator-fixed";
  		regulator-name = "vcc3v3_minipcie";

@@ -142,10 +137,9 @@ vcc3v3_minipcie: regulator-vcc3v3-minipcie {
  		pinctrl-names = "default";
  		pinctrl-0 = <&minipcie_enable_h>;
  		startup-delay-us = <50000>;
-		vin-supply = <&vcc3v3_pi6c_05>;
+		vin-supply = <&vcc3v3_sys>;
  	};
  
-	/* actually fed by vcc3v3_sys, dependent on pi6c clock generator */
  	vcc3v3_ngff: regulator-vcc3v3-ngff {
  		compatible = "regulator-fixed";
  		regulator-name = "vcc3v3_ngff";

@@ -156,7 +150,7 @@ vcc3v3_ngff: regulator-vcc3v3-ngff {
  		pinctrl-names = "default";
  		pinctrl-0 = <&ngffpcie_enable_h>;
  		startup-delay-us = <50000>;
-		vin-supply = <&vcc3v3_pi6c_05>;
+		vin-supply = <&vcc3v3_sys>;
  	};
  
  	vcc5v0_usb: regulator-vcc5v0-usb {

@@ -586,12 +580,23 @@ rgmii_phy1: ethernet-phy@0 {
  
  &pcie30phy {
  	data-lanes = <1 2>;
-	phy-supply = <&vcc3v3_pi6c_05>;
+
  	status = "okay";
  };
  
  &pcie3x1 {
  	/* M.2 slot */
+	/*
+	 * The board has a gpio-controlled "pcie_refclk" generator,
+	 * so add it to the list of clocks.
+	 */
+	clocks = <&cru ACLK_PCIE30X1_MST>, <&cru ACLK_PCIE30X1_SLV>,
+		 <&cru ACLK_PCIE30X1_DBI>, <&cru PCLK_PCIE30X1>,
+		 <&cru CLK_PCIE30X1_AUX_NDFT>,
+		 <&pcie_refclk>;
+	clock-names = "aclk_mst", "aclk_slv",
+		      "aclk_dbi", "pclk", "aux",
+		      "ref";

Hi,

The DT binding in rockchip-dw-pcie-common.yaml defines clock-names as a
positional array:

     items:
       - const: aclk_mst
       - const: aclk_slv
       - const: aclk_dbi
       - const: pclk
       - const: aux
       - const: pipe   <-- position 5
       - const: ref    <-- position 6

With "ref" at position 5, this would fail dtbs_check because the schema
expects "pipe" there.

Other boards that include the ref clock (rk3588-rock-5-itx,
rk3588-jaguar) include "pipe" at position 5 and "ref" at position 6. The
rk3568 has CLK_PCIE30X1_PIPE_DFT available in the CRU header.

Should the pipe clock be included before ref to match the binding
schema?

Good catch, I'll put pipe  before the ref addition.

I'll let my friend test these changes and I'll submit another version incl the pipe.

David

[...]