Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/4] Layerscape: Remove num-lanes property from PCIe nodes
@ 2019-08-12  4:22 Z.q. Hou
  2019-08-12  4:22 ` [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties Z.q. Hou
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Z.q. Hou @ 2019-08-12  4:22 UTC (permalink / raw)
  To: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi
  Cc: M.h. Lian, Z.q. Hou

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

On FSL Layerscape SoCs, the number of lanes assigned to PCIe
controller is not fixed, it is determined by the selected
SerDes protocol. The current num-lanes indicates the max lanes
PCIe controller can support up to, instead of the lanes assigned
to the PCIe controller. This can result in PCIe link training fail
after hot-reset.

Hou Zhiqiang (4):
  dt-bingings: PCI: Remove the num-lanes from Required properties
  PCI: dwc: Return directly when num-lanes is not found
  ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes
  arm64: dts: fsl: Remove num-lanes property from PCIe nodes

 Documentation/devicetree/bindings/pci/designware-pcie.txt | 1 -
 arch/arm/boot/dts/ls1021a.dtsi                            | 2 --
 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi            | 1 -
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi            | 3 ---
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi            | 6 ------
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi            | 3 ---
 arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi            | 4 ----
 drivers/pci/controller/dwc/pcie-designware.c              | 6 ++++--
 8 files changed, 4 insertions(+), 22 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties
  2019-08-12  4:22 [PATCH 0/4] Layerscape: Remove num-lanes property from PCIe nodes Z.q. Hou
@ 2019-08-12  4:22 ` Z.q. Hou
  2019-08-12  8:45   ` Andrew Murray
  2019-08-19 19:20   ` Bjorn Helgaas
  2019-08-12  4:22 ` [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found Z.q. Hou
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 18+ messages in thread
From: Z.q. Hou @ 2019-08-12  4:22 UTC (permalink / raw)
  To: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi
  Cc: M.h. Lian, Z.q. Hou

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The num-lanes is not a mandatory property, e.g. on FSL
Layerscape SoCs, the PCIe link training is completed
automatically base on the selected SerDes protocol, it
doesn't need the num-lanes to set-up the link width.

It has been added in the Optional properties. This
patch is to remove it from the Required properties.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 Documentation/devicetree/bindings/pci/designware-pcie.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index 5561a1c060d0..bd880df39a79 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -11,7 +11,6 @@ Required properties:
 	     the ATU address space.
     (The old way of getting the configuration address space from "ranges"
     is deprecated and should be avoided.)
-- num-lanes: number of lanes to use
 RC mode:
 - #address-cells: set to <3>
 - #size-cells: set to <2>
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found
  2019-08-12  4:22 [PATCH 0/4] Layerscape: Remove num-lanes property from PCIe nodes Z.q. Hou
  2019-08-12  4:22 ` [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties Z.q. Hou
@ 2019-08-12  4:22 ` Z.q. Hou
  2019-08-12  8:34   ` Andrew Murray
  2019-08-12  4:22 ` [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes Z.q. Hou
  2019-08-12  4:22 ` [PATCH 4/4] arm64: dts: fsl: " Z.q. Hou
  3 siblings, 1 reply; 18+ messages in thread
From: Z.q. Hou @ 2019-08-12  4:22 UTC (permalink / raw)
  To: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi
  Cc: M.h. Lian, Z.q. Hou

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The num-lanes is optional, so probably it isn't added
on some platforms. The subsequent programming is base
on the num-lanes, hence return when it is not found.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 7d25102c304c..0a89bfd1636e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -423,8 +423,10 @@ void dw_pcie_setup(struct dw_pcie *pci)
 
 
 	ret = of_property_read_u32(np, "num-lanes", &lanes);
-	if (ret)
-		lanes = 0;
+	if (ret) {
+		dev_dbg(pci->dev, "property num-lanes isn't found\n");
+		return;
+	}
 
 	/* Set the number of lanes */
 	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes
  2019-08-12  4:22 [PATCH 0/4] Layerscape: Remove num-lanes property from PCIe nodes Z.q. Hou
  2019-08-12  4:22 ` [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties Z.q. Hou
  2019-08-12  4:22 ` [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found Z.q. Hou
@ 2019-08-12  4:22 ` Z.q. Hou
  2019-08-12  8:35   ` Andrew Murray
  2019-08-19 19:24   ` Bjorn Helgaas
  2019-08-12  4:22 ` [PATCH 4/4] arm64: dts: fsl: " Z.q. Hou
  3 siblings, 2 replies; 18+ messages in thread
From: Z.q. Hou @ 2019-08-12  4:22 UTC (permalink / raw)
  To: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi
  Cc: M.h. Lian, Z.q. Hou

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

On FSL Layerscape SoCs, the number of lanes assigned to PCIe
controller is not fixed, it is determined by the selected
SerDes protocol in the RCW (Reset Configuration Word), and
the PCIe link training is completed automatically base on
the selected SerDes protocol, and the link width set-up is
updated by hardware. So the num-lanes is not needed to
specify the link width.

The current num-lanes indicates the max lanes PCIe controller
can support up to, instead of the lanes assigned to the PCIe
controller. This can result in PCIe link training fail after
hot-reset. So remove the num-lanes to avoid set-up to incorrect
link width.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 arch/arm/boot/dts/ls1021a.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 464df4290ffc..2f6977ada447 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -874,7 +874,6 @@
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";
-			num-lanes = <4>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
@@ -899,7 +898,6 @@
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";
-			num-lanes = <4>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000   /* downstream I/O */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] arm64: dts: fsl: Remove num-lanes property from PCIe nodes
  2019-08-12  4:22 [PATCH 0/4] Layerscape: Remove num-lanes property from PCIe nodes Z.q. Hou
                   ` (2 preceding siblings ...)
  2019-08-12  4:22 ` [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes Z.q. Hou
@ 2019-08-12  4:22 ` " Z.q. Hou
  2019-08-12  8:35   ` Andrew Murray
  3 siblings, 1 reply; 18+ messages in thread
From: Z.q. Hou @ 2019-08-12  4:22 UTC (permalink / raw)
  To: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi
  Cc: M.h. Lian, Z.q. Hou

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

On FSL Layerscape SoCs, the number of lanes assigned to PCIe
controller is not fixed, it is determined by the selected
SerDes protocol in the RCW (Reset Configuration Word), and
the PCIe link training is completed automatically base on
the selected SerDes protocol, and the link width set-up is
updated by hardware. So the num-lanes is not needed to
specify the link width.

The current num-lanes indicates the max lanes PCIe controller
can support up to, instead of the lanes assigned to the PCIe
controller. This can result in PCIe link training fail after
hot-reset. So remove the num-lanes to avoid set-up to incorrect
link width.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 -
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 3 ---
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 6 ------
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 3 ---
 arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ----
 5 files changed, 17 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
index ec6257a5b251..119c597ca867 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -486,7 +486,6 @@
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";
-			num-lanes = <4>;
 			num-viewport = <2>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 71d9ed9ff985..c084c7a4b6a6 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -677,7 +677,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <4>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
@@ -704,7 +703,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <2>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000   /* downstream I/O */
@@ -731,7 +729,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <2>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x50 0x00010000 0x0 0x00010000   /* downstream I/O */
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index b0ef08b090dd..d4c1da3d4bde 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -649,7 +649,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <4>;
 			num-viewport = <8>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
@@ -671,7 +670,6 @@
 			reg-names = "regs", "addr_space";
 			num-ib-windows = <6>;
 			num-ob-windows = <8>;
-			num-lanes = <2>;
 			status = "disabled";
 		};
 
@@ -687,7 +685,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <2>;
 			num-viewport = <8>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000   /* downstream I/O */
@@ -709,7 +706,6 @@
 			reg-names = "regs", "addr_space";
 			num-ib-windows = <6>;
 			num-ob-windows = <8>;
-			num-lanes = <2>;
 			status = "disabled";
 		};
 
