Re: [PATCH v6 2/3] arm64: dts: ls1028a: Add PCIe controller DT nodes

From: Michael Walle <michael@walle.cc>
To: xiaowei.bao@nxp.com
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	lorenzo.pieralisi@arm.com, roy.zang@nxp.com,
	linux-pci@vger.kernel.org, Zhiqiang.Hou@nxp.com,
	linux-kernel@vger.kernel.org, leoyang.li@nxp.com,
	minghuan.Lian@nxp.com, Michael Walle <michael@walle.cc>,
	robh+dt@kernel.org, mingkai.hu@nxp.com, bhelgaas@google.com,
	shawnguo@kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 2/3] arm64: dts: ls1028a: Add PCIe controller DT nodes
Date: Mon, 24 Feb 2020 09:11:05 +0100	[thread overview]
Message-ID: <20200224081105.13878-1-michael@walle.cc> (raw)
In-Reply-To: <20190902034319.14026-2-xiaowei.bao@nxp.com>

Hi Xiaowei, Hi Shawn,

> LS1028a implements 2 PCIe 3.0 controllers.

Patch 1/3 and 3/3 are in Linus' tree but nobody seems to care about this patch
anymore :(

This doesn't work well with the IOMMU, because the iommu-map property is
missing. The bootloader needs the &smmu phandle to fixup the entry. See
below.

Shawn, will you add this patch to your tree once its fixed, considering it
just adds the device tree node for the LS1028A?

> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> v2:
>  - Fix up the legacy INTx allocate failed issue.
> v3:
>  - No change.
> v4:
>  - Remove the num-lanes property.
> v5:
>  - Add the num-viewport property.
> v6:
>  - move num-viewport to 8.
> 
>  arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 52 ++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index 72b9a75..c043b1d 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -625,6 +625,58 @@
>  			};
>  		};
>  
> +		pcie@3400000 {
> +			compatible = "fsl,ls1028a-pcie";
> +			reg = <0x00 0x03400000 0x0 0x00100000   /* controller registers */
> +			       0x80 0x00000000 0x0 0x00002000>; /* configuration space */
> +			reg-names = "regs", "config";
> +			interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>, /* PME interrupt */
> +				     <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
> +			interrupt-names = "pme", "aer";
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			dma-coherent;
> +			num-viewport = <8>;
> +			bus-range = <0x0 0xff>;
> +			ranges = <0x81000000 0x0 0x00000000 0x80 0x00010000 0x0 0x00010000   /* downstream I/O */
> +				  0x82000000 0x0 0x40000000 0x80 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
> +			msi-parent = <&its>;
iommu-map = <0 &smmu 0 0>; /* fixed up by bootloader */

> +			#interrupt-cells = <1>;
> +			interrupt-map-mask = <0 0 0 7>;
> +			interrupt-map = <0000 0 0 1 &gic 0 0 GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
> +					<0000 0 0 2 &gic 0 0 GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
> +					<0000 0 0 3 &gic 0 0 GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>,
> +					<0000 0 0 4 &gic 0 0 GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
> +		pcie@3500000 {
> +			compatible = "fsl,ls1028a-pcie";
> +			reg = <0x00 0x03500000 0x0 0x00100000   /* controller registers */
> +			       0x88 0x00000000 0x0 0x00002000>; /* configuration space */
> +			reg-names = "regs", "config";
> +			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "pme", "aer";
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			dma-coherent;
> +			num-viewport = <8>;
> +			bus-range = <0x0 0xff>;
> +			ranges = <0x81000000 0x0 0x00000000 0x88 0x00010000 0x0 0x00010000   /* downstream I/O */
> +				  0x82000000 0x0 0x40000000 0x88 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
> +			msi-parent = <&its>;
likewise

With these two fixes:

Tested-by: Michael Walle <michael@walle.cc>

-michael

> +			#interrupt-cells = <1>;
> +			interrupt-map-mask = <0 0 0 7>;
> +			interrupt-map = <0000 0 0 1 &gic 0 0 GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
> +					<0000 0 0 2 &gic 0 0 GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
> +					<0000 0 0 3 &gic 0 0 GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
> +					<0000 0 0 4 &gic 0 0 GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
>  		pcie@1f0000000 { /* Integrated Endpoint Root Complex */
>  			compatible = "pci-host-ecam-generic";
>  			reg = <0x01 0xf0000000 0x0 0x100000>;
> -- 
> 2.9.5
> 
> 