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

[David Heidelberg <david@ixit.cz>]

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

[...]