@@ -725,7 +721,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <2>;
 			num-viewport = <8>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x50 0x00010000 0x0 0x00010000   /* downstream I/O */
@@ -747,7 +742,6 @@
 			reg-names = "regs", "addr_space";
 			num-ib-windows = <6>;
 			num-ob-windows = <8>;
-			num-lanes = <2>;
 			status = "disabled";
 		};
 
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index dfbead405783..76c87afeba1e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -456,7 +456,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <4>;
 			num-viewport = <256>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x20 0x00010000 0x0 0x00010000   /* downstream I/O */
@@ -482,7 +481,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <4>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x28 0x00010000 0x0 0x00010000   /* downstream I/O */
@@ -508,7 +506,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <8>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			ranges = <0x81000000 0x0 0x00000000 0x30 0x00010000 0x0 0x00010000   /* downstream I/O */
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 64101c9962ce..7a0be8eaa84a 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -639,7 +639,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <4>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			msi-parent = <&its>;
@@ -661,7 +660,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <4>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			msi-parent = <&its>;
@@ -683,7 +681,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <8>;
 			num-viewport = <256>;
 			bus-range = <0x0 0xff>;
 			msi-parent = <&its>;
@@ -705,7 +702,6 @@
 			#size-cells = <2>;
 			device_type = "pci";
 			dma-coherent;
