[v6,7/8] arm64: dts: ti: k3-j721s2-main: Add PCIe device tree node
Commit Message
From: Aswath Govindraju <a-govindraju@ti.com>
Add PCIe device tree node (both RC and EP) for the single PCIe
instance present in j721s2.
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(+)
Comments
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>,
On Mon, Nov 21, 2022 at 11:12:18AM -0600, Andrew Davis wrote:
> 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.
>
This isn't how it is done in k3-j7200-common-proc-board.dts and k3-j7200-main.dtsi
Now I'm fine with making it common node if that is way to go..
- Matt
> 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>,
@@ -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>,