[PATCH 0/4] Layerscape: Remove num-lanes property from PCIe nodes

[PATCH 0/4] Layerscape: Remove num-lanes property from PCIe nodes

[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

[PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties

[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

[PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found

[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

[PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes

[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

[PATCH 4/4] arm64: dts: fsl: Remove num-lanes property from PCIe nodes

[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

Re: [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found

[Andrew Murray]

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
> 

Re: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes

[Andrew Murray]

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
> 

Re: [PATCH 4/4] arm64: dts: fsl: Remove num-lanes property from PCIe nodes

[Andrew Murray]

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
> 

Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties

[Andrew Murray]

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
> 

RE: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties

[Z.q. Hou]

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
> >

RE: [PATCH 2/4] PCI: dwc: Return directly when num-lanes is not found

[Z.q. Hou]

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
> >

RE: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes

[Z.q. Hou]

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
> >

RE: [PATCH 4/4] arm64: dts: fsl: Remove num-lanes property from PCIe nodes

[Z.q. Hou]

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
> >

Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties

[Kishon Vijay Abraham I]

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

Re: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties

[Bjorn Helgaas]

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
> 

Re: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes

[Bjorn Helgaas]

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
> 

RE: [PATCH 1/4] dt-bingings: PCI: Remove the num-lanes from Required properties

[Z.q. Hou]

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
> >

RE: [PATCH 3/4] ARM: dts: ls1021a: Remove num-lanes property from PCIe nodes

[Z.q. Hou]

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
> >