-			num-lanes = <4>;
 			num-viewport = <6>;
 			bus-range = <0x0 0xff>;
 			msi-parent = <&its>;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found
  2019-08-12  4:22 ` [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found Z.q. Hou
@ 2019-08-12  8:34   ` Andrew Murray
  2019-08-13  3:13     ` Z.q. Hou
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Murray @ 2019-08-12  8:34 UTC (permalink / raw)
  To: Z.q. Hou
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

On Mon, Aug 12, 2019 at 04:22:22AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> The num-lanes is optional, so probably it isn't added
> on some platforms. The subsequent programming is base
> on the num-lanes, hence return when it is not found.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 7d25102c304c..0a89bfd1636e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -423,8 +423,10 @@ void dw_pcie_setup(struct dw_pcie *pci)
>  
>  
>  	ret = of_property_read_u32(np, "num-lanes", &lanes);
> -	if (ret)
> -		lanes = 0;
> +	if (ret) {
> +		dev_dbg(pci->dev, "property num-lanes isn't found\n");
> +		return;
> +	}

The existing code would assign a value of 0 to lanes when num-lanes isn't
specified, however this value isn't supported by the following switch
statement - thus we'd also print an error and return.

Therefore the only and subtle effect effect of this patch is to change
a dev_err to a dev_dbg when num-lanes isn't specified and avoid a read of
PCIE_PORT_LINK_CONTROL.

Given that num-lanes is described as optional this makes perfect sense.
Though the commit message/hunk does give the apperance that this provides
a more functional change.

Anyway:

Reviewed-by: Andrew Murray <andrew.murray@arm.com>


>  
>  	/* Set the number of lanes */
>  	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes
  2019-08-12  4:22 ` [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes Z.q. Hou
@ 2019-08-12  8:35   ` Andrew Murray
  2019-08-13  3:13     ` Z.q. Hou
  2019-08-19 19:24   ` Bjorn Helgaas
  1 sibling, 1 reply; 18+ messages in thread
From: Andrew Murray @ 2019-08-12  8:35 UTC (permalink / raw)
  To: Z.q. Hou
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

On Mon, Aug 12, 2019 at 04:22:27AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> On FSL Layerscape SoCs, the number of lanes assigned to PCIe
> controller is not fixed, it is determined by the selected
> SerDes protocol in the RCW (Reset Configuration Word), and
> the PCIe link training is completed automatically base on
> the selected SerDes protocol, and the link width set-up is
> updated by hardware. So the num-lanes is not needed to
> specify the link width.
> 
> The current num-lanes indicates the max lanes PCIe controller
> can support up to, instead of the lanes assigned to the PCIe
> controller. This can result in PCIe link training fail after
> hot-reset. So remove the num-lanes to avoid set-up to incorrect
> link width.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  arch/arm/boot/dts/ls1021a.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
> index 464df4290ffc..2f6977ada447 100644
> --- a/arch/arm/boot/dts/ls1021a.dtsi
> +++ b/arch/arm/boot/dts/ls1021a.dtsi
> @@ -874,7 +874,6 @@
>  			#address-cells = <3>;
>  			#size-cells = <2>;
>  			device_type = "pci";
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -899,7 +898,6 @@
>  			#address-cells = <3>;
>  			#size-cells = <2>;
>  			device_type = "pci";
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000   /* downstream I/O */

Reviewed-by: Andrew Murray <andrew.murray@arm.com>

> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] arm64: dts: fsl: Remove num-lanes property from PCIe nodes
  2019-08-12  4:22 ` [PATCH 4/4] arm64: dts: fsl: " Z.q. Hou
@ 2019-08-12  8:35   ` Andrew Murray
  2019-08-13  3:14     ` Z.q. Hou
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Murray @ 2019-08-12  8:35 UTC (permalink / raw)
  To: Z.q. Hou
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

On Mon, Aug 12, 2019 at 04:22:33AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> On FSL Layerscape SoCs, the number of lanes assigned to PCIe
> controller is not fixed, it is determined by the selected
> SerDes protocol in the RCW (Reset Configuration Word), and
> the PCIe link training is completed automatically base on
> the selected SerDes protocol, and the link width set-up is
> updated by hardware. So the num-lanes is not needed to
> specify the link width.
> 
> The current num-lanes indicates the max lanes PCIe controller
> can support up to, instead of the lanes assigned to the PCIe
> controller. This can result in PCIe link training fail after
> hot-reset. So remove the num-lanes to avoid set-up to incorrect
> link width.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 -
>  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 3 ---
>  arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 6 ------
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 3 ---
>  arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ----
>  5 files changed, 17 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> index ec6257a5b251..119c597ca867 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> @@ -486,7 +486,6 @@
>  			#address-cells = <3>;
>  			#size-cells = <2>;
>  			device_type = "pci";
> -			num-lanes = <4>;
>  			num-viewport = <2>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index 71d9ed9ff985..c084c7a4b6a6 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> @@ -677,7 +677,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -704,7 +703,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <2>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -731,7 +729,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <2>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x50 0x00010000 0x0 0x00010000   /* downstream I/O */
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> index b0ef08b090dd..d4c1da3d4bde 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> @@ -649,7 +649,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <4>;
>  			num-viewport = <8>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -671,7 +670,6 @@
>  			reg-names = "regs", "addr_space";
>  			num-ib-windows = <6>;
>  			num-ob-windows = <8>;
> -			num-lanes = <2>;
>  			status = "disabled";
>  		};
>  
> @@ -687,7 +685,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <2>;
>  			num-viewport = <8>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -709,7 +706,6 @@
>  			reg-names = "regs", "addr_space";
>  			num-ib-windows = <6>;
>  			num-ob-windows = <8>;
> -			num-lanes = <2>;
>  			status = "disabled";
>  		};
>  
> @@ -725,7 +721,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <2>;
>  			num-viewport = <8>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x50 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -747,7 +742,6 @@
>  			reg-names = "regs", "addr_space";
>  			num-ib-windows = <6>;
>  			num-ob-windows = <8>;
> -			num-lanes = <2>;
>  			status = "disabled";
>  		};
>  
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> index dfbead405783..76c87afeba1e 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> @@ -456,7 +456,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <4>;
>  			num-viewport = <256>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x20 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -482,7 +481,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x28 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -508,7 +506,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <8>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x30 0x00010000 0x0 0x00010000   /* downstream I/O */
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> index 64101c9962ce..7a0be8eaa84a 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> @@ -639,7 +639,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			msi-parent = <&its>;
> @@ -661,7 +660,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			msi-parent = <&its>;
> @@ -683,7 +681,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <8>;
>  			num-viewport = <256>;
>  			bus-range = <0x0 0xff>;
>  			msi-parent = <&its>;
> @@ -705,7 +702,6 @@
>  			#size-cells = <2>;
>  			device_type = "pci";
>  			dma-coherent;
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			msi-parent = <&its>;

