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@xxxxxxxxxx>
> > ---
> > 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
