Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <dafbff84-209c-a4d5-1e84-f08e84156ed8@ti.com>
Date:   Mon, 21 Nov 2022 11:12:18 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v6 7/8] arm64: dts: ti: k3-j721s2-main: Add PCIe device
 tree node
Content-Language: en-US
To:     Matt Ranostay <mranostay@ti.com>, <nm@ti.com>, <vigneshr@ti.com>,
        <kristo@kernel.org>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <s-vadapalli@ti.com>,
        <r-gunasekaran@ti.com>
CC:     <linux-arm-kernel@lists.infradead.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20221119040906.9495-1-mranostay@ti.com>
 <20221119040906.9495-8-mranostay@ti.com>
From:   Andrew Davis <afd@ti.com>
In-Reply-To: <20221119040906.9495-8-mranostay@ti.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/18/22 10:09 PM, Matt Ranostay wrote:
> From: Aswath Govindraju <a-govindraju@ti.com>
> 
> Add PCIe device tree node (both RC and EP) for the single PCIe
> instance present in j721s2.
> 

So which is it, you have two nodes but this is one device. It can
switch between the two modes, a property should have been used to
select the mode for the device.

Making two nodes for the same device as examples of what they could
look like, then only enabling one of the two in the board level DT
is not how this is done anywhere else. Take the common parts and
make one node here with those. Then at the board level .dts where we
select what mode this driver will act in, add the specific bits for
the chosen mode.

Andrew

> Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> Signed-off-by: Matt Ranostay <mranostay@ti.com>
> ---
>   arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 61 ++++++++++++++++++++++
>   1 file changed, 61 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
> index adbb172658b9..04294e25d587 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
> @@ -841,6 +841,67 @@ serdes0: serdes@5060000 {
>   		};
>   	};
>   
> +	pcie1_rc: pcie@2910000 {
> +		compatible = "ti,j7200-pcie-host", "ti,j721e-pcie-host";
> +		reg = <0x00 0x02910000 0x00 0x1000>,
> +		      <0x00 0x02917000 0x00 0x400>,
> +		      <0x00 0x0d800000 0x00 0x00800000>,
> +		      <0x00 0x18000000 0x00 0x00001000>;
> +		reg-names = "intd_cfg", "user_cfg", "reg", "cfg";
> +		interrupt-names = "link_state";
> +		interrupts = <GIC_SPI 330 IRQ_TYPE_EDGE_RISING>;
> +		device_type = "pci";
> +		ti,syscon-pcie-ctrl = <&scm_conf 0x074>;
> +		max-link-speed = <3>;
> +		num-lanes = <4>;
> +		power-domains = <&k3_pds 276 TI_SCI_PD_EXCLUSIVE>;
> +		clocks = <&k3_clks 276 41>;
> +		clock-names = "fck";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		bus-range = <0x0 0xff>;
> +		vendor-id = <0x104c>;
> +		device-id = <0xb013>;
> +		msi-map = <0x0 &gic_its 0x0 0x10000>;
> +		dma-coherent;
> +		ranges = <0x01000000 0x0 0x18001000  0x00 0x18001000  0x0 0x0010000>,
> +			 <0x02000000 0x0 0x18011000  0x00 0x18011000  0x0 0x7fef000>;
> +		dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>;
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 7>;
> +		interrupt-map = <0 0 0 1 &pcie1_intc 0>, /* INT A */
> +				<0 0 0 2 &pcie1_intc 0>, /* INT B */
> +				<0 0 0 3 &pcie1_intc 0>, /* INT C */
> +				<0 0 0 4 &pcie1_intc 0>; /* INT D */
> +
> +		pcie1_intc: interrupt-controller {
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +			interrupt-parent = <&gic500>;
> +			interrupts = <GIC_SPI 324 IRQ_TYPE_EDGE_RISING>;
> +		};
> +	};
> +
> +	pcie1_ep: pcie-ep@2910000 {
> +		compatible = "ti,j7200-pcie-ep", "ti,j721e-pcie-ep";
> +		reg = <0x00 0x02910000 0x00 0x1000>,
> +		      <0x00 0x02917000 0x00 0x400>,
> +		      <0x00 0x0d800000 0x00 0x00800000>,
> +		      <0x00 0x18000000 0x00 0x08000000>;
> +		reg-names = "intd_cfg", "user_cfg", "reg", "mem";
> +		interrupt-names = "link_state";
> +		interrupts = <GIC_SPI 330 IRQ_TYPE_EDGE_RISING>;
> +		ti,syscon-pcie-ctrl = <&scm_conf 0x074>;
> +		max-link-speed = <3>;
> +		num-lanes = <4>;
> +		power-domains = <&k3_pds 276 TI_SCI_PD_EXCLUSIVE>;
> +		clocks = <&k3_clks 276 41>;
> +		clock-names = "fck";
> +		max-functions = /bits/ 8 <6>;
> +		max-virtual-functions = /bits/ 8 <4 4 4 4 0 0>;
> +		dma-coherent;
> +	};
> +
>   	main_mcan0: can@2701000 {
>   		compatible = "bosch,m_can";
>   		reg = <0x00 0x02701000 0x00 0x200>,