Reviewed-by: Andrew Murray <andrew.murray@arm.com> 

> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties
  2019-08-12  4:22 ` [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties Z.q. Hou
@ 2019-08-12  8:45   ` Andrew Murray
  2019-08-13  3:07     ` Z.q. Hou
  2019-08-19 19:20   ` Bjorn Helgaas
  1 sibling, 1 reply; 18+ messages in thread
From: Andrew Murray @ 2019-08-12  8:45 UTC (permalink / raw)
  To: Z.q. Hou
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian, Kishon Vijay Abraham I,
	Gabriele Paoloni

On Mon, Aug 12, 2019 at 04:22:16AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> The num-lanes is not a mandatory property, e.g. on FSL
> Layerscape SoCs, the PCIe link training is completed
> automatically base on the selected SerDes protocol, it
> doesn't need the num-lanes to set-up the link width.
> 
> It has been added in the Optional properties. This
> patch is to remove it from the Required properties.

For clarity, maybe this paragraph can be reworded to:

"It is previously in both Required and Optional properties,
 let's remove it from the Required properties".

I don't understand why this property is previously in
both required and optional...

It looks like num-lanes was first made optional back in
2015 and removed from the Required section (907fce090253).
But then re-added back into the Required section in 2017
with the adition of bindings for EP mode (b12befecd7de).

Is num-lanes actually required for EP mode?

Thanks,

Andrew Murray

> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 5561a1c060d0..bd880df39a79 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -11,7 +11,6 @@ Required properties:
>  	     the ATU address space.
>      (The old way of getting the configuration address space from "ranges"
>      is deprecated and should be avoided.)
> -- num-lanes: number of lanes to use
>  RC mode:
>  - #address-cells: set to <3>
>  - #size-cells: set to <2>
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties
  2019-08-12  8:45   ` Andrew Murray
@ 2019-08-13  3:07     ` Z.q. Hou
  2019-08-13  4:35       ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 18+ messages in thread
From: Z.q. Hou @ 2019-08-13  3:07 UTC (permalink / raw)
  To: Andrew Murray
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian, Kishon Vijay Abraham I,
	Gabriele Paoloni

Hi Andrew,

Thanks a lot for your comments!

> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019Äê8ÔÂ12ÈÕ 16:45
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; gustavo.pimentel@synopsys.com;
> jingoohan1@gmail.com; bhelgaas@google.com; robh+dt@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> <leoyang.li@nxp.com>; lorenzo.pieralisi@arm.com; M.h. Lian
> <minghuan.lian@nxp.com>; Kishon Vijay Abraham I <kishon@ti.com>;
> Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Subject: Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from
> Required properties
> 
> On Mon, Aug 12, 2019 at 04:22:16AM +0000, Z.q. Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The num-lanes is not a mandatory property, e.g. on FSL Layerscape
> > SoCs, the PCIe link training is completed automatically base on the
> > selected SerDes protocol, it doesn't need the num-lanes to set-up the
> > link width.
> >
> > It has been added in the Optional properties. This patch is to remove
> > it from the Required properties.
> 
> For clarity, maybe this paragraph can be reworded to:
> 
> "It is previously in both Required and Optional properties,  let's remove it
> from the Required properties".

Agree and will change in v2.

