Re: [PATCH v2 2/2] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
---
  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,




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux