* [PATCH v2 0/2] PCI: qcom: Avoid DBI ant ATU register space mirroring @ 2024-07-18 5:12 Prudhvi Yarlagadda 2024-07-18 5:12 ` [PATCH v2 1/2] PCI: dwc: Add dbi_phys_addr and atu_phys_addr to struct dw_pcie Prudhvi Yarlagadda 2024-07-18 5:12 ` [PATCH v2 2/2] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region Prudhvi Yarlagadda 0 siblings, 2 replies; 5+ messages in thread From: Prudhvi Yarlagadda @ 2024-07-18 5:12 UTC (permalink / raw) To: jingoohan1, manivannan.sadhasivam, lpieralisi, kw, robh, bhelgaas Cc: linux-pci, linux-kernel, linux-arm-msm, quic_mrana, quic_pyarlaga Qualcomm PCIe controller has a wrapper called PARF which supports the relocation of DBI and ATU address space within the system memory. PARF gets the location of DBI and ATU from PARF_DBI_BASE_ADDR and PARF_ATU_BASE_ADDR registers. PARF also mirrors the memory block containing DBI and ATU registers within the system memory. And the size of this memory block is programmed in PARF_SLV_ADDR_SPACE_SIZE register. Power on reset of values of the above mentioned registers are good enough on platforms which require smaller size (less than 16MB) BAR memory. For platforms that need bigger BAR memory size, this mirroring of DBI and ATU address space by PARF conflicts with BAR memory. So to allow usage of bigger size of BAR, it is required to program PARF registers to prevent mirroring of DBI and ATU blocks and provide the physical addresses of DBI and ATU to PARF. This patch series stores physical addresses of DBI and ATU address space in 'struct dw_pcie' and programs the required PARF registers in the pcie_qcom.c driver. Changes in v2: - Updated commit message as suggested by Bjorn Helgaas. - Updated function name from qcom_pcie_avoid_dbi_atu_mirroring() to qcom_pcie_configure_dbi_atu_base() as suggested by Bjorn Helgaas. - Removed check for pdev in qcom_pcie_configure_dbi_atu_base() as suggested by Bjorn Helgaas. - Moved the qcom_pcie_configure_dbi_atu_base() call in the qcom_pcie_init_2_7_0() to the same place where PARF_DBI_BASE_ADDR register is being programmed as suggested by Bjorn Helgaas. - Added 'dbi_phys_addr', 'atu_phys_addr' in the 'struct dw_pcie' to store the physical addresses of dbi, atu base registers in dw_pcie_get_resources() as suggested by Manivannan Sadhasivam. - Added separate functions qcom_pcie_configure_dbi_atu_base() and qcom_pcie_configure_dbi_base() to program PARF register of different PARF versions. This is to disable DBI mirroring in all Qualcomm PCIe controllers as suggested by Manivannan Sadhasivam. - Link to v1: https://lore.kernel.org/linux-pci/a01404d2-2f4d-4fb8-af9d-3db66d39acf7@quicinc.com/T/ Tested: - Validated NVME functionality with PCIe6a on x1e80100 platform. - Validated WiFi functionality with PCIe4 on x1e80100 platform. Prudhvi Yarlagadda (2): PCI: dwc: Add dbi_phys_addr and atu_phys_addr to struct dw_pcie PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region drivers/pci/controller/dwc/pcie-designware.c | 2 + drivers/pci/controller/dwc/pcie-designware.h | 2 + drivers/pci/controller/dwc/pcie-qcom.c | 62 ++++++++++++++------ 3 files changed, 49 insertions(+), 17 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] PCI: dwc: Add dbi_phys_addr and atu_phys_addr to struct dw_pcie 2024-07-18 5:12 [PATCH v2 0/2] PCI: qcom: Avoid DBI ant ATU register space mirroring Prudhvi Yarlagadda @ 2024-07-18 5:12 ` Prudhvi Yarlagadda 2024-07-18 5:12 ` [PATCH v2 2/2] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region Prudhvi Yarlagadda 1 sibling, 0 replies; 5+ messages in thread From: Prudhvi Yarlagadda @ 2024-07-18 5:12 UTC (permalink / raw) To: jingoohan1, manivannan.sadhasivam, lpieralisi, kw, robh, bhelgaas Cc: linux-pci, linux-kernel, linux-arm-msm, quic_mrana, quic_pyarlaga Both DBI and ATU physical base addresses are needed by pcie_qcom.c driver to program the location of DBI and ATU blocks in Qualcomm PCIe Controller specific PARF hardware block. Signed-off-by: Prudhvi Yarlagadda <quic_pyarlaga@quicinc.com> --- drivers/pci/controller/dwc/pcie-designware.c | 2 ++ drivers/pci/controller/dwc/pcie-designware.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index 1b5aba1f0c92..bc3a5d6b0177 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -112,6 +112,7 @@ int dw_pcie_get_resources(struct dw_pcie *pci) pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res); if (IS_ERR(pci->dbi_base)) return PTR_ERR(pci->dbi_base); + pci->dbi_phys_addr = res->start; } /* DBI2 is mainly useful for the endpoint controller */ @@ -134,6 +135,7 @@ int dw_pcie_get_resources(struct dw_pcie *pci) pci->atu_base = devm_ioremap_resource(pci->dev, res); if (IS_ERR(pci->atu_base)) return PTR_ERR(pci->atu_base); + pci->atu_phys_addr = res->start; } else { pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; } diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 53c4c8f399c8..efc72989330c 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -407,8 +407,10 @@ struct dw_pcie_ops { struct dw_pcie { struct device *dev; void __iomem *dbi_base; + phys_addr_t dbi_phys_addr; void __iomem *dbi_base2; void __iomem *atu_base; + phys_addr_t atu_phys_addr; size_t atu_size; u32 num_ib_windows; u32 num_ob_windows; -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region 2024-07-18 5:12 [PATCH v2 0/2] PCI: qcom: Avoid DBI ant ATU register space mirroring Prudhvi Yarlagadda 2024-07-18 5:12 ` [PATCH v2 1/2] PCI: dwc: Add dbi_phys_addr and atu_phys_addr to struct dw_pcie Prudhvi Yarlagadda @ 2024-07-18 5:12 ` Prudhvi Yarlagadda 2024-07-18 21:52 ` Mayank Rana 1 sibling, 1 reply; 5+ messages in thread From: Prudhvi Yarlagadda @ 2024-07-18 5:12 UTC (permalink / raw) To: jingoohan1, manivannan.sadhasivam, lpieralisi, kw, robh, bhelgaas Cc: linux-pci, linux-kernel, linux-arm-msm, quic_mrana, quic_pyarlaga PARF hardware block which is a wrapper on top of DWC PCIe controller mirrors the DBI and ATU register space. It uses PARF_SLV_ADDR_SPACE_SIZE register to get the size of the memory block to be mirrored and uses PARF_DBI_BASE_ADDR, PARF_ATU_BASE_ADDR registers to determine the base address of DBI and ATU space inside the memory block that is being mirrored. When a memory region which is located above the SLV_ADDR_SPACE_SIZE boundary is used for BAR region then there could be an overlap of DBI and ATU address space that is getting mirrored and the BAR region. This results in DBI and ATU address space contents getting updated when a PCIe function driver tries updating the BAR/MMIO memory region. Reference memory map of the PCIe memory region with DBI and ATU address space overlapping BAR region is as below. |---------------| | | | | ------- --------|---------------| | | |---------------| | | | DBI | | | |---------------|---->DBI_BASE_ADDR | | | | | | | | | PCIe | |---->2*SLV_ADDR_SPACE_SIZE | BAR/MMIO|---------------| | Region | ATU | | | |---------------|---->ATU_BASE_ADDR | | | | PCIe | |---------------| Memory | | DBI | Region | |---------------|---->DBI_BASE_ADDR | | | | | --------| | | | |---->SLV_ADDR_SPACE_SIZE | |---------------| | | ATU | | |---------------|---->ATU_BASE_ADDR | | | | |---------------| | | DBI | | |---------------|---->DBI_BASE_ADDR | | | | | | ----------------|---------------| | | | | | | |---------------| Currently memory region beyond the SLV_ADDR_SPACE_SIZE boundary is not used for BAR region which is why the above mentioned issue is not encountered. This issue is discovered as part of internal testing when we tried moving the BAR region beyond the SLV_ADDR_SPACE_SIZE boundary. Hence we are trying to fix this. As PARF hardware block mirrors DBI and ATU register space after every PARF_SLV_ADDR_SPACE_SIZE (default 0x1000000) boundary multiple, write U32_MAX (all 0xFF's) to PARF_SLV_ADDR_SPACE_SIZE register to avoid mirroring DBI and ATU to BAR/MMIO region. Write the physical base address of DBI and ATU register blocks to PARF_DBI_BASE_ADDR (default 0x0) and PARF_ATU_BASE_ADDR (default 0x1000) respectively to make sure DBI and ATU blocks are at expected memory locations. The register offsets PARF_DBI_BASE_ADDR_V2, PARF_SLV_ADDR_SPACE_SIZE_V2 and PARF_ATU_BASE_ADDR are applicable for platforms that use PARF Qcom IP rev 1.9.0, 2.7.0 and 2.9.0. PARF_DBI_BASE_ADDR_V2 and PARF_SLV_ADDR_SPACE_SIZE_V2 are applicable for PARF Qcom IP rev 2.3.3. PARF_DBI_BASE_ADDR and PARF_SLV_ADDR_SPACE_SIZE are applicable for PARF Qcom IP rev 1.0.0, 2.3.2 and 2.4.0. Updating the init()/post_init() functions of the respective PARF versions to program applicable PARF_DBI_BASE_ADDR, PARF_SLV_ADDR_SPACE_SIZE and PARF_ATU_BASE_ADDR register offsets. And remove the unused SLV_ADDR_SPACE_SZ macro. Signed-off-by: Prudhvi Yarlagadda <quic_pyarlaga@quicinc.com> --- drivers/pci/controller/dwc/pcie-qcom.c | 62 +++++++++++++++++++------- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 0180edf3310e..845c7641431f 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -45,6 +45,7 @@ #define PARF_PHY_REFCLK 0x4c #define PARF_CONFIG_BITS 0x50 #define PARF_DBI_BASE_ADDR 0x168 +#define PARF_SLV_ADDR_SPACE_SIZE 0x16C #define PARF_MHI_CLOCK_RESET_CTRL 0x174 #define PARF_AXI_MSTR_WR_ADDR_HALT 0x178 #define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8 @@ -52,8 +53,13 @@ #define PARF_LTSSM 0x1b0 #define PARF_SID_OFFSET 0x234 #define PARF_BDF_TRANSLATE_CFG 0x24c -#define PARF_SLV_ADDR_SPACE_SIZE 0x358 +#define PARF_DBI_BASE_ADDR_V2 0x350 +#define PARF_DBI_BASE_ADDR_V2_HI 0x354 +#define PARF_SLV_ADDR_SPACE_SIZE_V2 0x358 +#define PARF_SLV_ADDR_SPACE_SIZE_V2_HI 0x35C #define PARF_NO_SNOOP_OVERIDE 0x3d4 +#define PARF_ATU_BASE_ADDR 0x634 +#define PARF_ATU_BASE_ADDR_HI 0x638 #define PARF_DEVICE_TYPE 0x1000 #define PARF_BDF_TO_SID_TABLE_N 0x2000 #define PARF_BDF_TO_SID_CFG 0x2c00 @@ -107,9 +113,6 @@ /* PARF_CONFIG_BITS register fields */ #define PHY_RX0_EQ(x) FIELD_PREP(GENMASK(26, 24), x) -/* PARF_SLV_ADDR_SPACE_SIZE register value */ -#define SLV_ADDR_SPACE_SZ 0x10000000 - /* PARF_MHI_CLOCK_RESET_CTRL register fields */ #define AHB_CLK_EN BIT(0) #define MSTR_AXI_CLK_EN BIT(1) @@ -324,6 +327,39 @@ static void qcom_pcie_clear_hpc(struct dw_pcie *pci) dw_pcie_dbi_ro_wr_dis(pci); } +static void qcom_pcie_configure_dbi_base(struct qcom_pcie *pcie) +{ + struct dw_pcie *pci = pcie->pci; + + if (pci->dbi_phys_addr) + writel(lower_32_bits(pci->dbi_phys_addr), pcie->parf + + PARF_DBI_BASE_ADDR); + + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); +} + +static void qcom_pcie_configure_dbi_atu_base(struct qcom_pcie *pcie) +{ + struct dw_pcie *pci = pcie->pci; + + if (pci->dbi_phys_addr) { + writel(lower_32_bits(pci->dbi_phys_addr), pcie->parf + + PARF_DBI_BASE_ADDR_V2); + writel(upper_32_bits(pci->dbi_phys_addr), pcie->parf + + PARF_DBI_BASE_ADDR_V2_HI); + } + + if (pci->atu_phys_addr) { + writel(lower_32_bits(pci->atu_phys_addr), pcie->parf + + PARF_ATU_BASE_ADDR); + writel(upper_32_bits(pci->atu_phys_addr), pcie->parf + + PARF_ATU_BASE_ADDR_HI); + } + + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE_V2); + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE_V2_HI); +} + static void qcom_pcie_2_1_0_ltssm_enable(struct qcom_pcie *pcie) { u32 val; @@ -540,8 +576,7 @@ static int qcom_pcie_init_1_0_0(struct qcom_pcie *pcie) static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie) { - /* change DBI base address */ - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); + qcom_pcie_configure_dbi_base(pcie); if (IS_ENABLED(CONFIG_PCI_MSI)) { u32 val = readl(pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT); @@ -628,8 +663,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie) val &= ~PHY_TEST_PWR_DOWN; writel(val, pcie->parf + PARF_PHY_CTRL); - /* change DBI base address */ - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); + qcom_pcie_configure_dbi_base(pcie); /* MAC PHY_POWERDOWN MUX DISABLE */ val = readl(pcie->parf + PARF_SYS_CTRL); @@ -811,13 +845,11 @@ static int qcom_pcie_post_init_2_3_3(struct qcom_pcie *pcie) u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); u32 val; - writel(SLV_ADDR_SPACE_SZ, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); - val = readl(pcie->parf + PARF_PHY_CTRL); val &= ~PHY_TEST_PWR_DOWN; writel(val, pcie->parf + PARF_PHY_CTRL); - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); + qcom_pcie_configure_dbi_atu_base(pcie); writel(MST_WAKEUP_EN | SLV_WAKEUP_EN | MSTR_ACLK_CGC_DIS | SLV_ACLK_CGC_DIS | CORE_CLK_CGC_DIS | @@ -913,8 +945,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) val &= ~PHY_TEST_PWR_DOWN; writel(val, pcie->parf + PARF_PHY_CTRL); - /* change DBI base address */ - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); + qcom_pcie_configure_dbi_atu_base(pcie); /* MAC PHY_POWERDOWN MUX DISABLE */ val = readl(pcie->parf + PARF_SYS_CTRL); @@ -1123,14 +1154,11 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie) u32 val; int i; - writel(SLV_ADDR_SPACE_SZ, - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); - val = readl(pcie->parf + PARF_PHY_CTRL); val &= ~PHY_TEST_PWR_DOWN; writel(val, pcie->parf + PARF_PHY_CTRL); - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); + qcom_pcie_configure_dbi_atu_base(pcie); writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE); writel(BYPASS | MSTR_AXI_CLK_EN | AHB_CLK_EN, -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region 2024-07-18 5:12 ` [PATCH v2 2/2] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region Prudhvi Yarlagadda @ 2024-07-18 21:52 ` Mayank Rana 2024-07-19 0:41 ` Prudhvi Yarlagadda 0 siblings, 1 reply; 5+ messages in thread From: Mayank Rana @ 2024-07-18 21:52 UTC (permalink / raw) To: Prudhvi Yarlagadda, jingoohan1, manivannan.sadhasivam, lpieralisi, kw, robh, bhelgaas Cc: linux-pci, linux-kernel, linux-arm-msm Hi Prudhvi On 7/17/2024 10:12 PM, Prudhvi Yarlagadda wrote: > PARF hardware block which is a wrapper on top of DWC PCIe controller > mirrors the DBI and ATU register space. It uses PARF_SLV_ADDR_SPACE_SIZE > register to get the size of the memory block to be mirrored and uses > PARF_DBI_BASE_ADDR, PARF_ATU_BASE_ADDR registers to determine the base > address of DBI and ATU space inside the memory block that is being > mirrored. > > When a memory region which is located above the SLV_ADDR_SPACE_SIZE > boundary is used for BAR region then there could be an overlap of DBI and > ATU address space that is getting mirrored and the BAR region. This > results in DBI and ATU address space contents getting updated when a PCIe > function driver tries updating the BAR/MMIO memory region. Reference > memory map of the PCIe memory region with DBI and ATU address space > overlapping BAR region is as below. > > |---------------| > | | > | | > ------- --------|---------------| > | | |---------------| > | | | DBI | > | | |---------------|---->DBI_BASE_ADDR > | | | | > | | | | > | PCIe | |---->2*SLV_ADDR_SPACE_SIZE > | BAR/MMIO|---------------| > | Region | ATU | > | | |---------------|---->ATU_BASE_ADDR > | | | | > PCIe | |---------------| > Memory | | DBI | > Region | |---------------|---->DBI_BASE_ADDR > | | | | > | --------| | > | | |---->SLV_ADDR_SPACE_SIZE > | |---------------| > | | ATU | > | |---------------|---->ATU_BASE_ADDR > | | | > | |---------------| > | | DBI | > | |---------------|---->DBI_BASE_ADDR > | | | > | | | > ----------------|---------------| > | | > | | > | | > |---------------| > > Currently memory region beyond the SLV_ADDR_SPACE_SIZE boundary is not > used for BAR region which is why the above mentioned issue is not > encountered. This issue is discovered as part of internal testing when we > tried moving the BAR region beyond the SLV_ADDR_SPACE_SIZE boundary. Hence > we are trying to fix this. > > As PARF hardware block mirrors DBI and ATU register space after every > PARF_SLV_ADDR_SPACE_SIZE (default 0x1000000) boundary multiple, write > U32_MAX (all 0xFF's) to PARF_SLV_ADDR_SPACE_SIZE register to avoid > mirroring DBI and ATU to BAR/MMIO region. Write the physical base address > of DBI and ATU register blocks to PARF_DBI_BASE_ADDR (default 0x0) and > PARF_ATU_BASE_ADDR (default 0x1000) respectively to make sure DBI and ATU > blocks are at expected memory locations. > > The register offsets PARF_DBI_BASE_ADDR_V2, PARF_SLV_ADDR_SPACE_SIZE_V2 > and PARF_ATU_BASE_ADDR are applicable for platforms that use PARF > Qcom IP rev 1.9.0, 2.7.0 and 2.9.0. PARF_DBI_BASE_ADDR_V2 and > PARF_SLV_ADDR_SPACE_SIZE_V2 are applicable for PARF Qcom IP rev 2.3.3. > PARF_DBI_BASE_ADDR and PARF_SLV_ADDR_SPACE_SIZE are applicable for PARF > Qcom IP rev 1.0.0, 2.3.2 and 2.4.0. Updating the init()/post_init() > functions of the respective PARF versions to program applicable > PARF_DBI_BASE_ADDR, PARF_SLV_ADDR_SPACE_SIZE and PARF_ATU_BASE_ADDR > register offsets. And remove the unused SLV_ADDR_SPACE_SZ macro. > > Signed-off-by: Prudhvi Yarlagadda <quic_pyarlaga@quicinc.com> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 62 +++++++++++++++++++------- > 1 file changed, 45 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 0180edf3310e..845c7641431f 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -45,6 +45,7 @@ > #define PARF_PHY_REFCLK 0x4c > #define PARF_CONFIG_BITS 0x50 > #define PARF_DBI_BASE_ADDR 0x168 > +#define PARF_SLV_ADDR_SPACE_SIZE 0x16C > #define PARF_MHI_CLOCK_RESET_CTRL 0x174 > #define PARF_AXI_MSTR_WR_ADDR_HALT 0x178 > #define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8 [...]> +static void qcom_pcie_configure_dbi_base(struct qcom_pcie *pcie) > +{ > + struct dw_pcie *pci = pcie->pci; > + > + if (pci->dbi_phys_addr) > + writel(lower_32_bits(pci->dbi_phys_addr), pcie->parf + > + PARF_DBI_BASE_ADDR); > + > + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); We can't update PARF_SLV_ADDR_SPACE_SIZE without updating PARF_DBI_BASE_ADDR. Please make dbi_phys_addr mandatory to update PARF_SLV_ADDR_SPACE_SIZE. > +} > + > +static void qcom_pcie_configure_dbi_atu_base(struct qcom_pcie *pcie) > +{ > + struct dw_pcie *pci = pcie->pci; > + > + if (pci->dbi_phys_addr) { > + writel(lower_32_bits(pci->dbi_phys_addr), pcie->parf + > + PARF_DBI_BASE_ADDR_V2); > + writel(upper_32_bits(pci->dbi_phys_addr), pcie->parf + > + PARF_DBI_BASE_ADDR_V2_HI); > + } > + > + if (pci->atu_phys_addr) { > + writel(lower_32_bits(pci->atu_phys_addr), pcie->parf + > + PARF_ATU_BASE_ADDR); > + writel(upper_32_bits(pci->atu_phys_addr), pcie->parf + > + PARF_ATU_BASE_ADDR_HI); > + } > + > + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE_V2); > + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE_V2_HI); Same as above. atu_phys_addr shall be optional here but not dbi_phys_addr to update PARF_SLV_ADDR_SPACE_SIZE. > +} > + > static void qcom_pcie_2_1_0_ltssm_enable(struct qcom_pcie *pcie) > { > u32 val; > @@ -540,8 +576,7 @@ static int qcom_pcie_init_1_0_0(struct qcom_pcie *pcie) > > static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie) > { > - /* change DBI base address */ > - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); > + qcom_pcie_configure_dbi_base(pcie); > > if (IS_ENABLED(CONFIG_PCI_MSI)) { > u32 val = readl(pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT); > @@ -628,8 +663,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie) > val &= ~PHY_TEST_PWR_DOWN; > writel(val, pcie->parf + PARF_PHY_CTRL); > > - /* change DBI base address */ > - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); > + qcom_pcie_configure_dbi_base(pcie); > > /* MAC PHY_POWERDOWN MUX DISABLE */ > val = readl(pcie->parf + PARF_SYS_CTRL); > @@ -811,13 +845,11 @@ static int qcom_pcie_post_init_2_3_3(struct qcom_pcie *pcie) > u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); > u32 val; > > - writel(SLV_ADDR_SPACE_SZ, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); > - > val = readl(pcie->parf + PARF_PHY_CTRL); > val &= ~PHY_TEST_PWR_DOWN; > writel(val, pcie->parf + PARF_PHY_CTRL); > > - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); > + qcom_pcie_configure_dbi_atu_base(pcie); > > writel(MST_WAKEUP_EN | SLV_WAKEUP_EN | MSTR_ACLK_CGC_DIS > | SLV_ACLK_CGC_DIS | CORE_CLK_CGC_DIS | > @@ -913,8 +945,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) > val &= ~PHY_TEST_PWR_DOWN; > writel(val, pcie->parf + PARF_PHY_CTRL); > > - /* change DBI base address */ > - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); > + qcom_pcie_configure_dbi_atu_base(pcie); > > /* MAC PHY_POWERDOWN MUX DISABLE */ > val = readl(pcie->parf + PARF_SYS_CTRL); > @@ -1123,14 +1154,11 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie) > u32 val; > int i; > > - writel(SLV_ADDR_SPACE_SZ, > - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); > - > val = readl(pcie->parf + PARF_PHY_CTRL); > val &= ~PHY_TEST_PWR_DOWN; > writel(val, pcie->parf + PARF_PHY_CTRL); > > - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); > + qcom_pcie_configure_dbi_atu_base(pcie); > > writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE); > writel(BYPASS | MSTR_AXI_CLK_EN | AHB_CLK_EN, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region 2024-07-18 21:52 ` Mayank Rana @ 2024-07-19 0:41 ` Prudhvi Yarlagadda 0 siblings, 0 replies; 5+ messages in thread From: Prudhvi Yarlagadda @ 2024-07-19 0:41 UTC (permalink / raw) To: Mayank Rana, jingoohan1, manivannan.sadhasivam, lpieralisi, kw, robh, bhelgaas Cc: linux-pci, linux-kernel, linux-arm-msm Hi Mayank, Thanks for the review comments. On 7/18/2024 2:52 PM, Mayank Rana wrote: > Hi Prudhvi > > On 7/17/2024 10:12 PM, Prudhvi Yarlagadda wrote: >> PARF hardware block which is a wrapper on top of DWC PCIe controller >> mirrors the DBI and ATU register space. It uses PARF_SLV_ADDR_SPACE_SIZE >> register to get the size of the memory block to be mirrored and uses >> PARF_DBI_BASE_ADDR, PARF_ATU_BASE_ADDR registers to determine the base >> address of DBI and ATU space inside the memory block that is being >> mirrored. >> >> When a memory region which is located above the SLV_ADDR_SPACE_SIZE >> boundary is used for BAR region then there could be an overlap of DBI and >> ATU address space that is getting mirrored and the BAR region. This >> results in DBI and ATU address space contents getting updated when a PCIe >> function driver tries updating the BAR/MMIO memory region. Reference >> memory map of the PCIe memory region with DBI and ATU address space >> overlapping BAR region is as below. >> >> |---------------| >> | | >> | | >> ------- --------|---------------| >> | | |---------------| >> | | | DBI | >> | | |---------------|---->DBI_BASE_ADDR >> | | | | >> | | | | >> | PCIe | |---->2*SLV_ADDR_SPACE_SIZE >> | BAR/MMIO|---------------| >> | Region | ATU | >> | | |---------------|---->ATU_BASE_ADDR >> | | | | >> PCIe | |---------------| >> Memory | | DBI | >> Region | |---------------|---->DBI_BASE_ADDR >> | | | | >> | --------| | >> | | |---->SLV_ADDR_SPACE_SIZE >> | |---------------| >> | | ATU | >> | |---------------|---->ATU_BASE_ADDR >> | | | >> | |---------------| >> | | DBI | >> | |---------------|---->DBI_BASE_ADDR >> | | | >> | | | >> ----------------|---------------| >> | | >> | | >> | | >> |---------------| >> >> Currently memory region beyond the SLV_ADDR_SPACE_SIZE boundary is not >> used for BAR region which is why the above mentioned issue is not >> encountered. This issue is discovered as part of internal testing when we >> tried moving the BAR region beyond the SLV_ADDR_SPACE_SIZE boundary. Hence >> we are trying to fix this. >> >> As PARF hardware block mirrors DBI and ATU register space after every >> PARF_SLV_ADDR_SPACE_SIZE (default 0x1000000) boundary multiple, write >> U32_MAX (all 0xFF's) to PARF_SLV_ADDR_SPACE_SIZE register to avoid >> mirroring DBI and ATU to BAR/MMIO region. Write the physical base address >> of DBI and ATU register blocks to PARF_DBI_BASE_ADDR (default 0x0) and >> PARF_ATU_BASE_ADDR (default 0x1000) respectively to make sure DBI and ATU >> blocks are at expected memory locations. >> >> The register offsets PARF_DBI_BASE_ADDR_V2, PARF_SLV_ADDR_SPACE_SIZE_V2 >> and PARF_ATU_BASE_ADDR are applicable for platforms that use PARF >> Qcom IP rev 1.9.0, 2.7.0 and 2.9.0. PARF_DBI_BASE_ADDR_V2 and >> PARF_SLV_ADDR_SPACE_SIZE_V2 are applicable for PARF Qcom IP rev 2.3.3. >> PARF_DBI_BASE_ADDR and PARF_SLV_ADDR_SPACE_SIZE are applicable for PARF >> Qcom IP rev 1.0.0, 2.3.2 and 2.4.0. Updating the init()/post_init() >> functions of the respective PARF versions to program applicable >> PARF_DBI_BASE_ADDR, PARF_SLV_ADDR_SPACE_SIZE and PARF_ATU_BASE_ADDR >> register offsets. And remove the unused SLV_ADDR_SPACE_SZ macro. >> >> Signed-off-by: Prudhvi Yarlagadda <quic_pyarlaga@quicinc.com> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 62 +++++++++++++++++++------- >> 1 file changed, 45 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index 0180edf3310e..845c7641431f 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -45,6 +45,7 @@ >> #define PARF_PHY_REFCLK 0x4c >> #define PARF_CONFIG_BITS 0x50 >> #define PARF_DBI_BASE_ADDR 0x168 >> +#define PARF_SLV_ADDR_SPACE_SIZE 0x16C >> #define PARF_MHI_CLOCK_RESET_CTRL 0x174 >> #define PARF_AXI_MSTR_WR_ADDR_HALT 0x178 >> #define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8 > [...]> +static void qcom_pcie_configure_dbi_base(struct qcom_pcie *pcie) >> +{ >> + struct dw_pcie *pci = pcie->pci; >> + >> + if (pci->dbi_phys_addr) >> + writel(lower_32_bits(pci->dbi_phys_addr), pcie->parf + >> + PARF_DBI_BASE_ADDR); >> + >> + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); > We can't update PARF_SLV_ADDR_SPACE_SIZE without updating PARF_DBI_BASE_ADDR. > Please make dbi_phys_addr mandatory to update PARF_SLV_ADDR_SPACE_SIZE. ACK. I will update it in the next patch. >> +} >> + >> +static void qcom_pcie_configure_dbi_atu_base(struct qcom_pcie *pcie) >> +{ >> + struct dw_pcie *pci = pcie->pci; >> + >> + if (pci->dbi_phys_addr) { >> + writel(lower_32_bits(pci->dbi_phys_addr), pcie->parf + >> + PARF_DBI_BASE_ADDR_V2); >> + writel(upper_32_bits(pci->dbi_phys_addr), pcie->parf + >> + PARF_DBI_BASE_ADDR_V2_HI); >> + } >> + >> + if (pci->atu_phys_addr) { >> + writel(lower_32_bits(pci->atu_phys_addr), pcie->parf + >> + PARF_ATU_BASE_ADDR); >> + writel(upper_32_bits(pci->atu_phys_addr), pcie->parf + >> + PARF_ATU_BASE_ADDR_HI); >> + } >> + >> + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE_V2); >> + writel(U32_MAX, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE_V2_HI); > Same as above. atu_phys_addr shall be optional here but not dbi_phys_addr to update PARF_SLV_ADDR_SPACE_SIZE. ACK. I will update it in the next patch. Thanks, Prudhvi >> +} >> + >> static void qcom_pcie_2_1_0_ltssm_enable(struct qcom_pcie *pcie) >> { >> u32 val; >> @@ -540,8 +576,7 @@ static int qcom_pcie_init_1_0_0(struct qcom_pcie *pcie) >> static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie) >> { >> - /* change DBI base address */ >> - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); >> + qcom_pcie_configure_dbi_base(pcie); >> if (IS_ENABLED(CONFIG_PCI_MSI)) { >> u32 val = readl(pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT); >> @@ -628,8 +663,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie) >> val &= ~PHY_TEST_PWR_DOWN; >> writel(val, pcie->parf + PARF_PHY_CTRL); >> - /* change DBI base address */ >> - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); >> + qcom_pcie_configure_dbi_base(pcie); >> /* MAC PHY_POWERDOWN MUX DISABLE */ >> val = readl(pcie->parf + PARF_SYS_CTRL); >> @@ -811,13 +845,11 @@ static int qcom_pcie_post_init_2_3_3(struct qcom_pcie *pcie) >> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); >> u32 val; >> - writel(SLV_ADDR_SPACE_SZ, pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); >> - >> val = readl(pcie->parf + PARF_PHY_CTRL); >> val &= ~PHY_TEST_PWR_DOWN; >> writel(val, pcie->parf + PARF_PHY_CTRL); >> - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); >> + qcom_pcie_configure_dbi_atu_base(pcie); >> writel(MST_WAKEUP_EN | SLV_WAKEUP_EN | MSTR_ACLK_CGC_DIS >> | SLV_ACLK_CGC_DIS | CORE_CLK_CGC_DIS | >> @@ -913,8 +945,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) >> val &= ~PHY_TEST_PWR_DOWN; >> writel(val, pcie->parf + PARF_PHY_CTRL); >> - /* change DBI base address */ >> - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); >> + qcom_pcie_configure_dbi_atu_base(pcie); >> /* MAC PHY_POWERDOWN MUX DISABLE */ >> val = readl(pcie->parf + PARF_SYS_CTRL); >> @@ -1123,14 +1154,11 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie) >> u32 val; >> int i; >> - writel(SLV_ADDR_SPACE_SZ, >> - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE); >> - >> val = readl(pcie->parf + PARF_PHY_CTRL); >> val &= ~PHY_TEST_PWR_DOWN; >> writel(val, pcie->parf + PARF_PHY_CTRL); >> - writel(0, pcie->parf + PARF_DBI_BASE_ADDR); >> + qcom_pcie_configure_dbi_atu_base(pcie); >> writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE); >> writel(BYPASS | MSTR_AXI_CLK_EN | AHB_CLK_EN, ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-07-19 0:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-07-18 5:12 [PATCH v2 0/2] PCI: qcom: Avoid DBI ant ATU register space mirroring Prudhvi Yarlagadda 2024-07-18 5:12 ` [PATCH v2 1/2] PCI: dwc: Add dbi_phys_addr and atu_phys_addr to struct dw_pcie Prudhvi Yarlagadda 2024-07-18 5:12 ` [PATCH v2 2/2] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region Prudhvi Yarlagadda 2024-07-18 21:52 ` Mayank Rana 2024-07-19 0:41 ` Prudhvi Yarlagadda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®