From: Hou Zhiqiang <[email protected]>
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
From: Hou Zhiqiang <[email protected]>
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 <[email protected]>
---
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
From: Hou Zhiqiang <[email protected]>
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 <[email protected]>
---
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
From: Hou Zhiqiang <[email protected]>
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 <[email protected]>
---
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
From: Hou Zhiqiang <[email protected]>
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 <[email protected]>
---
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
On Mon, Aug 12, 2019 at 04:22:22AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
>
> /* Set the number of lanes */
> val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> --
> 2.17.1
>
On Mon, Aug 12, 2019 at 04:22:27AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
> --
> 2.17.1
>
On Mon, Aug 12, 2019 at 04:22:33AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
> --
> 2.17.1
>
On Mon, Aug 12, 2019 at 04:22:16AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
Hi Andrew,
Thanks a lot for your review!
B.R,
Zhiqiang
> -----Original Message-----
> From: Andrew Murray <[email protected]>
> Sent: 2019??8??12?? 16:34
> To: Z.q. Hou <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; M.h. Lian
> <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
>
> >
> > /* Set the number of lanes */
> > val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> > --
> > 2.17.1
> >
Hi Andrew,
Thanks a lot for your review!
Regards,
Zhiqiang
> -----Original Message-----
> From: Andrew Murray <[email protected]>
> Sent: 2019??8??12?? 16:35
> To: Z.q. Hou <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; M.h. Lian
> <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
> > --
> > 2.17.1
> >
Hi Andrew,
Thanks a lot for your comments!
> -----Original Message-----
> From: Andrew Murray <[email protected]>
> Sent: 2019??8??12?? 16:45
> To: Z.q. Hou <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; M.h. Lian
> <[email protected]>; Kishon Vijay Abraham I <[email protected]>;
> Gabriele Paoloni <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
> >
Hi Andrew,
Thanks a lot for your review!
Regards,
Zhiqiang
> -----Original Message-----
> From: Andrew Murray <[email protected]>
> Sent: 2019??8??12?? 16:36
> To: Z.q. Hou <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; M.h. Lian
> <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
> > --
> > 2.17.1
> >
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 <[email protected]>
>> Sent: 2019??8??12?? 16:45
>> To: Z.q. Hou <[email protected]>
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; Leo Li
>> <[email protected]>; [email protected]; M.h. Lian
>> <[email protected]>; Kishon Vijay Abraham I <[email protected]>;
>> Gabriele Paoloni <[email protected]>
>> 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 <[email protected]>
>>>
>>> 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
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 <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
On Mon, Aug 12, 2019 at 04:22:27AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
Hi Bjorn,
Thanks a lot for your comments!
> -----Original Message-----
> From: Bjorn Helgaas [mailto:[email protected]]
> Sent: 2019??8??20?? 3:20
> To: Z.q. Hou <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Leo Li <[email protected]>;
> [email protected]; M.h. Lian <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
> >
Hi Bjorn,
Thanks a lot for your comments!
> -----Original Message-----
> From: Bjorn Helgaas [mailto:[email protected]]
> Sent: 2019??8??20?? 3:25
> To: Z.q. Hou <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Leo Li <[email protected]>;
> [email protected]; M.h. Lian <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
> >