diff mbox series

[v2,13/14] arm64: dts: rockchip: Add PCIe endpoint mode support

Message ID 20240430-rockchip-pcie-ep-v1-v2-13-a0f5ee2a77b6@kernel.org (mailing list archive)
State Superseded
Delegated to: Manivannan Sadhasivam
Headers show
Series PCI: dw-rockchip: Add endpoint mode support | expand

Commit Message

Niklas Cassel April 30, 2024, 12:01 p.m. UTC
Add a device tree node representing PCIe endpoint mode.

The controller can either be configured to run in Root Complex or Endpoint
node.

If a user wants to run the controller in endpoint mode, the user has to
disable the pcie3x4 node and enable the pcie3x4_ep node.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 arch/arm64/boot/dts/rockchip/rk3588.dtsi | 35 ++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

Manivannan Sadhasivam May 4, 2024, 5:34 p.m. UTC | #1
On Tue, Apr 30, 2024 at 02:01:10PM +0200, Niklas Cassel wrote:
> Add a device tree node representing PCIe endpoint mode.
> 
> The controller can either be configured to run in Root Complex or Endpoint
> node.
> 
> If a user wants to run the controller in endpoint mode, the user has to
> disable the pcie3x4 node and enable the pcie3x4_ep node.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588.dtsi | 35 ++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> index 5519c1430cb7..09a06e8c43b7 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> @@ -136,6 +136,41 @@ pcie3x4_intc: legacy-interrupt-controller {
>  		};
>  	};
>  
> +	pcie3x4_ep: pcie-ep@fe150000 {
> +		compatible = "rockchip,rk3588-pcie-ep";
> +		clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
> +			 <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
> +			 <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>;
> +		clock-names = "aclk_mst", "aclk_slv",
> +			      "aclk_dbi", "pclk",
> +			      "aux", "pipe";
> +		interrupts = <GIC_SPI 263 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 261 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH 0>;
> +		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
> +				  "dma0", "dma1", "dma2", "dma3";
> +		max-link-speed = <3>;
> +		num-lanes = <4>;
> +		phys = <&pcie30phy>;
> +		phy-names = "pcie-phy";
> +		power-domains = <&power RK3588_PD_PCIE>;
> +		reg = <0xa 0x40000000 0x0 0x00100000>,
> +		      <0xa 0x40100000 0x0 0x00100000>,
> +		      <0x0 0xfe150000 0x0 0x00010000>,
> +		      <0x9 0x00000000 0x0 0x40000000>,
> +		      <0xa 0x40300000 0x0 0x00100000>;
> +		reg-names = "dbi", "dbi2", "apb", "addr_space", "atu";

Isn't it common to define 'reg' property just below 'compatible'?

- Mani
Niklas Cassel May 7, 2024, 11:51 p.m. UTC | #2
Hello Mani,

On Sat, May 04, 2024 at 11:04:20PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Apr 30, 2024 at 02:01:10PM +0200, Niklas Cassel wrote:
> > Add a device tree node representing PCIe endpoint mode.
> > 
> > The controller can either be configured to run in Root Complex or Endpoint
> > node.
> > 
> > If a user wants to run the controller in endpoint mode, the user has to
> > disable the pcie3x4 node and enable the pcie3x4_ep node.
> > 
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3588.dtsi | 35 ++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > index 5519c1430cb7..09a06e8c43b7 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > @@ -136,6 +136,41 @@ pcie3x4_intc: legacy-interrupt-controller {
> >  		};
> >  	};
> >  
> > +	pcie3x4_ep: pcie-ep@fe150000 {
> > +		compatible = "rockchip,rk3588-pcie-ep";
> > +		clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
> > +			 <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
> > +			 <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>;
> > +		clock-names = "aclk_mst", "aclk_slv",
> > +			      "aclk_dbi", "pclk",
> > +			      "aux", "pipe";
> > +		interrupts = <GIC_SPI 263 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 261 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH 0>;
> > +		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
> > +				  "dma0", "dma1", "dma2", "dma3";
> > +		max-link-speed = <3>;
> > +		num-lanes = <4>;
> > +		phys = <&pcie30phy>;
> > +		phy-names = "pcie-phy";
> > +		power-domains = <&power RK3588_PD_PCIE>;
> > +		reg = <0xa 0x40000000 0x0 0x00100000>,
> > +		      <0xa 0x40100000 0x0 0x00100000>,
> > +		      <0x0 0xfe150000 0x0 0x00010000>,
> > +		      <0x9 0x00000000 0x0 0x40000000>,
> > +		      <0xa 0x40300000 0x0 0x00100000>;
> > +		reg-names = "dbi", "dbi2", "apb", "addr_space", "atu";
> 
> Isn't it common to define 'reg' property just below 'compatible'?

Looking at the result from:

$ git grep -A 4 "compatible = \"" Documentation/devicetree/bindings/pci/

Common, yes, but far from all examples in
Documentation/devicetree/bindings/pci/ do it that way.

The example in the yaml for this binding passes "make dt_binding_check".
If the device tree maintainers had a strong opinion on this, I would have
expected "make dt_binding_check" to emit a warning or error for this.

Additionally, the "pcie3x4" (RC-node) in rk3588.dtsi already use this same
ordering. I do think there is some value in keeping the ordering in "pcie3x4"
and "pcie3x4_ep" the same, so I will keep the ordering unless the device tree
maintainers start screaming at me :)


Kind regards,
Niklas
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 5519c1430cb7..09a06e8c43b7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -136,6 +136,41 @@  pcie3x4_intc: legacy-interrupt-controller {
 		};
 	};
 
+	pcie3x4_ep: pcie-ep@fe150000 {
+		compatible = "rockchip,rk3588-pcie-ep";
+		clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
+			 <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
+			 <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>;
+		clock-names = "aclk_mst", "aclk_slv",
+			      "aclk_dbi", "pclk",
+			      "aux", "pipe";
+		interrupts = <GIC_SPI 263 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 261 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH 0>;
+		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
+				  "dma0", "dma1", "dma2", "dma3";
+		max-link-speed = <3>;
+		num-lanes = <4>;
+		phys = <&pcie30phy>;
+		phy-names = "pcie-phy";
+		power-domains = <&power RK3588_PD_PCIE>;
+		reg = <0xa 0x40000000 0x0 0x00100000>,
+		      <0xa 0x40100000 0x0 0x00100000>,
+		      <0x0 0xfe150000 0x0 0x00010000>,
+		      <0x9 0x00000000 0x0 0x40000000>,
+		      <0xa 0x40300000 0x0 0x00100000>;
+		reg-names = "dbi", "dbi2", "apb", "addr_space", "atu";
+		resets = <&cru SRST_PCIE0_POWER_UP>, <&cru SRST_P_PCIE0>;
+		reset-names = "pwr", "pipe";
+		status = "disabled";
+	};
+
 	pcie3x2: pcie@fe160000 {
 		compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
 		#address-cells = <3>;