| Message ID | 20241101030902.579789-6-quic_qianyu@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add support for PCIe3 on x1e80100 | expand |
On Thu, Oct 31, 2024 at 08:09:02PM -0700, Qiang Yu wrote: > Describe PCIe3 controller and PHY. Also add required system resources like > regulators, clocks, interrupts and registers configuration for PCIe3. > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Reviewed-by: Johan Hovold <johan+linaro@kernel.org> > + pcie3: pcie@1bd0000 { > + device_type = "pci"; > + compatible = "qcom,pcie-x1e80100"; > + reg = <0x0 0x01bd0000 0x0 0x3000>, > + <0x0 0x78000000 0x0 0xf1d>, > + <0x0 0x78000f40 0x0 0xa8>, > + <0x0 0x78001000 0x0 0x1000>, > + <0x0 0x78100000 0x0 0x100000>, > + <0x0 0x01bd3000 0x0 0x1000>; > + reg-names = "parf", > + "dbi", > + "elbi", > + "atu", > + "config", > + "mhi"; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 0x100000>, > + <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 0x3d00000>, Can you double check the size here so that it is indeed correct and not just copied from the other nodes which initially got it wrong: https://lore.kernel.org/lkml/20240710-topic-barman-v1-1-5f63fca8d0fc@linaro.org/ > + <0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 0x40000000>; > + bus-range = <0x00 0xff>; > + clocks = <&gcc GCC_PCIE_3_AUX_CLK>, > + <&gcc GCC_PCIE_3_CFG_AHB_CLK>, > + <&gcc GCC_PCIE_3_MSTR_AXI_CLK>, > + <&gcc GCC_PCIE_3_SLV_AXI_CLK>, > + <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>, > + <&gcc GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK>, > + <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>; > + clock-names = "aux", > + "cfg", > + "bus_master", > + "bus_slave", > + "slave_q2a", > + "noc_aggr", > + "cnoc_sf_axi"; > + > + assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>; > + assigned-clock-rates = <19200000>; > + > + interconnects = <&pcie_south_anoc MASTER_PCIE_3 QCOM_ICC_TAG_ALWAYS This should be &pcie_north_anoc > + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, > + <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS > + &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>; > + interconnect-names = "pcie-mem", > + "cpu-pcie"; With the above addressed, feel free to keep my Reviewed-by tag. Johan
On 11/4/2024 10:35 PM, Johan Hovold wrote: > On Thu, Oct 31, 2024 at 08:09:02PM -0700, Qiang Yu wrote: >> Describe PCIe3 controller and PHY. Also add required system resources like >> regulators, clocks, interrupts and registers configuration for PCIe3. >> >> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> > >> + pcie3: pcie@1bd0000 { >> + device_type = "pci"; >> + compatible = "qcom,pcie-x1e80100"; >> + reg = <0x0 0x01bd0000 0x0 0x3000>, >> + <0x0 0x78000000 0x0 0xf1d>, >> + <0x0 0x78000f40 0x0 0xa8>, >> + <0x0 0x78001000 0x0 0x1000>, >> + <0x0 0x78100000 0x0 0x100000>, >> + <0x0 0x01bd3000 0x0 0x1000>; >> + reg-names = "parf", >> + "dbi", >> + "elbi", >> + "atu", >> + "config", >> + "mhi"; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 0x100000>, >> + <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 0x3d00000>, > Can you double check the size here so that it is indeed correct and not > just copied from the other nodes which initially got it wrong: > > https://lore.kernel.org/lkml/20240710-topic-barman-v1-1-5f63fca8d0fc@linaro.org/ From memory maps, region of PCIe3 is 64MB, the size here is correct. Thanks, Qiang Yu > >> + <0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 0x40000000>; >> + bus-range = <0x00 0xff>; >> + clocks = <&gcc GCC_PCIE_3_AUX_CLK>, >> + <&gcc GCC_PCIE_3_CFG_AHB_CLK>, >> + <&gcc GCC_PCIE_3_MSTR_AXI_CLK>, >> + <&gcc GCC_PCIE_3_SLV_AXI_CLK>, >> + <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>, >> + <&gcc GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK>, >> + <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>; >> + clock-names = "aux", >> + "cfg", >> + "bus_master", >> + "bus_slave", >> + "slave_q2a", >> + "noc_aggr", >> + "cnoc_sf_axi"; >> + >> + assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>; >> + assigned-clock-rates = <19200000>; >> + >> + interconnects = <&pcie_south_anoc MASTER_PCIE_3 QCOM_ICC_TAG_ALWAYS > This should be &pcie_north_anoc > >> + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, >> + <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS >> + &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>; >> + interconnect-names = "pcie-mem", >> + "cpu-pcie"; > With the above addressed, feel free to keep my Reviewed-by tag. > > Johan
On 11/5/2024 1:28 PM, Qiang Yu wrote: > > On 11/4/2024 10:35 PM, Johan Hovold wrote: >> On Thu, Oct 31, 2024 at 08:09:02PM -0700, Qiang Yu wrote: >>> Describe PCIe3 controller and PHY. Also add required system >>> resources like >>> regulators, clocks, interrupts and registers configuration for PCIe3. >>> >>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >>> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> >>> + pcie3: pcie@1bd0000 { >>> + device_type = "pci"; >>> + compatible = "qcom,pcie-x1e80100"; >>> + reg = <0x0 0x01bd0000 0x0 0x3000>, >>> + <0x0 0x78000000 0x0 0xf1d>, >>> + <0x0 0x78000f40 0x0 0xa8>, >>> + <0x0 0x78001000 0x0 0x1000>, >>> + <0x0 0x78100000 0x0 0x100000>, >>> + <0x0 0x01bd3000 0x0 0x1000>; >>> + reg-names = "parf", >>> + "dbi", >>> + "elbi", >>> + "atu", >>> + "config", >>> + "mhi"; >>> + #address-cells = <3>; >>> + #size-cells = <2>; >>> + ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 >>> 0x100000>, >>> + <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 >>> 0x3d00000>, >> Can you double check the size here so that it is indeed correct and not >> just copied from the other nodes which initially got it wrong: >> >> https://lore.kernel.org/lkml/20240710-topic-barman-v1-1-5f63fca8d0fc@linaro.org/ BTW, regions of PCIe6a, PCIe4, PCIe5 are 64MB, 32MB, 32MB, respectively. Why range size is set to 0x1d00000 for PCIe6a, any issue is reported on PCIe6a? Thanks, Qiang Yu > From memory maps, region of PCIe3 is 64MB, the size here is correct. > > Thanks, > Qiang Yu >> >>> + <0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 >>> 0x40000000>; >>> + bus-range = <0x00 0xff>; >>> + clocks = <&gcc GCC_PCIE_3_AUX_CLK>, >>> + <&gcc GCC_PCIE_3_CFG_AHB_CLK>, >>> + <&gcc GCC_PCIE_3_MSTR_AXI_CLK>, >>> + <&gcc GCC_PCIE_3_SLV_AXI_CLK>, >>> + <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>, >>> + <&gcc GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK>, >>> + <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>; >>> + clock-names = "aux", >>> + "cfg", >>> + "bus_master", >>> + "bus_slave", >>> + "slave_q2a", >>> + "noc_aggr", >>> + "cnoc_sf_axi"; >>> + >>> + assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>; >>> + assigned-clock-rates = <19200000>; >>> + >>> + interconnects = <&pcie_south_anoc MASTER_PCIE_3 >>> QCOM_ICC_TAG_ALWAYS >> This should be &pcie_north_anoc >> >>> + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, >>> + <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS >>> + &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>; >>> + interconnect-names = "pcie-mem", >>> + "cpu-pcie"; >> With the above addressed, feel free to keep my Reviewed-by tag. >> >> Johan
On Mon, Nov 11, 2024 at 11:44:17AM +0800, Qiang Yu wrote: > On 11/5/2024 1:28 PM, Qiang Yu wrote: > > On 11/4/2024 10:35 PM, Johan Hovold wrote: > >> On Thu, Oct 31, 2024 at 08:09:02PM -0700, Qiang Yu wrote: > >>> + ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 > >>> 0x100000>, > >>> + <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 > >>> 0x3d00000>, > >> Can you double check the size here so that it is indeed correct and not > >> just copied from the other nodes which initially got it wrong: > >> > >> https://lore.kernel.org/lkml/20240710-topic-barman-v1-1-5f63fca8d0fc@linaro.org/ > BTW, regions of PCIe6a, PCIe4, PCIe5 are 64MB, 32MB, 32MB, respectively. > Why range size is set to 0x1d00000 for PCIe6a, any issue is reported on > PCIe6a? Thanks for checking. It seems the patch linked to above was broken for PCIe6a then. We did see PCIe5 probe breaking due to the overlap with PCIe4 but the patch predates PCIe5 support being posted and merged so it was probably just based on inspection. Could you send a fix for PCIe6a? Johan
On 11/13/2024 1:29 AM, Johan Hovold wrote: > On Mon, Nov 11, 2024 at 11:44:17AM +0800, Qiang Yu wrote: >> On 11/5/2024 1:28 PM, Qiang Yu wrote: >>> On 11/4/2024 10:35 PM, Johan Hovold wrote: >>>> On Thu, Oct 31, 2024 at 08:09:02PM -0700, Qiang Yu wrote: >>>>> + ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 >>>>> 0x100000>, >>>>> + <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 >>>>> 0x3d00000>, >>>> Can you double check the size here so that it is indeed correct and not >>>> just copied from the other nodes which initially got it wrong: >>>> >>>> https://lore.kernel.org/lkml/20240710-topic-barman-v1-1-5f63fca8d0fc@linaro.org/ >> BTW, regions of PCIe6a, PCIe4, PCIe5 are 64MB, 32MB, 32MB, respectively. >> Why range size is set to 0x1d00000 for PCIe6a, any issue is reported on >> PCIe6a? > Thanks for checking. It seems the patch linked to above was broken for > PCIe6a then. > > We did see PCIe5 probe breaking due to the overlap with PCIe4 but the > patch predates PCIe5 support being posted and merged so it was probably > just based on inspection. > > Could you send a fix for PCIe6a? Sure, will send the fix. Thanks, Qiang Yu > > Johan
On 13.11.2024 4:15 AM, Qiang Yu wrote: > > On 11/13/2024 1:29 AM, Johan Hovold wrote: >> On Mon, Nov 11, 2024 at 11:44:17AM +0800, Qiang Yu wrote: >>> On 11/5/2024 1:28 PM, Qiang Yu wrote: >>>> On 11/4/2024 10:35 PM, Johan Hovold wrote: >>>>> On Thu, Oct 31, 2024 at 08:09:02PM -0700, Qiang Yu wrote: >>>>>> + ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 >>>>>> 0x100000>, >>>>>> + <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 >>>>>> 0x3d00000>, >>>>> Can you double check the size here so that it is indeed correct and not >>>>> just copied from the other nodes which initially got it wrong: >>>>> >>>>> https://lore.kernel.org/lkml/20240710-topic-barman-v1-1-5f63fca8d0fc@linaro.org/ >>> BTW, regions of PCIe6a, PCIe4, PCIe5 are 64MB, 32MB, 32MB, respectively. >>> Why range size is set to 0x1d00000 for PCIe6a, any issue is reported on >>> PCIe6a? >> Thanks for checking. It seems the patch linked to above was broken for >> PCIe6a then. >> >> We did see PCIe5 probe breaking due to the overlap with PCIe4 but the >> patch predates PCIe5 support being posted and merged so it was probably >> just based on inspection. >> >> Could you send a fix for PCIe6a? > Sure, will send the fix. So the patch I posted made it match the DSDT/Windows state. I assumed there must have been something wrong as docs suggested the value that you did. But both work. In case any issues pop up, we can revisit this. Konrad
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi index 99f8bee10a38..c48fb505fb32 100644 --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi @@ -743,7 +743,7 @@ gcc: clock-controller@100000 { clocks = <&bi_tcxo_div2>, <&sleep_clk>, - <0>, + <&pcie3_phy>, <&pcie4_phy>, <&pcie5_phy>, <&pcie6a_phy>, @@ -2906,6 +2906,208 @@ mmss_noc: interconnect@1780000 { #interconnect-cells = <2>; }; + pcie3: pcie@1bd0000 { + device_type = "pci"; + compatible = "qcom,pcie-x1e80100"; + reg = <0x0 0x01bd0000 0x0 0x3000>, + <0x0 0x78000000 0x0 0xf1d>, + <0x0 0x78000f40 0x0 0xa8>, + <0x0 0x78001000 0x0 0x1000>, + <0x0 0x78100000 0x0 0x100000>, + <0x0 0x01bd3000 0x0 0x1000>; + reg-names = "parf", + "dbi", + "elbi", + "atu", + "config", + "mhi"; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x01000000 0x0 0x00000000 0x0 0x78200000 0x0 0x100000>, + <0x02000000 0x0 0x78300000 0x0 0x78300000 0x0 0x3d00000>, + <0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 0x40000000>; + bus-range = <0x00 0xff>; + + dma-coherent; + + linux,pci-domain = <3>; + num-lanes = <8>; + + interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi0", + "msi1", + "msi2", + "msi3", + "msi4", + "msi5", + "msi6", + "msi7", + "global"; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0x7>; + interrupt-map = <0 0 0 1 &intc 0 0 GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 2 &intc 0 0 GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 3 &intc 0 0 GIC_SPI 237 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 4 &intc 0 0 GIC_SPI 238 IRQ_TYPE_LEVEL_HIGH>; + + clocks = <&gcc GCC_PCIE_3_AUX_CLK>, + <&gcc GCC_PCIE_3_CFG_AHB_CLK>, + <&gcc GCC_PCIE_3_MSTR_AXI_CLK>, + <&gcc GCC_PCIE_3_SLV_AXI_CLK>, + <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>, + <&gcc GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK>, + <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>; + clock-names = "aux", + "cfg", + "bus_master", + "bus_slave", + "slave_q2a", + "noc_aggr", + "cnoc_sf_axi"; + + assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>; + assigned-clock-rates = <19200000>; + + interconnects = <&pcie_south_anoc MASTER_PCIE_3 QCOM_ICC_TAG_ALWAYS + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, + <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS + &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>; + interconnect-names = "pcie-mem", + "cpu-pcie"; + + resets = <&gcc GCC_PCIE_3_BCR>, + <&gcc GCC_PCIE_3_LINK_DOWN_BCR>; + reset-names = "pci", + "link_down"; + + power-domains = <&gcc GCC_PCIE_3_GDSC>; + + phys = <&pcie3_phy>; + phy-names = "pciephy"; + + operating-points-v2 = <&pcie3_opp_table>; + + status = "disabled"; + + pcie3_opp_table: opp-table { + compatible = "operating-points-v2"; + + /* GEN 1 x1 */ + opp-2500000 { + opp-hz = /bits/ 64 <2500000>; + required-opps = <&rpmhpd_opp_low_svs>; + opp-peak-kBps = <250000 1>; + }; + + /* GEN 1 x2 and GEN 2 x1 */ + opp-5000000 { + opp-hz = /bits/ 64 <5000000>; + required-opps = <&rpmhpd_opp_low_svs>; + opp-peak-kBps = <500000 1>; + }; + + /* GEN 1 x4 and GEN 2 x2 */ + opp-10000000 { + opp-hz = /bits/ 64 <10000000>; + required-opps = <&rpmhpd_opp_low_svs>; + opp-peak-kBps = <1000000 1>; + }; + + /* GEN 1 x8 and GEN 2 x4 */ + opp-20000000 { + opp-hz = /bits/ 64 <20000000>; + required-opps = <&rpmhpd_opp_low_svs>; + opp-peak-kBps = <2000000 1>; + }; + + /* GEN 2 x8 */ + opp-40000000 { + opp-hz = /bits/ 64 <40000000>; + required-opps = <&rpmhpd_opp_low_svs>; + opp-peak-kBps = <4000000 1>; + }; + + /* GEN 3 x1 */ + opp-8000000 { + opp-hz = /bits/ 64 <8000000>; + required-opps = <&rpmhpd_opp_svs>; + opp-peak-kBps = <984500 1>; + }; + + /* GEN 3 x2 and GEN 4 x1 */ + opp-16000000 { + opp-hz = /bits/ 64 <16000000>; + required-opps = <&rpmhpd_opp_svs>; + opp-peak-kBps = <1969000 1>; + }; + + /* GEN 3 x4 and GEN 4 x2 */ + opp-32000000 { + opp-hz = /bits/ 64 <32000000>; + required-opps = <&rpmhpd_opp_svs>; + opp-peak-kBps = <3938000 1>; + }; + + /* GEN 3 x8 and GEN 4 x4 */ + opp-64000000 { + opp-hz = /bits/ 64 <64000000>; + required-opps = <&rpmhpd_opp_svs>; + opp-peak-kBps = <7876000 1>; + }; + + /* GEN 4 x8 */ + opp-128000000 { + opp-hz = /bits/ 64 <128000000>; + required-opps = <&rpmhpd_opp_svs>; + opp-peak-kBps = <15753000 1>; + }; + }; + }; + + pcie3_phy: phy@1be0000 { + compatible = "qcom,x1e80100-qmp-gen4x8-pcie-phy"; + reg = <0 0x01be0000 0 0x10000>; + + clocks = <&gcc GCC_PCIE_3_PHY_AUX_CLK>, + <&gcc GCC_PCIE_3_CFG_AHB_CLK>, + <&tcsr TCSR_PCIE_8L_CLKREF_EN>, + <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>, + <&gcc GCC_PCIE_3_PIPE_CLK>, + <&gcc GCC_PCIE_3_PIPEDIV2_CLK>; + clock-names = "aux", + "cfg_ahb", + "ref", + "rchng", + "pipe", + "pipediv2"; + + resets = <&gcc GCC_PCIE_3_PHY_BCR>, + <&gcc GCC_PCIE_3_NOCSR_COM_PHY_BCR>; + reset-names = "phy", + "phy_nocsr"; + + assigned-clocks = <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>; + assigned-clock-rates = <100000000>; + + power-domains = <&gcc GCC_PCIE_3_PHY_GDSC>; + + #clock-cells = <0>; + clock-output-names = "pcie3_pipe_clk"; + + #phy-cells = <0>; + + status = "disabled"; + }; + pcie6a: pci@1bf8000 { device_type = "pci"; compatible = "qcom,pcie-x1e80100";