> 
> I don't understand why this property is previously in both required and
> optional...
> 
> It looks like num-lanes was first made optional back in
> 2015 and removed from the Required section (907fce090253).
> But then re-added back into the Required section in 2017 with the adition of
> bindings for EP mode (b12befecd7de).
> 
> Is num-lanes actually required for EP mode?

Kishon, please help to answer this query?

Thanks,
Zhiqiang

> 
> Thanks,
> 
> Andrew Murray
> 
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/pci/designware-pcie.txt | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > index 5561a1c060d0..bd880df39a79 100644
> > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > @@ -11,7 +11,6 @@ Required properties:
> >  	     the ATU address space.
> >      (The old way of getting the configuration address space from
> "ranges"
> >      is deprecated and should be avoided.)
> > -- num-lanes: number of lanes to use
> >  RC mode:
> >  - #address-cells: set to <3>
> >  - #size-cells: set to <2>
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found
  2019-08-12  8:34   ` Andrew Murray
@ 2019-08-13  3:13     ` Z.q. Hou
  0 siblings, 0 replies; 18+ messages in thread
From: Z.q. Hou @ 2019-08-13  3:13 UTC (permalink / raw)
  To: Andrew Murray
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

Hi Andrew,

Thanks a lot for your review!

B.R,
Zhiqiang

> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019Äê8ÔÂ12ÈÕ 16:34
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; gustavo.pimentel@synopsys.com;
> jingoohan1@gmail.com; bhelgaas@google.com; robh+dt@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> <leoyang.li@nxp.com>; lorenzo.pieralisi@arm.com; M.h. Lian
> <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not
> found
> 
> On Mon, Aug 12, 2019 at 04:22:22AM +0000, Z.q. Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The num-lanes is optional, so probably it isn't added on some
> > platforms. The subsequent programming is base on the num-lanes, hence
> > return when it is not found.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > b/drivers/pci/controller/dwc/pcie-designware.c
> > index 7d25102c304c..0a89bfd1636e 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -423,8 +423,10 @@ void dw_pcie_setup(struct dw_pcie *pci)
> >
> >
> >  	ret = of_property_read_u32(np, "num-lanes", &lanes);
> > -	if (ret)
> > -		lanes = 0;
> > +	if (ret) {
> > +		dev_dbg(pci->dev, "property num-lanes isn't found\n");
> > +		return;
> > +	}
> 
> The existing code would assign a value of 0 to lanes when num-lanes isn't
> specified, however this value isn't supported by the following switch
> statement - thus we'd also print an error and return.
> 
> Therefore the only and subtle effect effect of this patch is to change a
> dev_err to a dev_dbg when num-lanes isn't specified and avoid a read of
> PCIE_PORT_LINK_CONTROL.
> 
> Given that num-lanes is described as optional this makes perfect sense.
> Though the commit message/hunk does give the apperance that this
> provides a more functional change.
> 
> Anyway:
> 
> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> 
> 
> >
> >  	/* Set the number of lanes */
> >  	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes
  2019-08-12  8:35   ` Andrew Murray
@ 2019-08-13  3:13     ` Z.q. Hou
  0 siblings, 0 replies; 18+ messages in thread
From: Z.q. Hou @ 2019-08-13  3:13 UTC (permalink / raw)
  To: Andrew Murray
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

Hi Andrew,

Thanks a lot for your review!

Regards,
Zhiqiang

> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019Äê8ÔÂ12ÈÕ 16:35
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; gustavo.pimentel@synopsys.com;
> jingoohan1@gmail.com; bhelgaas@google.com; robh+dt@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> <leoyang.li@nxp.com>; lorenzo.pieralisi@arm.com; M.h. Lian
> <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property
> from PCIe nodes
> 
> On Mon, Aug 12, 2019 at 04:22:27AM +0000, Z.q. Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > On FSL Layerscape SoCs, the number of lanes assigned to PCIe
> > controller is not fixed, it is determined by the selected SerDes
> > protocol in the RCW (Reset Configuration Word), and the PCIe link
> > training is completed automatically base on the selected SerDes
> > protocol, and the link width set-up is updated by hardware. So the
> > num-lanes is not needed to specify the link width.
> >
> > The current num-lanes indicates the max lanes PCIe controller can
> > support up to, instead of the lanes assigned to the PCIe controller.
> > This can result in PCIe link training fail after hot-reset. So remove
> > the num-lanes to avoid set-up to incorrect link width.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  arch/arm/boot/dts/ls1021a.dtsi | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/ls1021a.dtsi
> > b/arch/arm/boot/dts/ls1021a.dtsi index 464df4290ffc..2f6977ada447
> > 100644
> > --- a/arch/arm/boot/dts/ls1021a.dtsi
> > +++ b/arch/arm/boot/dts/ls1021a.dtsi
> > @@ -874,7 +874,6 @@
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -899,7 +898,6 @@
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> 
> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> 
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 4/4] arm64: dts: fsl: Remove num-lanes property from PCIe nodes
  2019-08-12  8:35   ` Andrew Murray
@ 2019-08-13  3:14     ` Z.q. Hou
  0 siblings, 0 replies; 18+ messages in thread
