devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add PCIe Gen2x1 controller support for RK3528
@ 2025-09-06 13:52 Yao Zi
  2025-09-06 13:52 ` [PATCH 1/3] dt-bindings: PCI: dwc: rockchip: Add RK3528 variant Yao Zi
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Yao Zi @ 2025-09-06 13:52 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue
  Cc: linux-pci, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, Jonas Karlman, Chukun Pan, Yao Zi

Rockchip RK3528 ships one PCIe Gen2x1 controller that operates in RC
mode only. The SoC doesn't provide a separate MSI controller, thus the
one integrated in designware PCIe IP must be used. This series documents
the PCIe controller in dt-binding and describes it in the SoC devicetree.

Radxa E20C board is used for testing, whose LAN GbE port is provided
through an RTL8111H chip connected to PCIe controller. Its devicetree
is adjusted to enable the controller, and IPERF3 shows the interface
runs at full-speed. A typical result looks like

[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  1.09 GBytes   936 Mbits/sec    0            sender
[  5]   0.00-10.00  sec  1.09 GBytes   934 Mbits/sec                  receiver

This series is based on next-20250905. It's worth noting that
commit 727e914bbfbb ("PCI/MSI: Check MSI_FLAG_PCI_MSI_MASK_PARENT in
cond_[startup|shutdown]_parent()") (already contained in next-20250905)
is necessary for normal operation of designware PCIe IP's integrated MSI
controller.

Thanks for your time and review.

Yao Zi (3):
  dt-bindings: PCI: dwc: rockchip: Add RK3528 variant
  arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
  arm64: dts: rockchip: Enable PCIe controller on Radxa E20C

 .../bindings/pci/rockchip-dw-pcie.yaml        |  3 +
 .../boot/dts/rockchip/rk3528-radxa-e20c.dts   | 17 ++++++
 arch/arm64/boot/dts/rockchip/rk3528.dtsi      | 56 ++++++++++++++++++-
 3 files changed, 75 insertions(+), 1 deletion(-)

-- 
2.50.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] dt-bindings: PCI: dwc: rockchip: Add RK3528 variant
  2025-09-06 13:52 [PATCH 0/3] Add PCIe Gen2x1 controller support for RK3528 Yao Zi
@ 2025-09-06 13:52 ` Yao Zi
  2025-09-09  0:47   ` Rob Herring (Arm)
  2025-09-06 13:52 ` [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528 Yao Zi
  2025-09-06 13:52 ` [PATCH 3/3] arm64: dts: rockchip: Enable PCIe controller on Radxa E20C Yao Zi
  2 siblings, 1 reply; 12+ messages in thread
From: Yao Zi @ 2025-09-06 13:52 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue
  Cc: linux-pci, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, Jonas Karlman, Chukun Pan, Yao Zi

RK3528 ships a PCIe Gen2x1 controller that operates in RC mode only.
Since the SoC has no separate MSI controller, the one integrated in the
DWC PCIe IP must be used, and thus its interrupt scheme is similar to
variants found in RK3562 and RK3576.

Older BSP code claimed its integrated MSI controller supports only 8
MSIs[1], but this has been changed in newer BSP[2] and testing proves
the controller works correctly with more than 8 MSIs allocated,
suggesting the controller should be compatible with the RK3568 variant.
Let's document its compatible string.

Link: https://github.com/rockchip-linux/kernel/blob/792a7d4273a5/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L1610-L1613 # [1]
Link: https://github.com/rockchip-linux/kernel/blob/1ba51b059f25/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L904-L906 # [2]
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 6c6d828ce964..67f1a5502048 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -22,6 +22,7 @@ properties:
       - const: rockchip,rk3568-pcie
       - items:
           - enum:
+              - rockchip,rk3528-pcie
               - rockchip,rk3562-pcie
               - rockchip,rk3576-pcie
               - rockchip,rk3588-pcie
@@ -78,6 +79,7 @@ allOf:
           compatible:
             contains:
               enum:
+                - rockchip,rk3528-pcie
                 - rockchip,rk3562-pcie
                 - rockchip,rk3576-pcie
     then:
@@ -89,6 +91,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - rockchip,rk3528-pcie
               - rockchip,rk3562-pcie
               - rockchip,rk3576-pcie
     then:
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
  2025-09-06 13:52 [PATCH 0/3] Add PCIe Gen2x1 controller support for RK3528 Yao Zi
  2025-09-06 13:52 ` [PATCH 1/3] dt-bindings: PCI: dwc: rockchip: Add RK3528 variant Yao Zi
@ 2025-09-06 13:52 ` Yao Zi
  2025-09-09 12:50   ` Chukun Pan
  2025-09-10 21:29   ` Jonas Karlman
  2025-09-06 13:52 ` [PATCH 3/3] arm64: dts: rockchip: Enable PCIe controller on Radxa E20C Yao Zi
  2 siblings, 2 replies; 12+ messages in thread
From: Yao Zi @ 2025-09-06 13:52 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue
  Cc: linux-pci, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, Jonas Karlman, Chukun Pan, Yao Zi

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] arm64: dts: rockchip: Enable PCIe controller on Radxa E20C
  2025-09-06 13:52 [PATCH 0/3] Add PCIe Gen2x1 controller support for RK3528 Yao Zi
  2025-09-06 13:52 ` [PATCH 1/3] dt-bindings: PCI: dwc: rockchip: Add RK3528 variant Yao Zi
  2025-09-06 13:52 ` [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528 Yao Zi
@ 2025-09-06 13:52 ` Yao Zi
  2025-09-09 13:00   ` Chukun Pan
  2 siblings, 1 reply; 12+ messages in thread
From: Yao Zi @ 2025-09-06 13:52 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue
  Cc: linux-pci, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel, Jonas Karlman, Chukun Pan, Yao Zi

Radxa E20C provides one of its GbE ports through RTL8111H connected to
SoC's PCIe controller. Let's enable the controller and the PHY used by
it to allow usage of the port.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 .../boot/dts/rockchip/rk3528-radxa-e20c.dts     | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
index 12eec2c1db22..e880c7a7e674 100644
--- a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
@@ -171,6 +171,10 @@ vdd_logic: regulator-vdd-logic {
 	};
 };
 
