[PATCH v2 1/9] arm64: dts: qcom: ipq8074: fix Gen2 PCIe QMP PHY

[PATCH v2 1/9] arm64: dts: qcom: ipq8074: fix Gen2 PCIe QMP PHY

[Robert Marko]

Serdes register space sizes are incorrect, update them to match the
actual sizes from downstream QCA 5.4 kernel.

Fixes: 942bcd33ed45 ("arm64: dts: qcom: Fix IPQ8074 PCIe PHY nodes")
Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 06e2f0157396..31ec24100213 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -277,9 +277,9 @@ pcie_qmp1: phy@8e000 {
 			status = "disabled";
 
 			pcie_phy1: phy@8e200 {
-				reg = <0x8e200 0x16c>,
+				reg = <0x8e200 0x130>,
 				      <0x8e400 0x200>,
-				      <0x8e800 0x4f4>;
+				      <0x8e800 0x1f8>;
 				#phy-cells = <0>;
 				#clock-cells = <0>;
 				clocks = <&gcc GCC_PCIE1_PIPE_CLK>;
-- 
2.39.0

[PATCH v2 2/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe QMP PHY

[Robert Marko]

IPQ8074 comes in 2 silicon versions:
* v1 with 2x Gen2 PCIe ports and QMP PHY-s
* v2 with 1x Gen3 and 1x Gen2 PCIe ports and QMP PHY-s

v2 is the final and production version that is actually supported by the
kernel, however it looks like PCIe related nodes were added for the v1 SoC.

Now that we have Gen3 QMP PHY support, we can start fixing the PCIe support
by fixing the Gen3 QMP PHY node first.

Change the compatible to the Gen3 QMP PHY, correct the register space start
and size, add the missing misc PCS register space.

Fixes: 33057e1672fe ("ARM: dts: ipq8074: Add pcie nodes")
Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 31ec24100213..c93e3f39f33d 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -232,9 +232,9 @@ qusb_phy_0: phy@79000 {
 			status = "disabled";
 		};
 
-		pcie_qmp0: phy@86000 {
-			compatible = "qcom,ipq8074-qmp-pcie-phy";
-			reg = <0x00086000 0x1c4>;
+		pcie_qmp0: phy@84000 {
+			compatible = "qcom,ipq8074-qmp-gen3-pcie-phy";
+			reg = <0x00084000 0x1bc>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
@@ -248,10 +248,11 @@ pcie_qmp0: phy@86000 {
 				      "common";
 			status = "disabled";
 
-			pcie_phy0: phy@86200 {
-				reg = <0x86200 0x16c>,
-				      <0x86400 0x200>,
-				      <0x86800 0x4f4>;
+			pcie_phy0: phy@84200 {
+				reg = <0x84200 0x16c>,
+				      <0x84400 0x200>,
+				      <0x84800 0x1f0>,
+				      <0x84c00 0xf4>;
 				#phy-cells = <0>;
 				#clock-cells = <0>;
 				clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
-- 
2.39.0

[PATCH v2 3/9] arm64: dts: qcom: ipq8074: correct Gen2 PCIe ranges

[Robert Marko]

Current ranges property set in Gen2 PCIe node is incorrect, replace it
with the downstream 5.4 QCA kernel value.

Fixes: 33057e1672fe ("ARM: dts: ipq8074: Add pcie nodes")
Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index c93e3f39f33d..06e1da176334 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -808,9 +808,9 @@ pcie1: pci@10000000 {
 			phy-names = "pciephy";
 
 			ranges = <0x81000000 0 0x10200000 0x10200000
-				  0 0x100000   /* downstream I/O */
-				  0x82000000 0 0x10300000 0x10300000
-				  0 0xd00000>; /* non-prefetchable memory */
+				  0 0x10000>,   /* downstream I/O */
+				 <0x82000000 0 0x10220000 0x10220000
+				  0 0xfde0000>; /* non-prefetchable memory */
 
 			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "msi";
-- 
2.39.0

[PATCH v2 4/9] arm64: dts: qcom: ipq8074: set Gen2 PCIe pcie max-link-speed

[Robert Marko]

Add the generic 'max-link-speed' property to describe the Gen2 PCIe link
generation limit.
This allows the generic DWC code to configure the link speed correctly.

Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 06e1da176334..5ef4383ab18b 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -801,6 +801,7 @@ pcie1: pci@10000000 {
 			linux,pci-domain = <1>;
 			bus-range = <0x00 0xff>;
 			num-lanes = <1>;
+			max-link-speed = <2>;
 			#address-cells = <3>;
 			#size-cells = <2>;
 
-- 
2.39.0

[PATCH v2 5/9] dt-bindings: PCI: qcom: alphabetically sort compatibles

[Robert Marko]

Sort the compatibles list alphabetically for maintenance.

Signed-off-by: Robert Marko <robimarko@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 8da8ece1d4d7..19580350db11 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -17,13 +17,14 @@ description: |
 properties:
   compatible:
     enum:
-      - qcom,pcie-ipq8064
-      - qcom,pcie-ipq8064-v2
       - qcom,pcie-apq8064
       - qcom,pcie-apq8084
-      - qcom,pcie-msm8996
       - qcom,pcie-ipq4019
+      - qcom,pcie-ipq6018
+      - qcom,pcie-ipq8064
+      - qcom,pcie-ipq8064-v2
       - qcom,pcie-ipq8074
+      - qcom,pcie-msm8996
       - qcom,pcie-qcs404
       - qcom,pcie-sa8540p
       - qcom,pcie-sc7280
@@ -35,7 +36,6 @@ properties:
       - qcom,pcie-sm8350
       - qcom,pcie-sm8450-pcie0
       - qcom,pcie-sm8450-pcie1
-      - qcom,pcie-ipq6018
 
   reg:
     minItems: 4
-- 
2.39.0

[PATCH v2 6/9] dt-bindings: PCI: qcom: document IPQ8074 Gen3 port

[Robert Marko]

IPQ8074 has one Gen2 and one Gen3 PCIe port, with Gen2 already supported.
Document Gen3 port which uses the same controller as IPQ6018.

Signed-off-by: Robert Marko <robimarko@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 19580350db11..0f258aa1bfaf 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -24,6 +24,7 @@ properties:
       - qcom,pcie-ipq8064
       - qcom,pcie-ipq8064-v2
       - qcom,pcie-ipq8074
+      - qcom,pcie-ipq8074-gen3
       - qcom,pcie-msm8996
       - qcom,pcie-qcs404
       - qcom,pcie-sa8540p
@@ -154,6 +155,7 @@ allOf:
           contains:
             enum:
               - qcom,pcie-ipq6018
+              - qcom,pcie-ipq8074-gen3
     then:
       properties:
         reg:
@@ -375,6 +377,7 @@ allOf:
           contains:
             enum:
               - qcom,pcie-ipq6018
+              - qcom,pcie-ipq8074-gen3
     then:
       properties:
         clocks:
@@ -695,6 +698,7 @@ allOf:
                 - qcom,pcie-ipq8064
                 - qcom,pcie-ipq8064v2
                 - qcom,pcie-ipq8074
+                - qcom,pcie-ipq8074-gen3
                 - qcom,pcie-qcs404
     then:
       required:
@@ -778,6 +782,7 @@ allOf:
               - qcom,pcie-ipq8064
               - qcom,pcie-ipq8064-v2
               - qcom,pcie-ipq8074
+              - qcom,pcie-ipq8074-gen3
               - qcom,pcie-qcs404
               - qcom,pcie-sa8540p
     then:
-- 
2.39.0

[PATCH v2 7/9] PCI: qcom: Add support for IPQ8074 Gen3 port

[Robert Marko]

IPQ8074 has one Gen2 and one Gen3 port, with Gen2 port already supported.
Add compatible for Gen3 port which uses the same controller as IPQ6018.

Signed-off-by: Robert Marko <robimarko@gmail.com>
---
Changes in v2:
* Capitalize subject to match previous history
---
 drivers/pci/controller/dwc/pcie-qcom.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index b9350d93b4ba..b69a406a2660 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1817,6 +1817,7 @@ static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-ipq8064", .data = &cfg_2_1_0 },
 	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
 	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
+	{ .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
 	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
 	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
 	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
-- 
2.39.0

[PATCH v2 8/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe node

[Robert Marko]

IPQ8074 comes in 2 silicon versions:
* v1 with 2x Gen2 PCIe ports and QMP PHY-s
* v2 with 1x Gen3 and 1x Gen2 PCIe ports and QMP PHY-s

v2 is the final and production version that is actually supported by the
kernel, however it looks like PCIe related nodes were added for the v1 SoC.

Finish the PCIe fixup by using the correct compatible, adding missing ATU
register space, declaring max-link-speed, use correct ranges, add missing
clocks and resets.

Fixes: 33057e1672fe ("ARM: dts: ipq8074: Add pcie nodes")
Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 30 +++++++++++++++------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 5ef4383ab18b..74eecca4f9e3 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -854,16 +854,18 @@ IRQ_TYPE_LEVEL_HIGH>, /* int_c */
 		};
 
 		pcie0: pci@20000000 {
-			compatible = "qcom,pcie-ipq8074";
+			compatible = "qcom,pcie-ipq8074-gen3";
 			reg = <0x20000000 0xf1d>,
 			      <0x20000f20 0xa8>,
-			      <0x00080000 0x2000>,
+			      <0x20001000 0x1000>,
+			      <0x00080000 0x4000>,
 			      <0x20100000 0x1000>;
-			reg-names = "dbi", "elbi", "parf", "config";
+			reg-names = "dbi", "elbi", "atu", "parf", "config";
 			device_type = "pci";
 			linux,pci-domain = <0>;
 			bus-range = <0x00 0xff>;
 			num-lanes = <1>;
+			max-link-speed = <3>;
 			#address-cells = <3>;
 			#size-cells = <2>;
 
@@ -871,9 +873,9 @@ pcie0: pci@20000000 {
 			phy-names = "pciephy";
 
 			ranges = <0x81000000 0 0x20200000 0x20200000
-				  0 0x100000   /* downstream I/O */
-				  0x82000000 0 0x20300000 0x20300000
-				  0 0xd00000>; /* non-prefetchable memory */
+				  0 0x10000>, /* downstream I/O */
+				 <0x82000000 0 0x20220000 0x20220000
+				  0 0xfde0000>; /* non-prefetchable memory */
 
 			interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "msi";
@@ -891,28 +893,30 @@ IRQ_TYPE_LEVEL_HIGH>, /* int_c */
 			clocks = <&gcc GCC_SYS_NOC_PCIE0_AXI_CLK>,
 				 <&gcc GCC_PCIE0_AXI_M_CLK>,
 				 <&gcc GCC_PCIE0_AXI_S_CLK>,
-				 <&gcc GCC_PCIE0_AHB_CLK>,
-				 <&gcc GCC_PCIE0_AUX_CLK>;
-
+				 <&gcc GCC_PCIE0_AXI_S_BRIDGE_CLK>,
+				 <&gcc GCC_PCIE0_RCHNG_CLK>;
 			clock-names = "iface",
 				      "axi_m",
 				      "axi_s",
-				      "ahb",
-				      "aux";
+				      "axi_bridge",
+				      "rchng";
+
 			resets = <&gcc GCC_PCIE0_PIPE_ARES>,
 				 <&gcc GCC_PCIE0_SLEEP_ARES>,
 				 <&gcc GCC_PCIE0_CORE_STICKY_ARES>,
 				 <&gcc GCC_PCIE0_AXI_MASTER_ARES>,
 				 <&gcc GCC_PCIE0_AXI_SLAVE_ARES>,
 				 <&gcc GCC_PCIE0_AHB_ARES>,
-				 <&gcc GCC_PCIE0_AXI_MASTER_STICKY_ARES>;
+				 <&gcc GCC_PCIE0_AXI_MASTER_STICKY_ARES>,
+				 <&gcc GCC_PCIE0_AXI_SLAVE_STICKY_ARES>;
 			reset-names = "pipe",
 				      "sleep",
 				      "sticky",
 				      "axi_m",
 				      "axi_s",
 				      "ahb",
-				      "axi_m_sticky";
+				      "axi_m_sticky",
+				      "axi_s_sticky";
 			status = "disabled";
 		};
 	};
-- 
2.39.0

[PATCH v2 9/9] arm64: dts: qcom: ipq8074: correct PCIe QMP PHY output clock names

[Robert Marko]

Current PCIe QMP PHY output name were changed in ("arm64: dts: qcom: Fix
IPQ8074 PCIe PHY nodes") however it did not account for the fact that GCC
driver is relying on the old names to match them as they are being used as
the parent for the gcc_pcie0_pipe_clk and gcc_pcie1_pipe_clk.

This broke parenting as GCC could not find the parent clock, so fix it by
changing to the names that driver is expecting.

Fixes: 942bcd33ed45 ("arm64: dts: qcom: Fix IPQ8074 PCIe PHY nodes")
Signed-off-by: Robert Marko <robimarko@gmail.com>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 74eecca4f9e3..c6cbeb66c0e7 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -257,7 +257,7 @@ pcie_phy0: phy@84200 {
 				#clock-cells = <0>;
 				clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
 				clock-names = "pipe0";
-				clock-output-names = "pcie_0_pipe_clk";
+				clock-output-names = "pcie20_phy0_pipe_clk";
 			};
 		};
 
@@ -285,7 +285,7 @@ pcie_phy1: phy@8e200 {
 				#clock-cells = <0>;
 				clocks = <&gcc GCC_PCIE1_PIPE_CLK>;
 				clock-names = "pipe0";
-				clock-output-names = "pcie_1_pipe_clk";
+				clock-output-names = "pcie20_phy1_pipe_clk";
 			};
 		};
 
-- 
2.39.0

Re: (subset) [PATCH v2 1/9] arm64: dts: qcom: ipq8074: fix Gen2 PCIe QMP PHY

[Lorenzo Pieralisi]

On Fri, 13 Jan 2023 17:44:41 +0100, Robert Marko wrote:
> Serdes register space sizes are incorrect, update them to match the
> actual sizes from downstream QCA 5.4 kernel.
> 
> 

Applied to pci/qcom, thanks!

[5/9] dt-bindings: PCI: qcom: alphabetically sort compatibles
      https://git.kernel.org/lpieralisi/pci/c/4bc08cf23aaa
[6/9] dt-bindings: PCI: qcom: document IPQ8074 Gen3 port
      https://git.kernel.org/lpieralisi/pci/c/3271543941d8
[7/9] PCI: qcom: Add support for IPQ8074 Gen3 port
      https://git.kernel.org/lpieralisi/pci/c/0591d47a0217

Thanks,
Lorenzo

Re: (subset) [PATCH v2 1/9] arm64: dts: qcom: ipq8074: fix Gen2 PCIe QMP PHY

[Bjorn Andersson]

On Fri, 13 Jan 2023 17:44:41 +0100, Robert Marko wrote:
> Serdes register space sizes are incorrect, update them to match the
> actual sizes from downstream QCA 5.4 kernel.
> 
> 

Applied, thanks!

[1/9] arm64: dts: qcom: ipq8074: fix Gen2 PCIe QMP PHY
      commit: 100d9c94ccf15b02742c326cd04f422ab729153b
[2/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe QMP PHY
      commit: 7ba33591b45f9d547a317e42f1c2acd19c925eb6
[3/9] arm64: dts: qcom: ipq8074: correct Gen2 PCIe ranges
      commit: 2055cb7dccea16bafa3adf9c5e3216949512c34a
[4/9] arm64: dts: qcom: ipq8074: set Gen2 PCIe pcie max-link-speed
      commit: b60590314828e3da670bed94129f4ebc02b87548
[8/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe node
      commit: 3e83a9c41ab0244a45a4a2800b9adb8de0d15f82
[9/9] arm64: dts: qcom: ipq8074: correct PCIe QMP PHY output clock names
      commit: 0e8b90c0256cf9c9589e2cee517dedc987a34355

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [PATCH v2 8/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe node

[Arnd Bergmann]

On Fri, Jan 13, 2023, at 17:44, Robert Marko wrote:
> IPQ8074 comes in 2 silicon versions:
> * v1 with 2x Gen2 PCIe ports and QMP PHY-s
> * v2 with 1x Gen3 and 1x Gen2 PCIe ports and QMP PHY-s
>
> v2 is the final and production version that is actually supported by the
> kernel, however it looks like PCIe related nodes were added for the v1 SoC.
>
> Finish the PCIe fixup by using the correct compatible, adding missing ATU
> register space, declaring max-link-speed, use correct ranges, add missing
> clocks and resets.
>
> Fixes: 33057e1672fe ("ARM: dts: ipq8074: Add pcie nodes")
> Signed-off-by: Robert Marko <robimarko@gmail.com>

I was reading through the pull request today and saw this patch
along with the Gen2 one:


> @@ -871,9 +873,9 @@ pcie0: pci@20000000 {
>  			phy-names = "pciephy";
> 
>  			ranges = <0x81000000 0 0x20200000 0x20200000
> -				  0 0x100000   /* downstream I/O */
> +				  0 0x10000>, /* downstream I/O */

Fixing the length here seems fine, but the bus-side address
still looks wrong: 0x20200000 is way outside of the usual
port ranges from 0 to 0x10000 on the local bus.

> -				  0x82000000 0 0x20300000 0x20300000
> -				  0 0xd00000>; /* non-prefetchable memory */
> +				 <0x82000000 0 0x20220000 0x20220000
> +				  0 0xfde0000>; /* non-prefetchable memory */

I see the total size of the memory space is under 256MB. Are you
sure that there is no 64-bit BAR in addition to this?

I also see commit 7d1158c984d3 ("arm64: dts: qcom: sm8550: Add
PCIe PHYs and controllers nodes") introduce the same broken
I/O port range (oversized 1MB space wiht an identity map) for a
new SoC. This should probably be fixed as well, along with
reviewing the other ones.

Has the I/O space mapping on any of these actually been tested,
or just copied from one SoC to another? Very few devices actually
use I/O space, so it wouldn't be surprising if it never worked
in the first place.

       Arnd

Re: [PATCH v2 8/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe node

[Robert Marko]

On Mon, 30 Jan 2023 at 18:11, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Jan 13, 2023, at 17:44, Robert Marko wrote:
> > IPQ8074 comes in 2 silicon versions:
> > * v1 with 2x Gen2 PCIe ports and QMP PHY-s
> > * v2 with 1x Gen3 and 1x Gen2 PCIe ports and QMP PHY-s
> >
> > v2 is the final and production version that is actually supported by the
> > kernel, however it looks like PCIe related nodes were added for the v1 SoC.
> >
> > Finish the PCIe fixup by using the correct compatible, adding missing ATU
> > register space, declaring max-link-speed, use correct ranges, add missing
> > clocks and resets.
> >
> > Fixes: 33057e1672fe ("ARM: dts: ipq8074: Add pcie nodes")
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
>
> I was reading through the pull request today and saw this patch
> along with the Gen2 one:
>
>
> > @@ -871,9 +873,9 @@ pcie0: pci@20000000 {
> >                       phy-names = "pciephy";
> >
> >                       ranges = <0x81000000 0 0x20200000 0x20200000
> > -                               0 0x100000   /* downstream I/O */
> > +                               0 0x10000>, /* downstream I/O */
>
> Fixing the length here seems fine, but the bus-side address
> still looks wrong: 0x20200000 is way outside of the usual
> port ranges from 0 to 0x10000 on the local bus.
>
> > -                               0x82000000 0 0x20300000 0x20300000
> > -                               0 0xd00000>; /* non-prefetchable memory */
> > +                              <0x82000000 0 0x20220000 0x20220000
> > +                               0 0xfde0000>; /* non-prefetchable memory */
>
> I see the total size of the memory space is under 256MB. Are you
> sure that there is no 64-bit BAR in addition to this?
>
> I also see commit 7d1158c984d3 ("arm64: dts: qcom: sm8550: Add
> PCIe PHYs and controllers nodes") introduce the same broken
> I/O port range (oversized 1MB space wiht an identity map) for a
> new SoC. This should probably be fixed as well, along with
> reviewing the other ones.
>
> Has the I/O space mapping on any of these actually been tested,
> or just copied from one SoC to another? Very few devices actually
> use I/O space, so it wouldn't be surprising if it never worked
> in the first place.

Hi Arnd,
As pointed out in the commit description, the ranges property was copied
from the QCA-s downstream 5.4 kernel [1] as I dont have any documentation
on the SoC.

I have runtime tested this on Xiaomi AX3600 which has a QCA9889 card on the
Gen3 PCIe port, and on Xiaomi AX9000 which has QCA9889 on Gen2 port
and QCN9074 on the Gen3 port and they are working fine.

[1] https://git.codelinaro.org/clo/qsdk/oss/kernel/linux-ipq-5.4/-/blob/NHSS.QSDK.12.3.r2/arch/arm64/boot/dts/qcom/ipq8074.dtsi#L834

Regards,
Robert
>
>        Arnd

Re: [PATCH v2 8/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe node

[Arnd Bergmann]

On Thu, Feb 2, 2023, at 10:16, Robert Marko wrote:
> On Mon, 30 Jan 2023 at 18:11, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Fri, Jan 13, 2023, at 17:44, Robert Marko wrote:
>
> As pointed out in the commit description, the ranges property was copied
> from the QCA-s downstream 5.4 kernel [1] as I dont have any documentation
> on the SoC.
>
> I have runtime tested this on Xiaomi AX3600 which has a QCA9889 card on the
> Gen3 PCIe port, and on Xiaomi AX9000 which has QCA9889 on Gen2 port
> and QCN9074 on the Gen3 port and they are working fine.

Neither of those use I/O ports though, nor does any other sensible
device that was made in the past decade.

The compatible string tells me that this is a designware pcie block,
so I think driver actually sets up the mapping based on the ranges
property in DT in dw_pcie_iatu_detect() and dw_pcie_iatu_setup(),
rather than the mapping being determined by hardware or firmware
in advance.

Not sure about the general policy we have for this case, maybe the
pci controller or pci-dwc maintainers have an idea here. I would
think it's better to either have no I/O ranges in DT or have
sensible ones than ones that are clearly bogus, if the controller
is able to set up the ranges.

     Arnd

Re: [PATCH v2 8/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe node

[Manivannan Sadhasivam]

On Thu, Feb 02, 2023 at 10:42:15AM +0100, Arnd Bergmann wrote:
> On Thu, Feb 2, 2023, at 10:16, Robert Marko wrote:
> > On Mon, 30 Jan 2023 at 18:11, Arnd Bergmann <arnd@arndb.de> wrote:
> >> On Fri, Jan 13, 2023, at 17:44, Robert Marko wrote:
> >
> > As pointed out in the commit description, the ranges property was copied
> > from the QCA-s downstream 5.4 kernel [1] as I dont have any documentation
> > on the SoC.
> >
> > I have runtime tested this on Xiaomi AX3600 which has a QCA9889 card on the
> > Gen3 PCIe port, and on Xiaomi AX9000 which has QCA9889 on Gen2 port
> > and QCN9074 on the Gen3 port and they are working fine.
> 
> Neither of those use I/O ports though, nor does any other sensible
> device that was made in the past decade.
> 
> The compatible string tells me that this is a designware pcie block,
> so I think driver actually sets up the mapping based on the ranges
> property in DT in dw_pcie_iatu_detect() and dw_pcie_iatu_setup(),
> rather than the mapping being determined by hardware or firmware
> in advance.
> 
> Not sure about the general policy we have for this case, maybe the
> pci controller or pci-dwc maintainers have an idea here. I would
> think it's better to either have no I/O ranges in DT or have
> sensible ones than ones that are clearly bogus, if the controller
> is able to set up the ranges.
> 

Just happen to see this thread and I agree that the I/O port range is indeeed
bogus. This is due to the fact that no one tested I/O range with a compatible
device.

I'm not sure about the PCI policy though but we should fix the mapping across
all SoCs. I will send out a series for that.

Thanks for spotting, Arnd!

- Mani

>      Arnd

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

Re: [PATCH v2 8/9] arm64: dts: qcom: ipq8074: fix Gen3 PCIe node

[Robert Marko]

On Tue, 28 Feb 2023 at 14:20, Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Thu, Feb 02, 2023 at 10:42:15AM +0100, Arnd Bergmann wrote:
> > On Thu, Feb 2, 2023, at 10:16, Robert Marko wrote:
> > > On Mon, 30 Jan 2023 at 18:11, Arnd Bergmann <arnd@arndb.de> wrote:
> > >> On Fri, Jan 13, 2023, at 17:44, Robert Marko wrote:
> > >
> > > As pointed out in the commit description, the ranges property was copied
> > > from the QCA-s downstream 5.4 kernel [1] as I dont have any documentation
> > > on the SoC.
> > >
> > > I have runtime tested this on Xiaomi AX3600 which has a QCA9889 card on the
> > > Gen3 PCIe port, and on Xiaomi AX9000 which has QCA9889 on Gen2 port
> > > and QCN9074 on the Gen3 port and they are working fine.
> >
> > Neither of those use I/O ports though, nor does any other sensible
> > device that was made in the past decade.
> >
> > The compatible string tells me that this is a designware pcie block,
> > so I think driver actually sets up the mapping based on the ranges
> > property in DT in dw_pcie_iatu_detect() and dw_pcie_iatu_setup(),
> > rather than the mapping being determined by hardware or firmware
> > in advance.
> >
> > Not sure about the general policy we have for this case, maybe the
> > pci controller or pci-dwc maintainers have an idea here. I would
> > think it's better to either have no I/O ranges in DT or have
> > sensible ones than ones that are clearly bogus, if the controller
> > is able to set up the ranges.
> >
>
> Just happen to see this thread and I agree that the I/O port range is indeeed
> bogus. This is due to the fact that no one tested I/O range with a compatible
> device.
>
> I'm not sure about the PCI policy though but we should fix the mapping across
> all SoCs. I will send out a series for that.

Thanks for sorting this out.

Regards,
Robert
>
> Thanks for spotting, Arnd!
>
> - Mani
>
> >      Arnd
>
> --
> மணிவண்ணன் சதாசிவம்