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

David Heidelberg via B4 Relay posted 3 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v2 2/3] arm64: dts: rockchip: Use reference PCIe clock generator for BPI-R2-Pro
Posted by David Heidelberg via B4 Relay 1 month, 1 week ago
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 <freebsd@sysctl.cz>
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";
 	num-lanes = <1>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&ngffpcie_reset_h>;
@@ -602,6 +607,18 @@ &pcie3x1 {
 
 &pcie3x2 {
 	/* mPCIe slot */
+	/*
+	 * The board has a gpio-controlled "pcie_refclk" generator,
+	 * so add it to the list of clocks.
+	 */
+	clocks = <&cru ACLK_PCIE30X2_MST>, <&cru ACLK_PCIE30X2_SLV>,
+		 <&cru ACLK_PCIE30X2_DBI>, <&cru PCLK_PCIE30X2>,
+		 <&cru CLK_PCIE30X2_AUX_NDFT>,
+		 <&pcie_refclk>;
+	clock-names = "aclk_mst", "aclk_slv",
+		      "aclk_dbi", "pclk", "aux",
+		      "ref";
+
 	num-lanes = <1>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&minipcie_reset_h>;

-- 
2.53.0
Re: [PATCH v2 2/3] arm64: dts: rockchip: Use reference PCIe clock generator for BPI-R2-Pro
Posted by Charalampos Mitrodimas 1 month, 1 week ago
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> writes:

> 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 <freebsd@sysctl.cz>
> 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?

>  	num-lanes = <1>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&ngffpcie_reset_h>;
> @@ -602,6 +607,18 @@ &pcie3x1 {
>  
>  &pcie3x2 {
>  	/* mPCIe slot */
> +	/*
> +	 * The board has a gpio-controlled "pcie_refclk" generator,
> +	 * so add it to the list of clocks.
> +	 */
> +	clocks = <&cru ACLK_PCIE30X2_MST>, <&cru ACLK_PCIE30X2_SLV>,
> +		 <&cru ACLK_PCIE30X2_DBI>, <&cru PCLK_PCIE30X2>,
> +		 <&cru CLK_PCIE30X2_AUX_NDFT>,
> +		 <&pcie_refclk>;
> +	clock-names = "aclk_mst", "aclk_slv",
> +		      "aclk_dbi", "pclk", "aux",
> +		      "ref";
> +

Same clock-names ordering issue I believe here for pci3c2, with
CLK_PCIE30X2_PIPE_DFT available.


>  	num-lanes = <1>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&minipcie_reset_h>;
Re: [PATCH v2 2/3] arm64: dts: rockchip: Use reference PCIe clock generator for BPI-R2-Pro
Posted by David Heidelberg 1 month ago
On 03/03/2026 18:38, Charalampos Mitrodimas wrote:
> David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> writes:
> 
>> 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 <freebsd@sysctl.cz>
>> 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

[...]