+&combphy {
+	status = "okay";
+};
+
 &cpu0 {
 	cpu-supply = <&vdd_arm>;
 };
@@ -229,6 +233,13 @@ rgmii_phy: ethernet-phy@1 {
 	};
 };
 
+&pcie {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pciem1_pins>, <&pcie_reset_g>;
+	reset-gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+};
+
 &pinctrl {
 	ethernet {
 		gmac1_rstn_l: gmac1-rstn-l {
@@ -256,6 +267,12 @@ wan_led_g: wan-led-g {
 		};
 	};
 
+	pcie {
+		pcie_reset_g: pcie-reset-g {
+			rockchip,pins = <1 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	sdmmc {
 		sdmmc_vol_ctrl_h: sdmmc-vol-ctrl-h {
 			rockchip,pins = <4 RK_PB6 RK_FUNC_GPIO &pcfg_pull_none>;
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: PCI: dwc: rockchip: Add RK3528 variant
  2025-09-06 13:52 ` [PATCH 1/3] dt-bindings: PCI: dwc: rockchip: Add RK3528 variant Yao Zi
@ 2025-09-09  0:47   ` Rob Herring (Arm)
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2025-09-09  0:47 UTC (permalink / raw)
  To: Yao Zi
  Cc: Bjorn Helgaas, Shawn Lin, Conor Dooley, Simon Xue, linux-rockchip,
	Krzysztof Wilczyński, Chukun Pan, Lorenzo Pieralisi,
	linux-arm-kernel, Manivannan Sadhasivam, Jonas Karlman,
	devicetree, Krzysztof Kozlowski, Heiko Stuebner, linux-pci,
	linux-kernel


On Sat, 06 Sep 2025 13:52:44 +0000, Yao Zi wrote:
> RK3528 ships a PCIe Gen2x1 controller that operates in RC mode only.
> Since the SoC has no separate MSI controller, the one integrated in the
> DWC PCIe IP must be used, and thus its interrupt scheme is similar to
> variants found in RK3562 and RK3576.
> 
> Older BSP code claimed its integrated MSI controller supports only 8
> MSIs[1], but this has been changed in newer BSP[2] and testing proves
> the controller works correctly with more than 8 MSIs allocated,
> suggesting the controller should be compatible with the RK3568 variant.
> Let's document its compatible string.
> 
> Link: https://github.com/rockchip-linux/kernel/blob/792a7d4273a5/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L1610-L1613 # [1]
> Link: https://github.com/rockchip-linux/kernel/blob/1ba51b059f25/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L904-L906 # [2]
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
  2025-09-06 13:52 ` [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528 Yao Zi
@ 2025-09-09 12:50   ` Chukun Pan
  2025-09-11  8:09     ` Yao Zi
  2025-09-10 21:29   ` Jonas Karlman
  1 sibling, 1 reply; 12+ messages in thread
From: Chukun Pan @ 2025-09-09 12:50 UTC (permalink / raw)
  To: ziyao
  Cc: amadeus, bhelgaas, conor+dt, devicetree, heiko, jonas, krzk+dt,
	kwilczynski, linux-arm-kernel, linux-kernel, linux-pci,
	linux-rockchip, lpieralisi, mani, robh

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Enable PCIe controller on Radxa E20C
  2025-09-06 13:52 ` [PATCH 3/3] arm64: dts: rockchip: Enable PCIe controller on Radxa E20C Yao Zi
@ 2025-09-09 13:00   ` Chukun Pan
  2025-09-11  8:10     ` Yao Zi
  0 siblings, 1 reply; 12+ messages in thread
From: Chukun Pan @ 2025-09-09 13:00 UTC (permalink / raw)
  To: ziyao
  Cc: amadeus, bhelgaas, conor+dt, devicetree, heiko, jonas, krzk+dt,
	kwilczynski, linux-arm-kernel, linux-kernel, linux-pci,
	linux-rockchip, lpieralisi, mani, robh

Hi,

> +&pcie {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pciem1_pins>, <&pcie_reset_g>;

The pciem1_pins contains PCIE20_PERSTn_M1 (GPIO1_A2)
This will cause conflicts, "pinctrl-0 = <&pciem1_pins>" is enough.

[    0.115608] rockchip-pinctrl pinctrl: not freeing pin 34 (gpio1-2) as part of deactivating group pciem1-pins - it is already used for some other setting
[    0.191042] rockchip-pinctrl pinctrl: pin gpio1-2 already requeste by pll_gpll; cannot claim for 140000000.pcie
[    0.191949] rockchip-pinctrl pinctrl: error -EINVAL: pin-34 (140000000.pcie)
[    0.192570] rockchip-pinctrl pinctrl: error -EINVAL: could not request pin 34 (gpio1-2) from group pciem1-pins on device rockchip-pinctrl

> +	reset-gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;

Missing supply: "vpcie3v3-supply = <&vcc_3v3>;"

> +	status = "okay";
> +};
> +

Thanks,
Chukun

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
  2025-09-06 13:52 ` [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528 Yao Zi
  2025-09-09 12:50   ` Chukun Pan
@ 2025-09-10 21:29   ` Jonas Karlman
  2025-09-11  7:56     ` Yao Zi
  1 sibling, 1 reply; 12+ messages in thread
From: Jonas Karlman @ 2025-09-10 21:29 UTC (permalink / raw)
  To: Yao Zi
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue, linux-pci,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	Chukun Pan

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>;
> +			};
> +		};
>  	};
>  };
>  


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
  2025-09-10 21:29   ` Jonas Karlman
@ 2025-09-11  7:56     ` Yao Zi
  2025-09-11  8:44       ` Jonas Karlman
  0 siblings, 1 reply; 12+ messages in thread
From: Yao Zi @ 2025-09-11  7:56 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue, linux-pci,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	Chukun Pan

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
  2025-09-09 12:50   ` Chukun Pan
@ 2025-09-11  8:09     ` Yao Zi
  0 siblings, 0 replies; 12+ messages in thread
From: Yao Zi @ 2025-09-11  8:09 UTC (permalink / raw)
  To: Chukun Pan
  Cc: bhelgaas, conor+dt, devicetree, heiko, jonas, krzk+dt,
	kwilczynski, linux-arm-kernel, linux-kernel, linux-pci,
	linux-rockchip, lpieralisi, mani, robh

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Enable PCIe controller on Radxa E20C
  2025-09-09 13:00   ` Chukun Pan
@ 2025-09-11  8:10     ` Yao Zi
  0 siblings, 0 replies; 12+ messages in thread
From: Yao Zi @ 2025-09-11  8:10 UTC (permalink / raw)
  To: Chukun Pan
  Cc: bhelgaas, conor+dt, devicetree, heiko, jonas, krzk+dt,
	kwilczynski, linux-arm-kernel, linux-kernel, linux-pci,
	linux-rockchip, lpieralisi, mani, robh

On Tue, Sep 09, 2025 at 09:00:09PM +0800, Chukun Pan wrote:
> Hi,
> 
> > +&pcie {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pciem1_pins>, <&pcie_reset_g>;
> 
> The pciem1_pins contains PCIE20_PERSTn_M1 (GPIO1_A2)
> This will cause conflicts, "pinctrl-0 = <&pciem1_pins>" is enough.
> 
> [    0.115608] rockchip-pinctrl pinctrl: not freeing pin 34 (gpio1-2) as part of deactivating group pciem1-pins - it is already used for some other setting
> [    0.191042] rockchip-pinctrl pinctrl: pin gpio1-2 already requeste by pll_gpll; cannot claim for 140000000.pcie
> [    0.191949] rockchip-pinctrl pinctrl: error -EINVAL: pin-34 (140000000.pcie)
> [    0.192570] rockchip-pinctrl pinctrl: error -EINVAL: could not request pin 34 (gpio1-2) from group pciem1-pins on device rockchip-pinctrl
> 
> > +	reset-gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
> 
> Missing supply: "vpcie3v3-supply = <&vcc_3v3>;"
> 
> > +	status = "okay";
> > +};
> > +

Will fix them in v2, thanks.

> Thanks,
> Chukun

Best regards,
Yao Zi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528
  2025-09-11  7:56     ` Yao Zi
@ 2025-09-11  8:44       ` Jonas Karlman
  0 siblings, 0 replies; 12+ messages in thread
From: Jonas Karlman @ 2025-09-11  8:44 UTC (permalink / raw)
  To: Yao Zi
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Shawn Lin, Simon Xue, linux-pci,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	Chukun Pan

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-09-11  8:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-06 13:52 [PATCH 0/3] Add PCIe Gen2x1 controller support for RK3528 Yao Zi
2025-09-06 13:52 ` [PATCH 1/3] dt-bindings: PCI: dwc: rockchip: Add RK3528 variant Yao Zi
2025-09-09  0:47   ` Rob Herring (Arm)
2025-09-06 13:52 ` [PATCH 2/3] arm64: dts: rockchip: Add PCIe Gen2x1 controller for RK3528 Yao Zi
2025-09-09 12:50   ` Chukun Pan
2025-09-11  8:09     ` Yao Zi
2025-09-10 21:29   ` Jonas Karlman
2025-09-11  7:56     ` Yao Zi
2025-09-11  8:44       ` Jonas Karlman
2025-09-06 13:52 ` [PATCH 3/3] arm64: dts: rockchip: Enable PCIe controller on Radxa E20C Yao Zi
2025-09-09 13:00   ` Chukun Pan
2025-09-11  8:10     ` Yao Zi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).