Re: [PATCH v4 12/13] arm64: dts: rockchip: Add PCIe endpoint mode support

[Manivannan Sadhasivam <mani@kernel.org>]

On Wed, May 29, 2024 at 10:29:06AM +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>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

One nitpick below.

> ---
>  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 5984016b5f96..6b5bf1055143 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> @@ -186,6 +186,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";

As suggested in the bindings patch, please move these reg properties below
compatible.

- Mani

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

-- 
மணிவண்ணன் சதாசிவம்