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

[Niklas Cassel]

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 at 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 at 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