[PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528

Yao Zi posted 3 patches 5 months ago
There is a newer version of this series
[PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
Posted by Yao Zi 5 months ago
Describes the PCIe Gen2x1 controller integrated in RK3528 SoC. The SoC
doesn't provide a separate MSI controller, thus the one integrated in
designware PCIe IP must be used.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/arm64/boot/dts/rockchip/rk3528.dtsi | 56 +++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
index db5dbcac7756..2d2af467e5ab 100644
--- a/arch/arm64/boot/dts/rockchip/rk3528.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
@@ -7,6 +7,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/phy/phy.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/clock/rockchip,rk3528-cru.h>
 #include <dt-bindings/power/rockchip,rk3528-power.h>
@@ -239,7 +240,7 @@ gmac0_clk: clock-gmac50m {
 
 	soc {
 		compatible = "simple-bus";
-		ranges = <0x0 0xfe000000 0x0 0xfe000000 0x0 0x2000000>;
+		ranges = <0x0 0xfc000000 0x0 0xfc000000 0x0 0x44400000>;
 		#address-cells = <2>;
 		#size-cells = <2>;
 
@@ -1133,6 +1134,59 @@ combphy: phy@ffdc0000 {
 			rockchip,pipe-phy-grf = <&pipe_phy_grf>;
 			status = "disabled";
 		};
+
+		pcie: pcie@fe4f0000 {
+			compatible = "rockchip,rk3528-pcie",
+				     "rockchip,rk3568-pcie";
+			reg = <0x1 0x40000000 0x0 0x400000>,
+			      <0x0 0xfe4f0000 0x0 0x10000>,
+			      <0x0 0xfc000000 0x0 0x100000>;
+			reg-names = "dbi", "apb", "config";
+			bus-range = <0x0 0xff>;
+			clocks = <&cru ACLK_PCIE>, <&cru HCLK_PCIE_SLV>,
+				 <&cru HCLK_PCIE_DBI>, <&cru PCLK_PCIE>,
+				 <&cru CLK_PCIE_AUX>, <&cru PCLK_PCIE_PHY>;
+			clock-names = "aclk_mst", "aclk_slv",
+				      "aclk_dbi", "pclk",
+				      "aux", "pipe";
+			device_type = "pci";
+			interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "sys", "pmc", "msg", "legacy", "err",
+					  "msi";
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0 0 0 1 &pcie_intc 0>,
+					<0 0 0 2 &pcie_intc 1>,
+					<0 0 0 3 &pcie_intc 2>,
+					<0 0 0 4 &pcie_intc 3>;
+			linux,pci-domain = <0>;
+			max-link-speed = <2>;
+			num-lanes = <1>;
+			phys = <&combphy PHY_TYPE_PCIE>;
+			phy-names = "pcie-phy";
+			power-domains = <&power RK3528_PD_VPU>;
+			ranges = <0x01000000 0x0 0xfc100000 0x0 0xfc100000 0x0 0x100000>,
+				 <0x02000000 0x0 0xfc200000 0x0 0xfc200000 0x0 0x1e00000>,
+				 <0x03000000 0x1 0x00000000 0x1 0x00000000 0x0 0x40000000>;
+			resets = <&cru SRST_PCIE_POWER_UP>, <&cru SRST_P_PCIE>;
+			reset-names = "pwr", "pipe";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			status = "disabled";
+
+			pcie_intc: legacy-interrupt-controller {
+				interrupt-controller;
+				interrupt-parent = <&gic>;
+				interrupts = <GIC_SPI 155 IRQ_TYPE_EDGE_RISING>;
+				#address-cells = <0>;
+				#interrupt-cells = <1>;
+			};
+		};
 	};
 };
 
-- 
2.50.1
Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
Posted by Jonas Karlman 5 months ago
Hi Yao Zi,

On 9/6/2025 3:52 PM, Yao Zi wrote:
> Describes the PCIe Gen2x1 controller integrated in RK3528 SoC. The SoC
> doesn't provide a separate MSI controller, thus the one integrated in
> designware PCIe IP must be used.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 56 +++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> index db5dbcac7756..2d2af467e5ab 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> @@ -7,6 +7,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/phy/phy.h>
>  #include <dt-bindings/pinctrl/rockchip.h>
>  #include <dt-bindings/clock/rockchip,rk3528-cru.h>
>  #include <dt-bindings/power/rockchip,rk3528-power.h>
> @@ -239,7 +240,7 @@ gmac0_clk: clock-gmac50m {
>  
>  	soc {
>  		compatible = "simple-bus";
> -		ranges = <0x0 0xfe000000 0x0 0xfe000000 0x0 0x2000000>;
> +		ranges = <0x0 0xfc000000 0x0 0xfc000000 0x0 0x44400000>;

We should use the dbi reg area in the 32-bit address space, please use:

  ranges = <0x0 0xfc000000 0x0 0xfc000000 0x0 0x4000000>;

>  		#address-cells = <2>;
>  		#size-cells = <2>;
>  
> @@ -1133,6 +1134,59 @@ combphy: phy@ffdc0000 {
>  			rockchip,pipe-phy-grf = <&pipe_phy_grf>;
>  			status = "disabled";
>  		};
> +
> +		pcie: pcie@fe4f0000 {

With the dbi reg area changed below, please update the node name and
move this node to top of the soc node.

  pcie@fe000000

> +			compatible = "rockchip,rk3528-pcie",
> +				     "rockchip,rk3568-pcie";
> +			reg = <0x1 0x40000000 0x0 0x400000>,

We should use the dbi reg area in the 32-bit address space, please use:

  reg = <0x0 0xfe000000 0x0 0x400000>,

> +			      <0x0 0xfe4f0000 0x0 0x10000>,
> +			      <0x0 0xfc000000 0x0 0x100000>;
> +			reg-names = "dbi", "apb", "config";
> +			bus-range = <0x0 0xff>;
> +			clocks = <&cru ACLK_PCIE>, <&cru HCLK_PCIE_SLV>,
> +				 <&cru HCLK_PCIE_DBI>, <&cru PCLK_PCIE>,
> +				 <&cru CLK_PCIE_AUX>, <&cru PCLK_PCIE_PHY>;
> +			clock-names = "aclk_mst", "aclk_slv",
> +				      "aclk_dbi", "pclk",
> +				      "aux", "pipe";

In my U-Boot test I did not have the pipe/phy clock here, do we need it?

With above fixed this more or less matches my U-Boot testing, and is:

Reviewed-by: Jonas Karlman <jonas@kwiboo.se>

Regards,
Jonas

> +			device_type = "pci";
> +			interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "sys", "pmc", "msg", "legacy", "err",
> +					  "msi";
> +			#interrupt-cells = <1>;
> +			interrupt-map-mask = <0 0 0 7>;
> +			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> +					<0 0 0 2 &pcie_intc 1>,
> +					<0 0 0 3 &pcie_intc 2>,
> +					<0 0 0 4 &pcie_intc 3>;
> +			linux,pci-domain = <0>;
> +			max-link-speed = <2>;
> +			num-lanes = <1>;
> +			phys = <&combphy PHY_TYPE_PCIE>;
> +			phy-names = "pcie-phy";
> +			power-domains = <&power RK3528_PD_VPU>;
> +			ranges = <0x01000000 0x0 0xfc100000 0x0 0xfc100000 0x0 0x100000>,
> +				 <0x02000000 0x0 0xfc200000 0x0 0xfc200000 0x0 0x1e00000>,
> +				 <0x03000000 0x1 0x00000000 0x1 0x00000000 0x0 0x40000000>;
> +			resets = <&cru SRST_PCIE_POWER_UP>, <&cru SRST_P_PCIE>;
> +			reset-names = "pwr", "pipe";
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			status = "disabled";
> +
> +			pcie_intc: legacy-interrupt-controller {
> +				interrupt-controller;
> +				interrupt-parent = <&gic>;
> +				interrupts = <GIC_SPI 155 IRQ_TYPE_EDGE_RISING>;
> +				#address-cells = <0>;
> +				#interrupt-cells = <1>;
> +			};
> +		};
>  	};
>  };
>
Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
Posted by Yao Zi 5 months ago
On Wed, Sep 10, 2025 at 11:29:00PM +0200, Jonas Karlman wrote:
> Hi Yao Zi,
> 
> On 9/6/2025 3:52 PM, Yao Zi wrote:
> > Describes the PCIe Gen2x1 controller integrated in RK3528 SoC. The SoC
> > doesn't provide a separate MSI controller, thus the one integrated in
> > designware PCIe IP must be used.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 56 +++++++++++++++++++++++-
> >  1 file changed, 55 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > index db5dbcac7756..2d2af467e5ab 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > @@ -7,6 +7,7 @@
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> >  #include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/phy/phy.h>
> >  #include <dt-bindings/pinctrl/rockchip.h>
> >  #include <dt-bindings/clock/rockchip,rk3528-cru.h>
> >  #include <dt-bindings/power/rockchip,rk3528-power.h>
> > @@ -239,7 +240,7 @@ gmac0_clk: clock-gmac50m {
> >  
> >  	soc {
> >  		compatible = "simple-bus";
> > -		ranges = <0x0 0xfe000000 0x0 0xfe000000 0x0 0x2000000>;
> > +		ranges = <0x0 0xfc000000 0x0 0xfc000000 0x0 0x44400000>;
> 
> We should use the dbi reg area in the 32-bit address space, please use:
> 
>   ranges = <0x0 0xfc000000 0x0 0xfc000000 0x0 0x4000000>;

This seems strange to me. I read through TRMs for RK3562 and RK3576, and
found it's common for Rockchip SoCs to map DBI regions of PCIe
controllers to two separate MMIO regions, but am still not sure why it's
necessary to use the mapping in the 32-bit address space.

However, I'm willing to follow the vendor's decision here in order to
avoid unexpected problems. Will adapt this in v2.

> >  		#address-cells = <2>;
> >  		#size-cells = <2>;
> >  
> > @@ -1133,6 +1134,59 @@ combphy: phy@ffdc0000 {
> >  			rockchip,pipe-phy-grf = <&pipe_phy_grf>;
> >  			status = "disabled";
> >  		};
> > +
> > +		pcie: pcie@fe4f0000 {
> 
> With the dbi reg area changed below, please update the node name and
> move this node to top of the soc node.
> 
>   pcie@fe000000
> 
> > +			compatible = "rockchip,rk3528-pcie",
> > +				     "rockchip,rk3568-pcie";
> > +			reg = <0x1 0x40000000 0x0 0x400000>,
> 
> We should use the dbi reg area in the 32-bit address space, please use:
> 
>   reg = <0x0 0xfe000000 0x0 0x400000>,
> 
> > +			      <0x0 0xfe4f0000 0x0 0x10000>,
> > +			      <0x0 0xfc000000 0x0 0x100000>;
> > +			reg-names = "dbi", "apb", "config";
> > +			bus-range = <0x0 0xff>;
> > +			clocks = <&cru ACLK_PCIE>, <&cru HCLK_PCIE_SLV>,
> > +				 <&cru HCLK_PCIE_DBI>, <&cru PCLK_PCIE>,
> > +				 <&cru CLK_PCIE_AUX>, <&cru PCLK_PCIE_PHY>;
> > +			clock-names = "aclk_mst", "aclk_slv",
> > +				      "aclk_dbi", "pclk",
> > +				      "aux", "pipe";
> 
> In my U-Boot test I did not have the pipe/phy clock here, do we need it?

Just as mentioned by Chukun, the clock should indeed be managed by phy
instead of the PCIe controller. Will fix it as well.

> With above fixed this more or less matches my U-Boot testing, and is:
> 
> Reviewed-by: Jonas Karlman <jonas@kwiboo.se>

Much thanks.

> Regards,
> Jonas

Best regards,
Yao Zi
Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
Posted by Jonas Karlman 5 months ago
On 9/11/2025 9:56 AM, Yao Zi wrote:
> On Wed, Sep 10, 2025 at 11:29:00PM +0200, Jonas Karlman wrote:
>> Hi Yao Zi,
>>
>> On 9/6/2025 3:52 PM, Yao Zi wrote:
>>> Describes the PCIe Gen2x1 controller integrated in RK3528 SoC. The SoC
>>> doesn't provide a separate MSI controller, thus the one integrated in
>>> designware PCIe IP must be used.
>>>
>>> Signed-off-by: Yao Zi <ziyao@disroot.org>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 56 +++++++++++++++++++++++-
>>>  1 file changed, 55 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>>> index db5dbcac7756..2d2af467e5ab 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>>> @@ -7,6 +7,7 @@
>>>  #include <dt-bindings/gpio/gpio.h>
>>>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/phy/phy.h>
>>>  #include <dt-bindings/pinctrl/rockchip.h>
>>>  #include <dt-bindings/clock/rockchip,rk3528-cru.h>
>>>  #include <dt-bindings/power/rockchip,rk3528-power.h>
>>> @@ -239,7 +240,7 @@ gmac0_clk: clock-gmac50m {
>>>  
>>>  	soc {
>>>  		compatible = "simple-bus";
>>> -		ranges = <0x0 0xfe000000 0x0 0xfe000000 0x0 0x2000000>;
>>> +		ranges = <0x0 0xfc000000 0x0 0xfc000000 0x0 0x44400000>;
>>
>> We should use the dbi reg area in the 32-bit address space, please use:
>>
>>   ranges = <0x0 0xfc000000 0x0 0xfc000000 0x0 0x4000000>;
> 
> This seems strange to me. I read through TRMs for RK3562 and RK3576, and
> found it's common for Rockchip SoCs to map DBI regions of PCIe
> controllers to two separate MMIO regions, but am still not sure why it's
> necessary to use the mapping in the 32-bit address space.

I prefer the use of the 32-bit address range to ensure better
compatibility with bootloaders and possible other OS that may have
issues with regs in 64-bit address range.

E.g. U-Boot have had issues with accessing >32-bit addressable range in
the past, use of the 32-bit dbi range ensure we can use pcie in
U-Boot without having to possible patch DT in a <soc>-u-boot.dtsi file.

Regards,
Jonas

> 
> However, I'm willing to follow the vendor's decision here in order to
> avoid unexpected problems. Will adapt this in v2.
> 
>>>  		#address-cells = <2>;
>>>  		#size-cells = <2>;
>>>  
>>> @@ -1133,6 +1134,59 @@ combphy: phy@ffdc0000 {
>>>  			rockchip,pipe-phy-grf = <&pipe_phy_grf>;
>>>  			status = "disabled";
>>>  		};
>>> +
>>> +		pcie: pcie@fe4f0000 {
>>
>> With the dbi reg area changed below, please update the node name and
>> move this node to top of the soc node.
>>
>>   pcie@fe000000
>>
>>> +			compatible = "rockchip,rk3528-pcie",
>>> +				     "rockchip,rk3568-pcie";
>>> +			reg = <0x1 0x40000000 0x0 0x400000>,
>>
>> We should use the dbi reg area in the 32-bit address space, please use:
>>
>>   reg = <0x0 0xfe000000 0x0 0x400000>,
>>
>>> +			      <0x0 0xfe4f0000 0x0 0x10000>,
>>> +			      <0x0 0xfc000000 0x0 0x100000>;
>>> +			reg-names = "dbi", "apb", "config";
>>> +			bus-range = <0x0 0xff>;
>>> +			clocks = <&cru ACLK_PCIE>, <&cru HCLK_PCIE_SLV>,
>>> +				 <&cru HCLK_PCIE_DBI>, <&cru PCLK_PCIE>,
>>> +				 <&cru CLK_PCIE_AUX>, <&cru PCLK_PCIE_PHY>;
>>> +			clock-names = "aclk_mst", "aclk_slv",
>>> +				      "aclk_dbi", "pclk",
>>> +				      "aux", "pipe";
>>
>> In my U-Boot test I did not have the pipe/phy clock here, do we need it?
> 
> Just as mentioned by Chukun, the clock should indeed be managed by phy
> instead of the PCIe controller. Will fix it as well.
> 
>> With above fixed this more or less matches my U-Boot testing, and is:
>>
>> Reviewed-by: Jonas Karlman <jonas@kwiboo.se>
> 
> Much thanks.
> 
>> Regards,
>> Jonas
> 
> Best regards,
> Yao Zi
Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
Posted by Chukun Pan 5 months ago
Hi,

> +			reg = <0x1 0x40000000 0x0 0x400000>,
> +			      <0x0 0xfe4f0000 0x0 0x10000>,
> +			      <0x0 0xfc000000 0x0 0x100000>;

Aligning the address for reg and ranges will look better:

		reg = <0x1 0x40000000 0x0 0x400000>,
		      <0x0 0xfe4f0000 0x0 0x010000>,
		      <0x0 0xfc000000 0x0 0x100000>;

BTW do we possibly need this?
https://github.com/rockchip-linux/kernel/commit/e9397245c4b1bd62ef929d221e20225d58467dc7

> +			clocks = <&cru ACLK_PCIE>, <&cru HCLK_PCIE_SLV>,
> +				 <&cru HCLK_PCIE_DBI>, <&cru PCLK_PCIE>,
> +				 <&cru CLK_PCIE_AUX>, <&cru PCLK_PCIE_PHY>;

<&cru PCLK_PCIE_PHY> has already been defined in the combphy node,
is it repeated here?

Thanks,
Chukun
Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
Posted by Yao Zi 5 months ago
On Tue, Sep 09, 2025 at 08:50:29PM +0800, Chukun Pan wrote:
> Hi,
> 
> > +			reg = <0x1 0x40000000 0x0 0x400000>,
> > +			      <0x0 0xfe4f0000 0x0 0x10000>,
> > +			      <0x0 0xfc000000 0x0 0x100000>;
> 
> Aligning the address for reg and ranges will look better:
> 
> 		reg = <0x1 0x40000000 0x0 0x400000>,
> 		      <0x0 0xfe4f0000 0x0 0x010000>,
> 		      <0x0 0xfc000000 0x0 0x100000>;

Thanks, this makes sense.

> BTW do we possibly need this?
> https://github.com/rockchip-linux/kernel/commit/e9397245c4b1bd62ef929d221e20225d58467dc7

I'm still unsure its purpose, but am willing to adapt this change. See
my reply to Jonas' comment.

> > +			clocks = <&cru ACLK_PCIE>, <&cru HCLK_PCIE_SLV>,
> > +				 <&cru HCLK_PCIE_DBI>, <&cru PCLK_PCIE>,
> > +				 <&cru CLK_PCIE_AUX>, <&cru PCLK_PCIE_PHY>;
> 
> <&cru PCLK_PCIE_PHY> has already been defined in the combphy node,
> is it repeated here?

Yes, it should be managed by PHY instead of the controller. I'll fix it
in v2.

> Thanks,
> Chukun

Best regards,
Yao Zi