From: Z.q. Hou @ 2019-08-13  3:14 UTC (permalink / raw)
  To: Andrew Murray
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

Hi Andrew,

Thanks a lot for your review!

Regards,
Zhiqiang

> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019Äê8ÔÂ12ÈÕ 16:36
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; gustavo.pimentel@synopsys.com;
> jingoohan1@gmail.com; bhelgaas@google.com; robh+dt@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> <leoyang.li@nxp.com>; lorenzo.pieralisi@arm.com; M.h. Lian
> <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 4/4] arm64: dts: fsl: Remove num-lanes property from
> PCIe nodes
> 
> On Mon, Aug 12, 2019 at 04:22:33AM +0000, Z.q. Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > On FSL Layerscape SoCs, the number of lanes assigned to PCIe
> > controller is not fixed, it is determined by the selected SerDes
> > protocol in the RCW (Reset Configuration Word), and the PCIe link
> > training is completed automatically base on the selected SerDes
> > protocol, and the link width set-up is updated by hardware. So the
> > num-lanes is not needed to specify the link width.
> >
> > The current num-lanes indicates the max lanes PCIe controller can
> > support up to, instead of the lanes assigned to the PCIe controller.
> > This can result in PCIe link training fail after hot-reset. So remove
> > the num-lanes to avoid set-up to incorrect link width.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 -
> > arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 3 ---
> > arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 6 ------
> > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 3 ---
> > arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ----
> >  5 files changed, 17 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> > index ec6257a5b251..119c597ca867 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> > @@ -486,7 +486,6 @@
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> > -			num-lanes = <4>;
> >  			num-viewport = <2>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> > index 71d9ed9ff985..c084c7a4b6a6 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> > @@ -677,7 +677,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -704,7 +703,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <2>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -731,7 +729,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <2>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x50 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > index b0ef08b090dd..d4c1da3d4bde 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > @@ -649,7 +649,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <4>;
> >  			num-viewport = <8>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -671,7 +670,6 @@
> >  			reg-names = "regs", "addr_space";
> >  			num-ib-windows = <6>;
> >  			num-ob-windows = <8>;
> > -			num-lanes = <2>;
> >  			status = "disabled";
> >  		};
> >
> > @@ -687,7 +685,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <2>;
> >  			num-viewport = <8>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -709,7 +706,6 @@
> >  			reg-names = "regs", "addr_space";
> >  			num-ib-windows = <6>;
> >  			num-ob-windows = <8>;
> > -			num-lanes = <2>;
> >  			status = "disabled";
> >  		};
> >
> > @@ -725,7 +721,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <2>;
> >  			num-viewport = <8>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x50 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -747,7 +742,6 @@
> >  			reg-names = "regs", "addr_space";
> >  			num-ib-windows = <6>;
> >  			num-ob-windows = <8>;
> > -			num-lanes = <2>;
> >  			status = "disabled";
> >  		};
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > index dfbead405783..76c87afeba1e 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> > @@ -456,7 +456,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <4>;
> >  			num-viewport = <256>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x20 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -482,7 +481,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x28 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -508,7 +506,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <8>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x30 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> > index 64101c9962ce..7a0be8eaa84a 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> > @@ -639,7 +639,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			msi-parent = <&its>;
> > @@ -661,7 +660,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			msi-parent = <&its>;
> > @@ -683,7 +681,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <8>;
> >  			num-viewport = <256>;
> >  			bus-range = <0x0 0xff>;
> >  			msi-parent = <&its>;
> > @@ -705,7 +702,6 @@
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> >  			dma-coherent;
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			msi-parent = <&its>;
> 
> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> 
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties
  2019-08-13  3:07     ` Z.q. Hou
@ 2019-08-13  4:35       ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2019-08-13  4:35 UTC (permalink / raw)
  To: Z.q. Hou, Andrew Murray
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, bhelgaas, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian, Gabriele Paoloni

Hi,

On 13/08/19 8:37 AM, Z.q. Hou wrote:
> Hi Andrew,
> 
> Thanks a lot for your comments!
> 
>> -----Original Message-----
>> From: Andrew Murray <andrew.murray@arm.com>
>> Sent: 2019Äê8ÔÂ12ÈÕ 16:45
>> To: Z.q. Hou <zhiqiang.hou@nxp.com>
>> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
>> linux-kernel@vger.kernel.org; gustavo.pimentel@synopsys.com;
>> jingoohan1@gmail.com; bhelgaas@google.com; robh+dt@kernel.org;
>> mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
>> <leoyang.li@nxp.com>; lorenzo.pieralisi@arm.com; M.h. Lian
>> <minghuan.lian@nxp.com>; Kishon Vijay Abraham I <kishon@ti.com>;
>> Gabriele Paoloni <gabriele.paoloni@huawei.com>
>> Subject: Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from
>> Required properties
>>
>> On Mon, Aug 12, 2019 at 04:22:16AM +0000, Z.q. Hou wrote:
>>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>>
>>> The num-lanes is not a mandatory property, e.g. on FSL Layerscape
>>> SoCs, the PCIe link training is completed automatically base on the
>>> selected SerDes protocol, it doesn't need the num-lanes to set-up the
>>> link width.
>>>
>>> It has been added in the Optional properties. This patch is to remove
>>> it from the Required properties.
>>
>> For clarity, maybe this paragraph can be reworded to:
>>
>> "It is previously in both Required and Optional properties,  let's remove it
>> from the Required properties".
> 
> Agree and will change in v2.
> 
>>
>> I don't understand why this property is previously in both required and
>> optional...
>>
>> It looks like num-lanes was first made optional back in
>> 2015 and removed from the Required section (907fce090253).
>> But then re-added back into the Required section in 2017 with the adition of
>> bindings for EP mode (b12befecd7de).
>>
>> Is num-lanes actually required for EP mode?
> 
> Kishon, please help to answer this query?

It should be optional for EP too.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties
  2019-08-12  4:22 ` [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties Z.q. Hou
  2019-08-12  8:45   ` Andrew Murray
@ 2019-08-19 19:20   ` Bjorn Helgaas
  2019-08-19 23:57     ` Z.q. Hou
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2019-08-19 19:20 UTC (permalink / raw)
  To: Z.q. Hou
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

In subject:

  s/dt-bingings/dt-bindings/

Also, possibly

  s/PCI:/PCI: designware:/

since this only applies to designware-pcie.txt.

On Mon, Aug 12, 2019 at 04:22:16AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> The num-lanes is not a mandatory property, e.g. on FSL
> Layerscape SoCs, the PCIe link training is completed
> automatically base on the selected SerDes protocol, it
> doesn't need the num-lanes to set-up the link width.
> 
> It has been added in the Optional properties. This
> patch is to remove it from the Required properties.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 5561a1c060d0..bd880df39a79 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -11,7 +11,6 @@ Required properties:
>  	     the ATU address space.
>      (The old way of getting the configuration address space from "ranges"
>      is deprecated and should be avoided.)
> -- num-lanes: number of lanes to use
>  RC mode:
>  - #address-cells: set to <3>
>  - #size-cells: set to <2>
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes
  2019-08-12  4:22 ` [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes Z.q. Hou
  2019-08-12  8:35   ` Andrew Murray
@ 2019-08-19 19:24   ` Bjorn Helgaas
  2019-08-20  0:12     ` Z.q. Hou
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2019-08-19 19:24 UTC (permalink / raw)
  To: Z.q. Hou
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

On Mon, Aug 12, 2019 at 04:22:27AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> On FSL Layerscape SoCs, the number of lanes assigned to PCIe
> controller is not fixed, it is determined by the selected
> SerDes protocol in the RCW (Reset Configuration Word), and
> the PCIe link training is completed automatically base on
> the selected SerDes protocol, and the link width set-up is
> updated by hardware. So the num-lanes is not needed to
> specify the link width.
> 
> The current num-lanes indicates the max lanes PCIe controller
> can support up to, instead of the lanes assigned to the PCIe
> controller. This can result in PCIe link training fail after
> hot-reset. So remove the num-lanes to avoid set-up to incorrect
> link width.

It would be useful to explain *why* "num-lanes" in DT causes a link
training failure.  Maybe the code programs "num-lanes" somewhere,
overriding what hardware automatically sensed?  Maybe the code tries
to bring up exactly "num-lanes" lanes even if not all are present?

> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  arch/arm/boot/dts/ls1021a.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
> index 464df4290ffc..2f6977ada447 100644
> --- a/arch/arm/boot/dts/ls1021a.dtsi
> +++ b/arch/arm/boot/dts/ls1021a.dtsi
> @@ -874,7 +874,6 @@
>  			#address-cells = <3>;
>  			#size-cells = <2>;
>  			device_type = "pci";
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
> @@ -899,7 +898,6 @@
>  			#address-cells = <3>;
>  			#size-cells = <2>;
>  			device_type = "pci";
> -			num-lanes = <4>;
>  			num-viewport = <6>;
>  			bus-range = <0x0 0xff>;
>  			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000   /* downstream I/O */
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties
  2019-08-19 19:20   ` Bjorn Helgaas
@ 2019-08-19 23:57     ` Z.q. Hou
  0 siblings, 0 replies; 18+ messages in thread
From: Z.q. Hou @ 2019-08-19 23:57 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

Hi Bjorn,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 2019Äê8ÔÂ20ÈÕ 3:20
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; gustavo.pimentel@synopsys.com;
> jingoohan1@gmail.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>;
> lorenzo.pieralisi@arm.com; M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from
> Required properties
> 
> In subject:
> 
>   s/dt-bingings/dt-bindings/
> 
> Also, possibly
> 
>   s/PCI:/PCI: designware:/
> 

I'll fix them in v2.

Thanks,
Zhiqiang
> since this only applies to designware-pcie.txt.
> 
> On Mon, Aug 12, 2019 at 04:22:16AM +0000, Z.q. Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The num-lanes is not a mandatory property, e.g. on FSL Layerscape
> > SoCs, the PCIe link training is completed automatically base on the
> > selected SerDes protocol, it doesn't need the num-lanes to set-up the
> > link width.
> >
> > It has been added in the Optional properties. This patch is to remove
> > it from the Required properties.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/pci/designware-pcie.txt | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > index 5561a1c060d0..bd880df39a79 100644
> > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > @@ -11,7 +11,6 @@ Required properties:
> >  	     the ATU address space.
> >      (The old way of getting the configuration address space from "ranges"
> >      is deprecated and should be avoided.)
> > -- num-lanes: number of lanes to use
> >  RC mode:
> >  - #address-cells: set to <3>
> >  - #size-cells: set to <2>
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes
  2019-08-19 19:24   ` Bjorn Helgaas
@ 2019-08-20  0:12     ` Z.q. Hou
  0 siblings, 0 replies; 18+ messages in thread
From: Z.q. Hou @ 2019-08-20  0:12 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, devicetree, linux-kernel, gustavo.pimentel,
	jingoohan1, robh+dt, mark.rutland, shawnguo, Leo Li,
	lorenzo.pieralisi, M.h. Lian

Hi Bjorn,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 2019Äê8ÔÂ20ÈÕ 3:25
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; gustavo.pimentel@synopsys.com;
> jingoohan1@gmail.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>;
> lorenzo.pieralisi@arm.com; M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property
> from PCIe nodes
> 
> On Mon, Aug 12, 2019 at 04:22:27AM +0000, Z.q. Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > On FSL Layerscape SoCs, the number of lanes assigned to PCIe
> > controller is not fixed, it is determined by the selected SerDes
> > protocol in the RCW (Reset Configuration Word), and the PCIe link
> > training is completed automatically base on the selected SerDes
> > protocol, and the link width set-up is updated by hardware. So the
> > num-lanes is not needed to specify the link width.
> >
> > The current num-lanes indicates the max lanes PCIe controller can
> > support up to, instead of the lanes assigned to the PCIe controller.
> > This can result in PCIe link training fail after hot-reset. So remove
> > the num-lanes to avoid set-up to incorrect link width.
> 
> It would be useful to explain *why* "num-lanes" in DT causes a link training
> failure.  Maybe the code programs "num-lanes" somewhere, overriding what
> hardware automatically sensed?  Maybe the code tries to bring up exactly
> "num-lanes" lanes even if not all are present?

Will add in v2.
As the Layerscape PCIe controller link training is completed automatically during
the power on reset. It doesn't need software to bring up. So I think it should be the
former.

Thanks,
Zhiqiang

> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  arch/arm/boot/dts/ls1021a.dtsi | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/ls1021a.dtsi
> > b/arch/arm/boot/dts/ls1021a.dtsi index 464df4290ffc..2f6977ada447
> > 100644
> > --- a/arch/arm/boot/dts/ls1021a.dtsi
> > +++ b/arch/arm/boot/dts/ls1021a.dtsi
> > @@ -874,7 +874,6 @@
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > @@ -899,7 +898,6 @@
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> >  			device_type = "pci";
> > -			num-lanes = <4>;
> >  			num-viewport = <6>;
> >  			bus-range = <0x0 0xff>;
> >  			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0
> 0x00010000   /* downstream I/O */
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-12  4:22 [PATCH 0/4] Layerscape: Remove num-lanes property from PCIe nodes Z.q. Hou
2019-08-12  4:22 ` [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties Z.q. Hou
2019-08-12  8:45   ` Andrew Murray
2019-08-13  3:07     ` Z.q. Hou
2019-08-13  4:35       ` Kishon Vijay Abraham I
2019-08-19 19:20   ` Bjorn Helgaas
2019-08-19 23:57     ` Z.q. Hou
2019-08-12  4:22 ` [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found Z.q. Hou
2019-08-12  8:34   ` Andrew Murray
2019-08-13  3:13     ` Z.q. Hou
2019-08-12  4:22 ` [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes Z.q. Hou
2019-08-12  8:35   ` Andrew Murray
2019-08-13  3:13     ` Z.q. Hou
2019-08-19 19:24   ` Bjorn Helgaas
2019-08-20  0:12     ` Z.q. Hou
2019-08-12  4:22 ` [PATCH 4/4] arm64: dts: fsl: " Z.q. Hou
2019-08-12  8:35   ` Andrew Murray
2019-08-13  3:14     ` Z.q. Hou

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org linux-pci@archiver.kernel.org
	public-inbox-index linux-pci


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox