| Message ID | 20240531-x1e80100-dts-fixes-pcie6a-v1-2-1573ebcae1e8@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: x1e80100: Fix the PCIe 6a node | expand |
On Fri, May 31, 2024 at 08:00:32PM +0300, Abel Vesa wrote: > So the PCIe 6 can be configured in 4-lane mode or 2-lane mode. For > 4-lane mode, it fetches the lanes provided by PCIe 6b. For 2-lane mode, > PCIe 6a uses 2 lanes and then PCIe 6b uses the other 2 lanes. Configure > it in 4-lane mode and then each board can configure it depending on the > design. Both the QCP and CRD boards, currently upstream, use the 6a for > NVMe in 4-lane mode. Also, mark the controller as 4-lane as well. > > Fixes: 5eb83fc10289 ("arm64: dts: qcom: x1e80100: Add PCIe nodes") > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > arch/arm64/boot/dts/qcom/x1e80100.dtsi | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > index fe7ca2a73f9d..17e4c5cda22d 100644 > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi > @@ -2838,7 +2838,7 @@ pcie6a: pci@1bf8000 { > dma-coherent; > > linux,pci-domain = <7>; > - num-lanes = <2>; > + num-lanes = <4>; > > interrupts = <GIC_SPI 773 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 774 IRQ_TYPE_LEVEL_HIGH>, > @@ -2903,19 +2903,21 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, > }; > > pcie6a_phy: phy@1bfc000 { > - compatible = "qcom,x1e80100-qmp-gen4x2-pcie-phy"; > - reg = <0 0x01bfc000 0 0x2000>; > + compatible = "qcom,x1e80100-qmp-gen4x4-pcie-phy"; > + reg = <0 0x01bfc000 0 0x2000>, > + <0 0x01bfe000 0 0x2000>; > > clocks = <&gcc GCC_PCIE_6A_PHY_AUX_CLK>, > <&gcc GCC_PCIE_6A_CFG_AHB_CLK>, > <&rpmhcc RPMH_CXO_CLK>, > <&gcc GCC_PCIE_6A_PHY_RCHNG_CLK>, > - <&gcc GCC_PCIE_6A_PIPE_CLK>; > + <&gcc GCC_PCIE_6A_PIPEDIV2_CLK>; > clock-names = "aux", > "cfg_ahb", > "ref", > "rchng", > - "pipe"; > + "pipe", > + "pipediv2"; I see 5 clocks and 6 clock-names here. > > resets = <&gcc GCC_PCIE_6A_PHY_BCR>, > <&gcc GCC_PCIE_6A_NOCSR_COM_PHY_BCR>; > @@ -2927,6 +2929,8 @@ pcie6a_phy: phy@1bfc000 { > > power-domains = <&gcc GCC_PCIE_6_PHY_GDSC>; > > + qcom,4ln-config-sel = <&tcsr 0x1a000 0>; > + > #clock-cells = <0>; > clock-output-names = "pcie6a_pipe_clk"; > > > -- > 2.34.1 >
On 6/3/24 14:52, Johan Hovold wrote: [...] > > As I just mentioned in my reply on the PHY patch, this does not seem to > work on the CRD were the link still come up as 2-lane (also with the > clocks fixed): > > qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up > > So something appears to be wrong here or in the PHY changes. Is the device on the other end x4-capable? Or does it not matter in this log line? Konrad
On Tue, Jun 04, 2024 at 02:00:10PM +0200, Konrad Dybcio wrote: > On 6/3/24 14:52, Johan Hovold wrote: > > As I just mentioned in my reply on the PHY patch, this does not seem to > > work on the CRD were the link still come up as 2-lane (also with the > > clocks fixed): > > > > qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up > > > > So something appears to be wrong here or in the PHY changes. > > Is the device on the other end x4-capable? Or does it not matter in > this log line? Yes, of course. It's the CRD as I wrote above, and you can tell from other log entries: pci 0007:01:00.0: 31.506 Gb/s available PCIe bandwidth, limited by 16.0 GT/s PCIe x2 link at 0007:00:00.0 (capable of 63.012 Gb/s with 16.0 GT/s PCIe x4 link) lspci and what Windows reports. Johan
On 6/4/24 14:20, Johan Hovold wrote: > On Tue, Jun 04, 2024 at 02:00:10PM +0200, Konrad Dybcio wrote: >> On 6/3/24 14:52, Johan Hovold wrote: > >>> As I just mentioned in my reply on the PHY patch, this does not seem to >>> work on the CRD were the link still come up as 2-lane (also with the >>> clocks fixed): >>> >>> qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up >>> >>> So something appears to be wrong here or in the PHY changes. >> >> Is the device on the other end x4-capable? Or does it not matter in >> this log line? > > Yes, of course. It's the CRD as I wrote above, and you can tell from > other log entries: > > pci 0007:01:00.0: 31.506 Gb/s available PCIe bandwidth, limited by 16.0 GT/s PCIe x2 link at 0007:00:00.0 (capable of 63.012 Gb/s with 16.0 GT/s PCIe x4 link) > > lspci and what Windows reports. Ok, good. I was scared of double-sourcing of parts that are not identical in spec.. [1] suggests this wasn't ever achieved.. which makes the cover letter of this series a bit misleading.. What does the TCSR check return? If 0, can you hardcode it to 1 and see if the link comes up at x4? Konrad [1] https://lore.kernel.org/all/Zl8H0KOrfuF91kpZ@linaro.org/
On 24-06-04 14:38:40, Konrad Dybcio wrote: > > > On 6/4/24 14:20, Johan Hovold wrote: > > On Tue, Jun 04, 2024 at 02:00:10PM +0200, Konrad Dybcio wrote: > > > On 6/3/24 14:52, Johan Hovold wrote: > > > > > > As I just mentioned in my reply on the PHY patch, this does not seem to > > > > work on the CRD were the link still come up as 2-lane (also with the > > > > clocks fixed): > > > > > > > > qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up > > > > > > > > So something appears to be wrong here or in the PHY changes. > > > > > > Is the device on the other end x4-capable? Or does it not matter in > > > this log line? > > > > Yes, of course. It's the CRD as I wrote above, and you can tell from > > other log entries: > > > > pci 0007:01:00.0: 31.506 Gb/s available PCIe bandwidth, limited by 16.0 GT/s PCIe x2 link at 0007:00:00.0 (capable of 63.012 Gb/s with 16.0 GT/s PCIe x4 link) > > > > lspci and what Windows reports. > Ok, good. I was scared of double-sourcing of parts that are not identical > in spec.. > On my CRD, there is a KBG50ZNS256G. > [1] suggests this wasn't ever achieved.. which makes the cover letter of > this series a bit misleading.. True ... > > What does the TCSR check return? If 0, can you hardcode it to 1 and see if > the link comes up at x4? TCSR check returns 1. But that is not enough. The PCIe controller needs to handles some stuff about margining. See the following patchset. https://lore.kernel.org/linux-pci/20240501163610.8900-3-quic_schintav@quicinc.com/ But even with this, I'm not able to get 4-lanes mode to work (yet). So it must be something else in the controller driver that is needed. IIRC, this is the first Qualcomm platform that would support Gen4 with 4-lanes upstream. Maybe I'm wrong. > > Konrad > > [1] https://lore.kernel.org/all/Zl8H0KOrfuF91kpZ@linaro.org/
On 6/4/24 14:45, Abel Vesa wrote: > On 24-06-04 14:38:40, Konrad Dybcio wrote: >> >> >> On 6/4/24 14:20, Johan Hovold wrote: >>> On Tue, Jun 04, 2024 at 02:00:10PM +0200, Konrad Dybcio wrote: >>>> On 6/3/24 14:52, Johan Hovold wrote: >>> >>>>> As I just mentioned in my reply on the PHY patch, this does not seem to >>>>> work on the CRD were the link still come up as 2-lane (also with the >>>>> clocks fixed): >>>>> >>>>> qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up >>>>> >>>>> So something appears to be wrong here or in the PHY changes. >>>> >>>> Is the device on the other end x4-capable? Or does it not matter in >>>> this log line? >>> >>> Yes, of course. It's the CRD as I wrote above, and you can tell from >>> other log entries: >>> >>> pci 0007:01:00.0: 31.506 Gb/s available PCIe bandwidth, limited by 16.0 GT/s PCIe x2 link at 0007:00:00.0 (capable of 63.012 Gb/s with 16.0 GT/s PCIe x4 link) >>> >>> lspci and what Windows reports. >> Ok, good. I was scared of double-sourcing of parts that are not identical >> in spec.. >> > > On my CRD, there is a KBG50ZNS256G. > >> [1] suggests this wasn't ever achieved.. which makes the cover letter of >> this series a bit misleading.. > > True ... > >> >> What does the TCSR check return? If 0, can you hardcode it to 1 and see if >> the link comes up at x4? > > TCSR check returns 1. But that is not enough. The PCIe controller needs to > handles some stuff about margining. See the following patchset. > > https://lore.kernel.org/linux-pci/20240501163610.8900-3-quic_schintav@quicinc.com/ > > But even with this, I'm not able to get 4-lanes mode to work (yet). > So it must be something else in the controller driver that is needed. The margining settings AFAIU shouldn't be necessary for just getting the link, but to ensure there aren't many errors while transacting.. > > IIRC, this is the first Qualcomm platform that would support Gen4 with > 4-lanes upstream. Maybe I'm wrong. Seems so Another idea I had, maybe the PCIE_PORT_LINK_CONTROL & PCIE_LINK_WIDTH_SPEED_CONTROL registers differ on qcom gen4 controllers.. Can you check the documentation and see if the defines in drivers/pci/controller/dwc/pcie-designware.h still hold true? Konrad
On 24-06-04 14:55:31, Konrad Dybcio wrote: > > > On 6/4/24 14:45, Abel Vesa wrote: > > On 24-06-04 14:38:40, Konrad Dybcio wrote: > > > > > > > > > On 6/4/24 14:20, Johan Hovold wrote: > > > > On Tue, Jun 04, 2024 at 02:00:10PM +0200, Konrad Dybcio wrote: > > > > > On 6/3/24 14:52, Johan Hovold wrote: > > > > > > > > > > As I just mentioned in my reply on the PHY patch, this does not seem to > > > > > > work on the CRD were the link still come up as 2-lane (also with the > > > > > > clocks fixed): > > > > > > > > > > > > qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up > > > > > > > > > > > > So something appears to be wrong here or in the PHY changes. > > > > > > > > > > Is the device on the other end x4-capable? Or does it not matter in > > > > > this log line? > > > > > > > > Yes, of course. It's the CRD as I wrote above, and you can tell from > > > > other log entries: > > > > > > > > pci 0007:01:00.0: 31.506 Gb/s available PCIe bandwidth, limited by 16.0 GT/s PCIe x2 link at 0007:00:00.0 (capable of 63.012 Gb/s with 16.0 GT/s PCIe x4 link) > > > > > > > > lspci and what Windows reports. > > > Ok, good. I was scared of double-sourcing of parts that are not identical > > > in spec.. > > > > > > > On my CRD, there is a KBG50ZNS256G. > > > > > [1] suggests this wasn't ever achieved.. which makes the cover letter of > > > this series a bit misleading.. > > > > True ... > > > > > > > > What does the TCSR check return? If 0, can you hardcode it to 1 and see if > > > the link comes up at x4? > > > > TCSR check returns 1. But that is not enough. The PCIe controller needs to > > handles some stuff about margining. See the following patchset. > > > > https://lore.kernel.org/linux-pci/20240501163610.8900-3-quic_schintav@quicinc.com/ > > > > But even with this, I'm not able to get 4-lanes mode to work (yet). > > So it must be something else in the controller driver that is needed. > > The margining settings AFAIU shouldn't be necessary for just getting the > link, but to ensure there aren't many errors while transacting.. Part of the margining settings is related number of lanes. > > > > > IIRC, this is the first Qualcomm platform that would support Gen4 with > > 4-lanes upstream. Maybe I'm wrong. > > Seems so > > Another idea I had, maybe the PCIE_PORT_LINK_CONTROL & > PCIE_LINK_WIDTH_SPEED_CONTROL registers differ on qcom gen4 controllers.. > > Can you check the documentation and see if the defines in > drivers/pci/controller/dwc/pcie-designware.h still hold true? Yep. No issue there. > > Konrad
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi index fe7ca2a73f9d..17e4c5cda22d 100644 --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi @@ -2838,7 +2838,7 @@ pcie6a: pci@1bf8000 { dma-coherent; linux,pci-domain = <7>; - num-lanes = <2>; + num-lanes = <4>; interrupts = <GIC_SPI 773 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 774 IRQ_TYPE_LEVEL_HIGH>, @@ -2903,19 +2903,21 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, }; pcie6a_phy: phy@1bfc000 { - compatible = "qcom,x1e80100-qmp-gen4x2-pcie-phy"; - reg = <0 0x01bfc000 0 0x2000>; + compatible = "qcom,x1e80100-qmp-gen4x4-pcie-phy"; + reg = <0 0x01bfc000 0 0x2000>, + <0 0x01bfe000 0 0x2000>; clocks = <&gcc GCC_PCIE_6A_PHY_AUX_CLK>, <&gcc GCC_PCIE_6A_CFG_AHB_CLK>, <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_PCIE_6A_PHY_RCHNG_CLK>, - <&gcc GCC_PCIE_6A_PIPE_CLK>; + <&gcc GCC_PCIE_6A_PIPEDIV2_CLK>; clock-names = "aux", "cfg_ahb", "ref", "rchng", - "pipe"; + "pipe", + "pipediv2"; resets = <&gcc GCC_PCIE_6A_PHY_BCR>, <&gcc GCC_PCIE_6A_NOCSR_COM_PHY_BCR>; @@ -2927,6 +2929,8 @@ pcie6a_phy: phy@1bfc000 { power-domains = <&gcc GCC_PCIE_6_PHY_GDSC>; + qcom,4ln-config-sel = <&tcsr 0x1a000 0>; + #clock-cells = <0>; clock-output-names = "pcie6a_pipe_clk";
So the PCIe 6 can be configured in 4-lane mode or 2-lane mode. For 4-lane mode, it fetches the lanes provided by PCIe 6b. For 2-lane mode, PCIe 6a uses 2 lanes and then PCIe 6b uses the other 2 lanes. Configure it in 4-lane mode and then each board can configure it depending on the design. Both the QCP and CRD boards, currently upstream, use the 6a for NVMe in 4-lane mode. Also, mark the controller as 4-lane as well. Fixes: 5eb83fc10289 ("arm64: dts: qcom: x1e80100: Add PCIe nodes") Signed-off-by: Abel Vesa <abel.vesa@linaro.org> --- arch/arm64/boot/dts/qcom/x1e80100.dtsi | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)