| Message ID | 20240924101444.3933828-1-quic_qianyu@quicinc.com |
|---|---|
| Headers | show |
| Series | Add support for PCIe3 on x1e80100 | expand |
On 9/25/2024 8:52 PM, Manivannan Sadhasivam wrote: > On Wed, Sep 25, 2024 at 11:46:35AM +0200, Konrad Dybcio wrote: >> On 25.09.2024 11:30 AM, Konrad Dybcio wrote: >>> On 25.09.2024 10:05 AM, Manivannan Sadhasivam wrote: >>>> On Tue, Sep 24, 2024 at 04:26:34PM +0200, Konrad Dybcio wrote: >>>>> On 24.09.2024 12:14 PM, 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> >>>>>> --- >>>>> Qiang, Mani >>>>> >>>>> I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop. >>>> Is it based on x1e80100? >>> You would think so :P >>> >>>>> Adding the global irq breaks sdcard detection (the chip still comes >>>>> up fine) somehow. Removing the irq makes it work again :| >>>>> >>>>> I've confirmed that the irq number is correct >>>>> >>>> Yeah, I did see some issues with MSI on SM8250 (RB5) when global interrupts are >>>> enabled and I'm working with the hw folks to understand what is going on. But >>>> I didn't see the same issues on newer platforms (sa8775p etc...). >>>> >>>> Can you please confirm if the issue is due to MSI not being received from the >>>> device? Checking the /proc/interrutps is enough. >>> There's no msi-map for PCIe3. I recall +Johan talking about some sort of >>> a bug that prevents us from adding it? > Yeah, that's for using GIC-ITS to receive MSIs. But the default one is the > internal MSI controller in DWC. > >> Unless you just meant the msi0..=7 interrupts, then yeah, I only get one irq >> event with "global" in place and it seems to never get more >> > Ok. Then most likely the same issue I saw on SM8250 as well. But I'm confused > why Qiang didn't see the issue. I specifically asked him to add the global > interrupt and test it with the endpoint to check if the issue arises or not. > > Qiang, can you please confirm? Sorry, I misunderstood what you mean. I only verified if link was up but ignored the status of ep device driver. But look like this issue is because snpsys MSI irq is msked during probe. Can you please also unmask BIT(23) - BIT(30) when unmask PARF_INT_ALL_LINK_UP, like this @@ -1772,7 +1772,8 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; } - writel_relaxed(PARF_INT_ALL_LINK_UP, pcie->parf + PARF_INT_ALL_MASK); + writel_relaxed(PARF_INT_ALL_LINK_UP | GENMASK(30, 23), pcie->parf + PARF_INT_ALL_MASK); + dev_err(dev, "INT_ALL_MASK: 0x%x\n", readl_relaxed(pcie->parf + PARF_INT_ALL_MASK)); } After that, this issue is fixed on my setup. Thanks, Qiang > > - Mani >
On 9/25/2024 4:07 PM, Manivannan Sadhasivam wrote: > On Wed, Sep 25, 2024 at 11:47:02AM +0800, Qiang Yu wrote: >> On 9/24/2024 11:17 PM, Johan Hovold wrote: >>> On Tue, Sep 24, 2024 at 03:50:21PM +0200, Manivannan Sadhasivam wrote: >>>> On Tue, Sep 24, 2024 at 03:14:43AM -0700, Qiang Yu wrote: >>>>> X1E80100 has PCIe ports that support up to Gen4 x8 based on hardware IP >>>>> version 1.38.0. >>>>> >>>>> Currently the ops_1_9_0 which is being used for X1E80100 has config_sid >>>>> callback to config BDF to SID table. However, this callback is not >>>>> required for X1E80100 because it has smmuv3 support and BDF to SID table >>>>> will be not present. >>>>> >>>>> Hence add support for X1E80100 by introducing a new ops and cfg structures >>>>> that don't require the config_sid callback. This could be reused by the >>>>> future platforms based on SMMUv3. >>>>> >>>> Oops... I completely overlooked that you are not adding the SoC support but >>>> fixing the existing one :( Sorry for suggesting a commit message that changed >>>> the context. >>>> >>>> For this, you can have something like: >>>> >>>> "PCI: qcom: Fix the ops for X1E80100 SoC >>>> >>>> X1E80100 SoC is based on SMMUv3, hence it doesn't need the BDF2SID mapping >>>> present in the existing cfg_1_9_0 ops. This is fixed by introducing new ops >>>> 'ops_1_38_0' and cfg 'cfg_1_38_0' structures. These are exactly same as the >>>> 1_9_0 ones, but they don't have the 'config_sid()' callback that handles the >>>> BDF2SID mapping in the hardware. These new structures could also be used by the >>>> future SoCs making use of SMMUv3." >>> Don't we need something like this for sc8280xp and other platforms using >>> SMMUv3 as well? >> From what I know, sc8280xp and other qcom platforms are not using SMMUv3. > sc8280xp indeed has SMMUv3 for PCIe, but I'm not sure how it is configured. So > not completely sure whether we can avoid the mapping table or not. > > Qiang, please check with the hw team and let us know. Sure, will update once I get any response from hw team. Thanks, Qiang > > - Mani >
On 9/26/2024 11:28 AM, Qiang Yu wrote: > > On 9/25/2024 4:07 PM, Manivannan Sadhasivam wrote: >> On Wed, Sep 25, 2024 at 11:47:02AM +0800, Qiang Yu wrote: >>> On 9/24/2024 11:17 PM, Johan Hovold wrote: >>>> On Tue, Sep 24, 2024 at 03:50:21PM +0200, Manivannan Sadhasivam wrote: >>>>> On Tue, Sep 24, 2024 at 03:14:43AM -0700, Qiang Yu wrote: >>>>>> X1E80100 has PCIe ports that support up to Gen4 x8 based on >>>>>> hardware IP >>>>>> version 1.38.0. >>>>>> >>>>>> Currently the ops_1_9_0 which is being used for X1E80100 has >>>>>> config_sid >>>>>> callback to config BDF to SID table. However, this callback is not >>>>>> required for X1E80100 because it has smmuv3 support and BDF to >>>>>> SID table >>>>>> will be not present. >>>>>> >>>>>> Hence add support for X1E80100 by introducing a new ops and cfg >>>>>> structures >>>>>> that don't require the config_sid callback. This could be reused >>>>>> by the >>>>>> future platforms based on SMMUv3. >>>>>> >>>>> Oops... I completely overlooked that you are not adding the SoC >>>>> support but >>>>> fixing the existing one :( Sorry for suggesting a commit message >>>>> that changed >>>>> the context. >>>>> >>>>> For this, you can have something like: >>>>> >>>>> "PCI: qcom: Fix the ops for X1E80100 SoC >>>>> >>>>> X1E80100 SoC is based on SMMUv3, hence it doesn't need the BDF2SID >>>>> mapping >>>>> present in the existing cfg_1_9_0 ops. This is fixed by >>>>> introducing new ops >>>>> 'ops_1_38_0' and cfg 'cfg_1_38_0' structures. These are exactly >>>>> same as the >>>>> 1_9_0 ones, but they don't have the 'config_sid()' callback that >>>>> handles the >>>>> BDF2SID mapping in the hardware. These new structures could also >>>>> be used by the >>>>> future SoCs making use of SMMUv3." >>>> Don't we need something like this for sc8280xp and other platforms >>>> using >>>> SMMUv3 as well? >>> From what I know, sc8280xp and other qcom platforms are not using >>> SMMUv3. >> sc8280xp indeed has SMMUv3 for PCIe, but I'm not sure how it is >> configured. So >> not completely sure whether we can avoid the mapping table or not. >> >> Qiang, please check with the hw team and let us know. > Sure, will update once I get any response from hw team. HW team confirmed sc8280xp uses smmv3 for PCIe and doesn't support BDF2SID map. Besides, Abel once got confirmation from Joe that we also need to disable L0s for X1E80100. So can we use cfg_sc8280xp for both X1E80100 and SC8280XP and change its ops to ops_1_38_0? Thanks, Qiang > > Thanks, > Qiang >> >> - Mani >>
On Thu, Sep 26, 2024 at 01:19:18PM +0800, Qiang Yu wrote: > > On 9/26/2024 11:28 AM, Qiang Yu wrote: > > > > On 9/25/2024 4:07 PM, Manivannan Sadhasivam wrote: > > > On Wed, Sep 25, 2024 at 11:47:02AM +0800, Qiang Yu wrote: > > > > On 9/24/2024 11:17 PM, Johan Hovold wrote: > > > > > On Tue, Sep 24, 2024 at 03:50:21PM +0200, Manivannan Sadhasivam wrote: > > > > > > On Tue, Sep 24, 2024 at 03:14:43AM -0700, Qiang Yu wrote: > > > > > > > X1E80100 has PCIe ports that support up to Gen4 x8 > > > > > > > based on hardware IP > > > > > > > version 1.38.0. > > > > > > > > > > > > > > Currently the ops_1_9_0 which is being used for > > > > > > > X1E80100 has config_sid > > > > > > > callback to config BDF to SID table. However, this callback is not > > > > > > > required for X1E80100 because it has smmuv3 support > > > > > > > and BDF to SID table > > > > > > > will be not present. > > > > > > > > > > > > > > Hence add support for X1E80100 by introducing a new > > > > > > > ops and cfg structures > > > > > > > that don't require the config_sid callback. This > > > > > > > could be reused by the > > > > > > > future platforms based on SMMUv3. > > > > > > > > > > > > > Oops... I completely overlooked that you are not adding > > > > > > the SoC support but > > > > > > fixing the existing one :( Sorry for suggesting a commit > > > > > > message that changed > > > > > > the context. > > > > > > > > > > > > For this, you can have something like: > > > > > > > > > > > > "PCI: qcom: Fix the ops for X1E80100 SoC > > > > > > > > > > > > X1E80100 SoC is based on SMMUv3, hence it doesn't need > > > > > > the BDF2SID mapping > > > > > > present in the existing cfg_1_9_0 ops. This is fixed by > > > > > > introducing new ops > > > > > > 'ops_1_38_0' and cfg 'cfg_1_38_0' structures. These are > > > > > > exactly same as the > > > > > > 1_9_0 ones, but they don't have the 'config_sid()' > > > > > > callback that handles the > > > > > > BDF2SID mapping in the hardware. These new structures > > > > > > could also be used by the > > > > > > future SoCs making use of SMMUv3." > > > > > Don't we need something like this for sc8280xp and other > > > > > platforms using > > > > > SMMUv3 as well? > > > > From what I know, sc8280xp and other qcom platforms are not > > > > using SMMUv3. > > > sc8280xp indeed has SMMUv3 for PCIe, but I'm not sure how it is > > > configured. So > > > not completely sure whether we can avoid the mapping table or not. > > > > > > Qiang, please check with the hw team and let us know. > > Sure, will update once I get any response from hw team. > HW team confirmed sc8280xp uses smmv3 for PCIe and doesn't support BDF2SID > map. > > Besides, Abel once got confirmation from Joe that we also need to disable > L0s for X1E80100. So can we use cfg_sc8280xp for both X1E80100 and SC8280XP > and change its ops to ops_1_38_0? > Sounds good. But, OPS naming should be based on the baseline version i.e., it should be based on the IP version of SC8280XP and reused by X1E80100. Not the other way around. - Mani > Thanks, > Qiang > > > > Thanks, > > Qiang > > > > > > - Mani > > >
On Thu, Sep 26, 2024 at 11:15:34AM +0800, Qiang Yu wrote: > > On 9/25/2024 8:52 PM, Manivannan Sadhasivam wrote: > > On Wed, Sep 25, 2024 at 11:46:35AM +0200, Konrad Dybcio wrote: > > > On 25.09.2024 11:30 AM, Konrad Dybcio wrote: > > > > On 25.09.2024 10:05 AM, Manivannan Sadhasivam wrote: > > > > > On Tue, Sep 24, 2024 at 04:26:34PM +0200, Konrad Dybcio wrote: > > > > > > On 24.09.2024 12:14 PM, 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> > > > > > > > --- > > > > > > Qiang, Mani > > > > > > > > > > > > I have a RTS5261 mmc chip on PCIe3 on the Surface Laptop. > > > > > Is it based on x1e80100? > > > > You would think so :P > > > > > > > > > > Adding the global irq breaks sdcard detection (the chip still comes > > > > > > up fine) somehow. Removing the irq makes it work again :| > > > > > > > > > > > > I've confirmed that the irq number is correct > > > > > > > > > > > Yeah, I did see some issues with MSI on SM8250 (RB5) when global interrupts are > > > > > enabled and I'm working with the hw folks to understand what is going on. But > > > > > I didn't see the same issues on newer platforms (sa8775p etc...). > > > > > > > > > > Can you please confirm if the issue is due to MSI not being received from the > > > > > device? Checking the /proc/interrutps is enough. > > > > There's no msi-map for PCIe3. I recall +Johan talking about some sort of > > > > a bug that prevents us from adding it? > > Yeah, that's for using GIC-ITS to receive MSIs. But the default one is the > > internal MSI controller in DWC. > > > > > Unless you just meant the msi0..=7 interrupts, then yeah, I only get one irq > > > event with "global" in place and it seems to never get more > > > > > Ok. Then most likely the same issue I saw on SM8250 as well. But I'm confused > > why Qiang didn't see the issue. I specifically asked him to add the global > > interrupt and test it with the endpoint to check if the issue arises or not. > > > > Qiang, can you please confirm? > Sorry, I misunderstood what you mean. I only verified if link was up but > ignored the status of ep device > driver. > > But look like this issue is because snpsys MSI irq is msked during probe. > Can you please also unmask BIT(23) - BIT(30) when unmask > PARF_INT_ALL_LINK_UP, > like this > > @@ -1772,7 +1772,8 @@ static int qcom_pcie_probe(struct platform_device > *pdev) > goto err_host_deinit; > } > > - writel_relaxed(PARF_INT_ALL_LINK_UP, pcie->parf + > PARF_INT_ALL_MASK); > + writel_relaxed(PARF_INT_ALL_LINK_UP | GENMASK(30, 23), > pcie->parf + PARF_INT_ALL_MASK); > + dev_err(dev, "INT_ALL_MASK: 0x%x\n", > readl_relaxed(pcie->parf + PARF_INT_ALL_MASK)); > } > > After that, this issue is fixed on my setup. > I did see those bits, but they were mentioned as diagnostic interrupts. So I was not sure why disabling them masks MSI. Furthermore, MSI seems to work on SM8450 without enabling these bits. Anyway, since you mentioned that it helps in bringing back MSI on this platform, it doesn't hurt to enable these interrupts. I will post a patch for that, thanks! - Mani
This series add support for PCIe3 on x1e80100. PCIe3 needs additional set of clocks, regulators and new set of PCIe QMP PHY configuration compare other PCIe instances on x1e80100. Hence add required resource configuration and usage for PCIe3. v3->v4: 1. Reword commit msg of [PATCH v3 5/6] 2. Drop opp-table property from qcom,pcie-sm8450.yaml 3. Add Reviewed-by tag v2->v3: 1. Use 'Gen 4 x8' in commit msg 2. Move opp-table property to qcom,pcie-common.yaml 3. Add Reviewed-by tag 4. Add global interrupt and use GIC_SPI for the parent interrupt specifier 5. Use 0x0 in reg property and use pcie@ for pcie3 device node 6. Show different IP version v6.30 in commit msg 7. Add logic in controller driver to have new ops for x1e80100 v2->v1: 1. Squash [PATCH 1/8], [PATCH 2/8],[PATCH 3/8] into one patch and make the indentation consistent. 2. Put dts patch at the end of the patchset. 3. Put dt-binding patch at the first of the patchset. 4. Add a new patch where opp-table is added in dt-binding to avoid dtbs checking error. 5. Remove GCC_PCIE_3_AUX_CLK, RPMH_CXO_CLK, put in TCSR_PCIE_8L_CLKREF_EN as ref. 6. Remove lane_broadcasting. 7. Add 64 bit bar, Remove GCC_PCIE_3_PIPE_CLK_SRC, GCC_CFG_NOC_PCIE_ANOC_SOUTH_AHB_CLK is changed to GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK. 8. Add Reviewed-by tag. 9. Remove [PATCH 7/8], [PATCH 8/8]. Qiang Yu (6): dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x8 dt-bindings: PCI: qcom: Move OPP table to qcom,pcie-common.yaml phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3 clk: qcom: gcc-x1e80100: Fix halt_check for pipediv2 clocks PCI: qcom: Add support for X1E80100 SoC arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100 .../bindings/pci/qcom,pcie-common.yaml | 4 + .../bindings/pci/qcom,pcie-sm8450.yaml | 4 - .../phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 3 + arch/arm64/boot/dts/qcom/x1e80100.dtsi | 204 ++++++++++++++++- drivers/clk/qcom/gcc-x1e80100.c | 10 +- drivers/pci/controller/dwc/pcie-qcom.c | 16 +- drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 211 ++++++++++++++++++ .../qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h | 25 +++ drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h | 19 ++ 9 files changed, 485 insertions(+), 11 deletions(-) create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h