[PATCH v4 0/8] PCI: dwc: Add support for Marvell AC5 SoC

[PATCH v4 0/8] PCI: dwc: Add support for Marvell AC5 SoC

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Add support for AC5 SoC with MSI and in message emulated INTx mode.
There are differences in the registers addresses, blocks, DDR location
for coherent DMA allocation and additional implementation specific registers.
In addition, support cases of older Designware IP (Armada 7020) which supports
above 4GB PCIe physical memory window by use of device tree.

v4:
   1) Fix commit subject / messages formatting and naming

   2) Remove empty lines removal / addition

   3) Split patch number five from series to two patches

   4) Replace added dt-binding for DMA mask with dma-ranges

v3:
   1) Add dt bindings for DMA and region mask bits

   2) Support AC5 Legacy PCIe interrupts

   3) Introduce Configurable DMA mask

   4) Introduce region limit from DT

v2:
   1) add patch with adding compatible string for dt-bindings description

   2) fix W1 warnings which caused by unused leftover code

   3) Use one xlate function to translate ac5 dbi access. Also add
      mode description in comments about this translation.

   4) Use correct name of Raz

   5) Use matching data to pass the SoC specific params (type & ops)


Elad Nachman (5):
  dt-bindings: PCI: dwc: Add dma-ranges, region mask
  PCI: armada8k: support AC5 INTx PCIe interrupts
  PCI: armada8k: support reg regions according to DT.
  PCI: dwc: Introduce configurable DMA mask
  PCI: dwc: Introduce region limit from DT

Raz Adashi (1):
  PCI: armada8k: Add AC5 SoC support

Vadym Kochan (1):
  dt-bindings: PCI: armada8k: Add compatible string for AC5 SoC

Yuval Shaia (1):
  PCI: armada8k: Add AC5 MSI support

 .../devicetree/bindings/pci/pci-armada8k.txt  |   4 +-
 .../bindings/pci/snps,dw-pcie-common.yaml     |   5 +
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |   6 +
 drivers/pci/controller/dwc/pcie-armada8k.c    | 184 +++++++++++++++---
 .../pci/controller/dwc/pcie-designware-host.c |  28 ++-
 drivers/pci/controller/dwc/pcie-designware.c  |  12 +-
 6 files changed, 206 insertions(+), 33 deletions(-)

-- 
2.17.1

[PATCH v4 1/8] dt-bindings: PCI: armada8k: Add compatible string for AC5 SoC

[Elad Nachman]

From: Vadym Kochan <vadym.kochan@plvision.eu>

AC5 SoC has armada8k PCIe IP so add compatible string for it.

Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/pci/pci-armada8k.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
index ff25a134befa..b272fa4f08b5 100644
--- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
+++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
@@ -4,7 +4,9 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
 and thus inherits all the common properties defined in snps,dw-pcie.yaml.
 
 Required properties:
-- compatible: "marvell,armada8k-pcie"
+- compatible: Should be set to one of the following:
+   - "marvell,armada8k-pcie" : For A7K/8K family of SoCs
+   - "marvell,ac5-pcie"      : For AC5 family of SoCs
 - reg: must contain two register regions
    - the control register region
    - the config space region
-- 
2.17.1

[PATCH v4 2/8] PCI: armada8k: Add AC5 SoC support

[Elad Nachman]

From: Raz Adashi <raza@marvell.com>

pcie-armada8k driver is utilized to serve also AC5.

Driver assumes interrupt mask registers are located
in the same address inboth CPUs. This assumption is
incorrect - fix it for AC5.

Co-developed-by: Yuval Shaia <yshaia@marvell.com>
Signed-off-by: Yuval Shaia <yshaia@marvell.com>
Signed-off-by: Raz Adashi <raza@marvell.com>
Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
---
v2:
   1) fix W1 warnings which caused by unused leftover code

   2) Use one xlate function to translate ac5 dbi access. Also add
      mode description in comments about this translation.

   3) Use correct name of Raz

   4) Use matching data to pass the SoC specific params (type & ops)

 drivers/pci/controller/dwc/pcie-armada8k.c | 145 +++++++++++++++++----
 1 file changed, 120 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 5c999e15c357..b9fb1375dc58 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/pci.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -26,15 +27,26 @@
 
 #define ARMADA8K_PCIE_MAX_LANES PCIE_LNK_X4
 
+enum armada8k_pcie_type {
+	ARMADA8K_PCIE_TYPE_A8K,
+	ARMADA8K_PCIE_TYPE_AC5
+};
+
 struct armada8k_pcie {
 	struct dw_pcie *pci;
 	struct clk *clk;
 	struct clk *clk_reg;
 	struct phy *phy[ARMADA8K_PCIE_MAX_LANES];
 	unsigned int phy_count;
+	enum armada8k_pcie_type pcie_type;
 };
 
-#define PCIE_VENDOR_REGS_OFFSET		0x8000
+struct armada8k_pcie_of_data {
+	enum armada8k_pcie_type pcie_type;
+	const struct dw_pcie_ops *pcie_ops;
+};
+
+#define PCIE_VENDOR_REGS_OFFSET		0x8000	/* in ac5 is 0x10000 */
 
 #define PCIE_GLOBAL_CONTROL_REG		(PCIE_VENDOR_REGS_OFFSET + 0x0)
 #define PCIE_APP_LTSSM_EN		BIT(2)
@@ -48,10 +60,17 @@ struct armada8k_pcie {
 
 #define PCIE_GLOBAL_INT_CAUSE1_REG	(PCIE_VENDOR_REGS_OFFSET + 0x1C)
 #define PCIE_GLOBAL_INT_MASK1_REG	(PCIE_VENDOR_REGS_OFFSET + 0x20)
+#define PCIE_GLOBAL_INT_MASK2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x28)
 #define PCIE_INT_A_ASSERT_MASK		BIT(9)
 #define PCIE_INT_B_ASSERT_MASK		BIT(10)
 #define PCIE_INT_C_ASSERT_MASK		BIT(11)
 #define PCIE_INT_D_ASSERT_MASK		BIT(12)
+#define PCIE_INT_A_ASSERT_MASK_AC5	BIT(12)
+#define PCIE_INT_B_ASSERT_MASK_AC5	BIT(13)
+#define PCIE_INT_C_ASSERT_MASK_AC5	BIT(14)
+#define PCIE_INT_D_ASSERT_MASK_AC5	BIT(15)
+
+#define PCIE_ATU_ACCESS_MASK_AC5	GENMASK(21, 20)
 
 #define PCIE_ARCACHE_TRC_REG		(PCIE_VENDOR_REGS_OFFSET + 0x50)
 #define PCIE_AWCACHE_TRC_REG		(PCIE_VENDOR_REGS_OFFSET + 0x54)
@@ -169,6 +188,7 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	u32 reg;
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct armada8k_pcie *pcie = to_armada8k_pcie(pci);
 
 	if (!dw_pcie_link_up(pci)) {
 		/* Disable LTSSM state machine to enable configuration */
@@ -177,32 +197,41 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
 		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, reg);
 	}
 
-	/* Set the device to root complex mode */
-	reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
-	reg &= ~(PCIE_DEVICE_TYPE_MASK << PCIE_DEVICE_TYPE_SHIFT);
-	reg |= PCIE_DEVICE_TYPE_RC << PCIE_DEVICE_TYPE_SHIFT;
-	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, reg);
+	if (pcie->pcie_type == ARMADA8K_PCIE_TYPE_A8K) {
+		/* Set the device to root complex mode */
+		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
+		reg &= ~(PCIE_DEVICE_TYPE_MASK << PCIE_DEVICE_TYPE_SHIFT);
+		reg |= PCIE_DEVICE_TYPE_RC << PCIE_DEVICE_TYPE_SHIFT;
+		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, reg);
 
-	/* Set the PCIe master AxCache attributes */
-	dw_pcie_writel_dbi(pci, PCIE_ARCACHE_TRC_REG, ARCACHE_DEFAULT_VALUE);
-	dw_pcie_writel_dbi(pci, PCIE_AWCACHE_TRC_REG, AWCACHE_DEFAULT_VALUE);
+		/* Set the PCIe master AxCache attributes */
+		dw_pcie_writel_dbi(pci, PCIE_ARCACHE_TRC_REG, ARCACHE_DEFAULT_VALUE);
+		dw_pcie_writel_dbi(pci, PCIE_AWCACHE_TRC_REG, AWCACHE_DEFAULT_VALUE);
 
-	/* Set the PCIe master AxDomain attributes */
-	reg = dw_pcie_readl_dbi(pci, PCIE_ARUSER_REG);
-	reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
-	reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
-	dw_pcie_writel_dbi(pci, PCIE_ARUSER_REG, reg);
+		/* Set the PCIe master AxDomain attributes */
+		reg = dw_pcie_readl_dbi(pci, PCIE_ARUSER_REG);
+		reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
+		reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
+		dw_pcie_writel_dbi(pci, PCIE_ARUSER_REG, reg);
 
-	reg = dw_pcie_readl_dbi(pci, PCIE_AWUSER_REG);
-	reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
-	reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
-	dw_pcie_writel_dbi(pci, PCIE_AWUSER_REG, reg);
+		reg = dw_pcie_readl_dbi(pci, PCIE_AWUSER_REG);
+		reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
+		reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
+		dw_pcie_writel_dbi(pci, PCIE_AWUSER_REG, reg);
+	}
 
 	/* Enable INT A-D interrupts */
-	reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG);
-	reg |= PCIE_INT_A_ASSERT_MASK | PCIE_INT_B_ASSERT_MASK |
-	       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
-	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
+	if (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5) {
+		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG);
+		reg |= PCIE_INT_A_ASSERT_MASK_AC5 | PCIE_INT_B_ASSERT_MASK_AC5 |
+		       PCIE_INT_C_ASSERT_MASK_AC5 | PCIE_INT_D_ASSERT_MASK_AC5;
+		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG, reg);
+	} else {
+		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG);
+		reg |= PCIE_INT_A_ASSERT_MASK | PCIE_INT_B_ASSERT_MASK |
+		       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
+		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
+	}
 
 	return 0;
 }
@@ -258,9 +287,61 @@ static int armada8k_add_pcie_port(struct armada8k_pcie *pcie,
 	return 0;
 }
 
-static const struct dw_pcie_ops dw_pcie_ops = {
+static u32 ac5_xlate_dbi_reg(u32 reg)
+{
+	/* Handle AC5 ATU access */
+	if ((reg & ~0xfffff) == PCIE_ATU_ACCESS_MASK_AC5) {
+		reg &= 0xfffff;
+		/* ATU registers offset is 0xC00 + 0x200 * n,
+		 * from RFU registers.
+		 */
+		reg = 0xc000 | (0x200 * (reg >> 9)) | (reg & 0xff);
+	} else if ((reg & 0xfffff000) == PCIE_VENDOR_REGS_OFFSET) {
+		/* PCIe RFU registers in A8K are at offset 0x8000 from base
+		 * (0xf2600000) while in AC5 offset is 0x10000 from base
+		 * (0x800a0000) therefore need the addition of 0x8000.
+		 */
+		reg += PCIE_VENDOR_REGS_OFFSET;
+	}
+
+	return reg;
+}
+
+static u32 ac5_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
+			     u32 reg, size_t size)
+{
+	u32 val;
+
+	dw_pcie_read(base + ac5_xlate_dbi_reg(reg), size, &val);
+	return val;
+}
+
+static void ac5_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
+			       u32 reg, size_t size, u32 val)
+{
+	dw_pcie_write(base + ac5_xlate_dbi_reg(reg), size, val);
+}
+
+static const struct dw_pcie_ops armada8k_dw_pcie_ops = {
+	.link_up = armada8k_pcie_link_up,
+	.start_link = armada8k_pcie_start_link,
+};
+
+static const struct dw_pcie_ops ac5_dw_pcie_ops = {
 	.link_up = armada8k_pcie_link_up,
 	.start_link = armada8k_pcie_start_link,
+	.read_dbi = ac5_pcie_read_dbi,
+	.write_dbi = ac5_pcie_write_dbi,
+};
+
+static const struct armada8k_pcie_of_data a8k_pcie_of_data = {
+	.pcie_type = ARMADA8K_PCIE_TYPE_A8K,
+	.pcie_ops = &armada8k_dw_pcie_ops,
+};
+
+static const struct armada8k_pcie_of_data ac5_pcie_of_data = {
+	.pcie_type = ARMADA8K_PCIE_TYPE_AC5,
+	.pcie_ops = &ac5_dw_pcie_ops,
 };
 
 static int armada8k_pcie_probe(struct platform_device *pdev)
@@ -268,9 +349,15 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 	struct dw_pcie *pci;
 	struct armada8k_pcie *pcie;
 	struct device *dev = &pdev->dev;
+	const struct armada8k_pcie_of_data *data;
 	struct resource *base;
 	int ret;
 
+	data = of_device_get_match_data(dev);
+	if (!data)
+		return -EINVAL;
+
+
 	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
 	if (!pcie)
 		return -ENOMEM;
@@ -279,9 +366,10 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 	if (!pci)
 		return -ENOMEM;
 
+	pci->ops = data->pcie_ops;
 	pci->dev = dev;
-	pci->ops = &dw_pcie_ops;
 
+	pcie->pcie_type = data->pcie_type;
 	pcie->pci = pci;
 
 	pcie->clk = devm_clk_get(dev, NULL);
@@ -334,7 +422,14 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id armada8k_pcie_of_match[] = {
-	{ .compatible = "marvell,armada8k-pcie", },
+	{
+		.compatible = "marvell,armada8k-pcie",
+		.data = &a8k_pcie_of_data,
+	},
+	{
+		.compatible = "marvell,ac5-pcie",
+		.data = &ac5_pcie_of_data,
+	},
 	{},
 };
 
-- 
2.17.1

[PATCH v4 3/8] PCI: armada8k: Add AC5 MSI support

[Elad Nachman]

From: Yuval Shaia <yshaia@marvell.com>

AC5 requires different handling for MSI as with armada8k.
Fix it by:

1. Enabling the relevant bits in init phase
2. Dispatch virtual IRQ handlers when MSI interrupts are received

Also enable/disable PCIE_APP_LTSSM for AC5.

Signed-off-by: Yuval Shaia <yshaia@marvell.com>
Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
---
v4:
   Fix commit subject to be aligned with previous patch in series

v2:
   1) fix W1 warnings which caused by unused leftover code

   2) fix type in "requieres" word in the description

 drivers/pci/controller/dwc/pcie-armada8k.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index b9fb1375dc58..02481ecadd25 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -50,6 +50,7 @@ struct armada8k_pcie_of_data {
 
 #define PCIE_GLOBAL_CONTROL_REG		(PCIE_VENDOR_REGS_OFFSET + 0x0)
 #define PCIE_APP_LTSSM_EN		BIT(2)
+#define PCIE_APP_LTSSM_EN_AC5		BIT(24)
 #define PCIE_DEVICE_TYPE_SHIFT		4
 #define PCIE_DEVICE_TYPE_MASK		0xF
 #define PCIE_DEVICE_TYPE_RC		0x4 /* Root complex */
@@ -69,6 +70,7 @@ struct armada8k_pcie_of_data {
 #define PCIE_INT_B_ASSERT_MASK_AC5	BIT(13)
 #define PCIE_INT_C_ASSERT_MASK_AC5	BIT(14)
 #define PCIE_INT_D_ASSERT_MASK_AC5	BIT(15)
+#define PCIE_MSI_MASK_AC5		BIT(11)
 
 #define PCIE_ATU_ACCESS_MASK_AC5	GENMASK(21, 20)
 
@@ -184,6 +186,16 @@ static int armada8k_pcie_start_link(struct dw_pcie *pci)
 	return 0;
 }
 
+static void ac5_pcie_msi_init(struct dw_pcie *pci)
+{
+	u32 val;
+
+	/* Set MSI bit in interrupt mask */
+	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG);
+	val |= PCIE_MSI_MASK_AC5;
+	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, val);
+}
+
 static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	u32 reg;
@@ -193,7 +205,10 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
 	if (!dw_pcie_link_up(pci)) {
 		/* Disable LTSSM state machine to enable configuration */
 		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
-		reg &= ~(PCIE_APP_LTSSM_EN);
+		if (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5)
+			reg &= ~(PCIE_APP_LTSSM_EN_AC5);
+		else
+			reg &= ~(PCIE_APP_LTSSM_EN);
 		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, reg);
 	}
 
@@ -233,6 +248,9 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
 		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
 	}
 
+	if (IS_ENABLED(CONFIG_PCI_MSI) && (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5))
+		ac5_pcie_msi_init(pci);
+
 	return 0;
 }
 
@@ -249,6 +267,8 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 	 */
 	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG);
 	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG, val);
+	if ((PCIE_MSI_MASK_AC5 & val) && (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5))
+		dw_handle_msi_irq(&pci->pp);
 
 	return IRQ_HANDLED;
 }
-- 
2.17.1

[PATCH v4 4/8] dt-bindings: PCI: dwc: Add dma-ranges, region mask

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Add properties to support configurable DMA mask bits and region mask bits:

 1. configurable dma-ranges is needed for Marvell AC5/AC5X SOCs which
    have their physical DDR memory start at address 0x2_0000_0000.

 2. Configurable region mask bits is needed for the Marvell Armada
    7020/7040/8040 SOCs when the DT file places the PCIe window above the 4GB region.
    The Synopsis Designware PCIe IP in these SOCs is too old to specify the
    highest memory location supported by the PCIe, but practically supports
    such locations. Allow these locations to be specified in the DT file.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
v4:
   1) Fix commit message and its formatting

   2) Replace num-dmamask with dma-ranges

 .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 5 +++++
 Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml     | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index d87e13496834..3cb9af1aefeb 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -261,6 +261,11 @@ properties:
 
   dma-coherent: true
 
+  num-regionmask:
+    description: |
+      number of region limit mask bits to use, if different than default 32
+    maximum: 64
+
 additionalProperties: true
 
 ...
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 1a83f0f65f19..ed7ae2a14804 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -197,6 +197,12 @@ properties:
       - contains:
           const: msi
 
+  dma-ranges:
+    description:
+      Defines the DMA mask for devices which due to non-standard HW address
+      assignment have their RAM starting address above the lower 32-bit region.
+      Since this is a mask, only the size attribute of the dma-ranges is used.
+
 additionalProperties: true
 
 required:
-- 
2.17.1

[PATCH v4 5/8] PCI: armada8k: support AC5 INTx PCIe interrupts

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Support message emulation of INTx PCIe interrupts for Marvell AC5/X.
These message emulations require writing an additional status register
with acknowledge bits.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
v4:
   Split the part not handling INTx interrupts to a separate patch

 drivers/pci/controller/dwc/pcie-armada8k.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 02481ecadd25..2b94e32853ad 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -61,6 +61,7 @@ struct armada8k_pcie_of_data {
 
 #define PCIE_GLOBAL_INT_CAUSE1_REG	(PCIE_VENDOR_REGS_OFFSET + 0x1C)
 #define PCIE_GLOBAL_INT_MASK1_REG	(PCIE_VENDOR_REGS_OFFSET + 0x20)
+#define PCIE_GLOBAL_INT_CAUSE2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x24)
 #define PCIE_GLOBAL_INT_MASK2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x28)
 #define PCIE_INT_A_ASSERT_MASK		BIT(9)
 #define PCIE_INT_B_ASSERT_MASK		BIT(10)
@@ -267,8 +268,14 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 	 */
 	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG);
 	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG, val);
-	if ((PCIE_MSI_MASK_AC5 & val) && (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5))
-		dw_handle_msi_irq(&pci->pp);
+	if (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5) {
+		if (PCIE_MSI_MASK_AC5 & val)
+			dw_handle_msi_irq(&pci->pp);
+
+		val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE2_REG);
+		/* Now clear the second interrupt cause. */
+		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE2_REG, val);
+	}
 
 	return IRQ_HANDLED;
 }
-- 
2.17.1

[PATCH v4 6/8] PCI: armada8k: support reg regions according to DT.

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Support atu/vendor registers regions start according to DT rather than using
inflexible offset arithmetics.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
v4:
   Split from previous patch in series

 drivers/pci/controller/dwc/pcie-armada8k.c | 30 ++++++++++++++--------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 2b94e32853ad..145434c7a9fb 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -46,7 +46,7 @@ struct armada8k_pcie_of_data {
 	const struct dw_pcie_ops *pcie_ops;
 };
 
-#define PCIE_VENDOR_REGS_OFFSET		0x8000	/* in ac5 is 0x10000 */
+#define PCIE_VENDOR_REGS_OFFSET		0x8000	/* in ac5 is in another region */
 
 #define PCIE_GLOBAL_CONTROL_REG		(PCIE_VENDOR_REGS_OFFSET + 0x0)
 #define PCIE_APP_LTSSM_EN		BIT(2)
@@ -314,24 +314,29 @@ static int armada8k_add_pcie_port(struct armada8k_pcie *pcie,
 	return 0;
 }
 
-static u32 ac5_xlate_dbi_reg(u32 reg)
+static void __iomem *ac5_xlate_dbi_reg(struct dw_pcie *pci,
+				       void __iomem *base,
+				       u32 reg)
 {
 	/* Handle AC5 ATU access */
 	if ((reg & ~0xfffff) == PCIE_ATU_ACCESS_MASK_AC5) {
 		reg &= 0xfffff;
-		/* ATU registers offset is 0xC00 + 0x200 * n,
+		/* ATU registers offset is 0xC000 + 0x200 * n,
 		 * from RFU registers.
 		 */
-		reg = 0xc000 | (0x200 * (reg >> 9)) | (reg & 0xff);
+		reg = (0x200 * (reg >> 9)) | (reg & 0xff);
+		return pci->atu_base + reg;
 	} else if ((reg & 0xfffff000) == PCIE_VENDOR_REGS_OFFSET) {
 		/* PCIe RFU registers in A8K are at offset 0x8000 from base
 		 * (0xf2600000) while in AC5 offset is 0x10000 from base
-		 * (0x800a0000) therefore need the addition of 0x8000.
+		 * (0x800a0000) therefore need to be reduced by 0x8000
+		 * and rebased from dbi2 base, which is set to the PCIe rfu
+		 * base in the AC5 dts:
 		 */
-		reg += PCIE_VENDOR_REGS_OFFSET;
+		reg -= PCIE_VENDOR_REGS_OFFSET;
+		return pci->dbi_base2 + reg;
 	}
-
-	return reg;
+	return base + reg;
 }
 
 static u32 ac5_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
@@ -339,14 +344,14 @@ static u32 ac5_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
 {
 	u32 val;
 
-	dw_pcie_read(base + ac5_xlate_dbi_reg(reg), size, &val);
+	dw_pcie_read(ac5_xlate_dbi_reg(pci, base, reg), size, &val);
 	return val;
 }
 
 static void ac5_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
 			       u32 reg, size_t size, u32 val)
 {
-	dw_pcie_write(base + ac5_xlate_dbi_reg(reg), size, val);
+	dw_pcie_write(ac5_xlate_dbi_reg(pci, base, reg), size, val);
 }
 
 static const struct dw_pcie_ops armada8k_dw_pcie_ops = {
@@ -425,7 +430,6 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 		ret = PTR_ERR(pci->dbi_base);
 		goto fail_clkreg;
 	}
-
 	ret = armada8k_pcie_setup_phys(pcie);
 	if (ret)
 		goto fail_clkreg;
@@ -436,6 +440,10 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_phy;
 
+	/* backwards compatibility with older dts files: */
+	if (!pci->dbi_base2)
+		pci->dbi_base2 = pci->dbi_base;
+
 	return 0;
 
 disable_phy:
-- 
2.17.1

[PATCH v4 7/8] PCI: dwc: Introduce configurable DMA mask

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Some devices, such as AC5 and AC5X have their physical DDR memory
start at address 0x2_0000_0000. In order to have the DMA coherent
allocation succeed later, a different DMA mask is required, as
defined in the DT file for such SOCs, using dma-ranges.

If not defined, fallback to 32-bit as previously done in the code.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
v4:
   1) Fix commit message formatting.

   2) Fix removal / addition of blank lines.

 .../pci/controller/dwc/pcie-designware-host.c | 28 +++++++++++++++++--
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 9952057c8819..74393e59e7a7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -325,10 +325,14 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct device *dev = pci->dev;
+	struct device_node *np = dev->of_node;
 	struct platform_device *pdev = to_platform_device(dev);
 	u64 *msi_vaddr;
 	int ret;
 	u32 ctrl, num_ctrls;
+	u32 num_dma_maskbits = 32;
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
 
 	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
 		pp->irq_mask[ctrl] = ~0;
@@ -367,18 +371,36 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
 	}
 
 	/*
+	 * Some devices, such as AC5 and AC5X have their physical DDR memory
+	 * start at address 0x2_0000_0000 . In order to have the DMA
+	 * coherent allocation succeed later, a different DMA mask is
+	 * required, as defined in the DT file for such SOCs using dma-ranges.
+	 * If not defined, fallback to 32-bit as described below:
+	 *
 	 * Even though the iMSI-RX Module supports 64-bit addresses some
 	 * peripheral PCIe devices may lack 64-bit message support. In
 	 * order not to miss MSI TLPs from those devices the MSI target
 	 * address has to be within the lowest 4GB.
 	 *
-	 * Note until there is a better alternative found the reservation is
+	 * Note until there is a better alternative found, the reservation is
 	 * done by allocating from the artificially limited DMA-coherent
 	 * memory.
 	 */
-	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
+	ret = of_pci_dma_range_parser_init(&parser, np);
+	if (!ret) {
+		if (of_pci_range_parser_one(&parser, &range)) {
+			if (range.size > BIT_MASK(32) ) {
+				num_dma_maskbits = fls64(range.size);
+				dev_info(dev, "Overriding DMA mask to %u bits...\n", num_dma_maskbits);
+			}
+		}
+	}
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(num_dma_maskbits));
 	if (ret)
-		dev_warn(dev, "Failed to set DMA mask to 32-bit. Devices with only 32-bit MSI support may not work properly\n");
+		dev_warn(dev,
+			 "Failed to set DMA mask to %u-bit. Devices with only 32-bit MSI support may not work properly\n",
+			 num_dma_maskbits);
 
 	msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
 					GFP_KERNEL);
-- 
2.17.1

[PATCH v4 8/8] PCI: dwc: Introduce region limit from DT

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Allow dts override of region limit for SOCs with older Synopsis
Designware PCIe IP but with greater than 32-bit address range support,
such as the Armada 7020/7040/8040 family of SOCs by Marvell,
when the DT file places the PCIe window above the 4GB region.
The Synopsis Designware PCIe IP in these SOCs is too old to specify the
highest memory location supported by the PCIe, but practically supports
such locations. Allow these locations to be specified in the DT file.
DT property is called num-regionmask , and can range between 33 and 64.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
v4:
   1) Fix blank lines removal / addition

   2) Remove usage of variable with same name as dt binding property

 drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 53a16b8b6ac2..9773c110c733 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -735,8 +735,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
 void dw_pcie_iatu_detect(struct dw_pcie *pci)
 {
 	int max_region, ob, ib;
-	u32 val, min, dir;
+	u32 val, min, dir, ret;
 	u64 max;
+	struct device *dev = pci->dev;
+	struct device_node *np = dev->of_node;
 
 	val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
 	if (val == 0xFFFFFFFF) {
@@ -781,7 +783,13 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)
 		dw_pcie_writel_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT, 0xFFFFFFFF);
 		max = dw_pcie_readl_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT);
 	} else {
-		max = 0;
+		/* Allow dts override of region limit for older IP with above 32-bit support: */
+		ret = of_property_read_u32(np, "num-regionmask", &val);
+		if (!ret && val > 32) {
+			max = GENMASK(val - 33, 0);
+			dev_info(pci->dev, "Overriding region limit to %u bits\n", val);
+		} else
+			max = 0;
 	}
 
 	pci->num_ob_windows = ob;
-- 
2.17.1

Re: [PATCH v4 0/8] PCI: dwc: Add support for Marvell AC5 SoC

[Bjorn Helgaas]

On Mon, Mar 13, 2023 at 02:40:08PM +0200, Elad Nachman wrote:
> ...
> Elad Nachman (5):
>   dt-bindings: PCI: dwc: Add dma-ranges, region mask
>   PCI: armada8k: support AC5 INTx PCIe interrupts
>   PCI: armada8k: support reg regions according to DT.
>   PCI: dwc: Introduce configurable DMA mask
>   PCI: dwc: Introduce region limit from DT

If you repost for any reason, please capitalize consistently ("Support
..." (twice)) and drop the period at end of subject to avoid
unnecessary line wrapping.

Re: [PATCH v4 2/8] PCI: armada8k: Add AC5 SoC support

[Bjorn Helgaas]

On Mon, Mar 13, 2023 at 02:40:10PM +0200, Elad Nachman wrote:
> From: Raz Adashi <raza@marvell.com>
> 
> pcie-armada8k driver is utilized to serve also AC5.
> 
> Driver assumes interrupt mask registers are located
> in the same address inboth CPUs. This assumption is
> incorrect - fix it for AC5.

s/inboth/in both/

Rewrap to fill 75 columns.

> +#define PCIE_VENDOR_REGS_OFFSET		0x8000	/* in ac5 is 0x10000 */

Don't add this comment in patch [2/8] and then update it in patch
[6/8].  Make it what you want the first time.  Also nice if you make
it fit in 80 columns like the rest of the file.

> -static const struct dw_pcie_ops dw_pcie_ops = {
> +static u32 ac5_xlate_dbi_reg(u32 reg)
> +{
> +	/* Handle AC5 ATU access */
> +	if ((reg & ~0xfffff) == PCIE_ATU_ACCESS_MASK_AC5) {
> +		reg &= 0xfffff;
> +		/* ATU registers offset is 0xC00 + 0x200 * n,
> +		 * from RFU registers.
> +		 */
> +		reg = 0xc000 | (0x200 * (reg >> 9)) | (reg & 0xff);
> +	} else if ((reg & 0xfffff000) == PCIE_VENDOR_REGS_OFFSET) {
> +		/* PCIe RFU registers in A8K are at offset 0x8000 from base
> +		 * (0xf2600000) while in AC5 offset is 0x10000 from base
> +		 * (0x800a0000) therefore need the addition of 0x8000.
> +		 */
> +		reg += PCIE_VENDOR_REGS_OFFSET;

This is a hidden assumption that the AC5 offset (0x10000) happens to
be twice PCIE_VENDOR_REGS_OFFSET (0x8000).  I think the AC5 offset
deserves its own separate #define.

Use the conventional multi-line comment style:

  /*
   * ...
   */

> +	pci->ops = data->pcie_ops;
>  	pci->dev = dev;
> -	pci->ops = &dw_pcie_ops;

The diff is easier to read if you don't move the "pci->ops ="
assignment.  Move it if there's a *reason* to move it, but leave it
at the same spot otherwise.

Re: [PATCH v4 8/8] PCI: dwc: Introduce region limit from DT

[Bjorn Helgaas]

[+cc Serge, who has done most of the recent work in this file]

On Mon, Mar 13, 2023 at 02:40:16PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Allow dts override of region limit for SOCs with older Synopsis
> Designware PCIe IP but with greater than 32-bit address range support,
> such as the Armada 7020/7040/8040 family of SOCs by Marvell,
> when the DT file places the PCIe window above the 4GB region.
> The Synopsis Designware PCIe IP in these SOCs is too old to specify the
> highest memory location supported by the PCIe, but practically supports
> such locations. Allow these locations to be specified in the DT file.
> DT property is called num-regionmask , and can range between 33 and 64.

s/Synopsis/Synopsys/ (several occurrences)

s/Designware/DesignWare/ (several occurrences)

Remove space before comma.

> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
> v4:
>    1) Fix blank lines removal / addition
> 
>    2) Remove usage of variable with same name as dt binding property
> 
>  drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 53a16b8b6ac2..9773c110c733 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -735,8 +735,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
>  void dw_pcie_iatu_detect(struct dw_pcie *pci)
>  {
>  	int max_region, ob, ib;
> -	u32 val, min, dir;
> +	u32 val, min, dir, ret;
>  	u64 max;
> +	struct device *dev = pci->dev;
> +	struct device_node *np = dev->of_node;
>  
>  	val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
>  	if (val == 0xFFFFFFFF) {
> @@ -781,7 +783,13 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)
>  		dw_pcie_writel_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT, 0xFFFFFFFF);
>  		max = dw_pcie_readl_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT);
>  	} else {
> -		max = 0;
> +		/* Allow dts override of region limit for older IP with above 32-bit support: */

Reflow comment to fit in 80 columns.

> +		ret = of_property_read_u32(np, "num-regionmask", &val);
> +		if (!ret && val > 32) {
> +			max = GENMASK(val - 33, 0);
> +			dev_info(pci->dev, "Overriding region limit to %u bits\n", val);
> +		} else
> +			max = 0;
>  	}
>  
>  	pci->num_ob_windows = ob;
> -- 
> 2.17.1
> 

Re: [PATCH v4 8/8] PCI: dwc: Introduce region limit from DT

[Serge Semin]

Hi Bjorn

On Mon, Mar 13, 2023 at 02:48:02PM -0500, Bjorn Helgaas wrote:
> [+cc Serge, who has done most of the recent work in this file]
> 

Thanks for sending copy to me. I'll have a look at the series on
this week.

-Serge(y)

> On Mon, Mar 13, 2023 at 02:40:16PM +0200, Elad Nachman wrote:
> > From: Elad Nachman <enachman@marvell.com>
> > 
> > Allow dts override of region limit for SOCs with older Synopsis
> > Designware PCIe IP but with greater than 32-bit address range support,
> > such as the Armada 7020/7040/8040 family of SOCs by Marvell,
> > when the DT file places the PCIe window above the 4GB region.
> > The Synopsis Designware PCIe IP in these SOCs is too old to specify the
> > highest memory location supported by the PCIe, but practically supports
> > such locations. Allow these locations to be specified in the DT file.
> > DT property is called num-regionmask , and can range between 33 and 64.
> 
> s/Synopsis/Synopsys/ (several occurrences)
> 
> s/Designware/DesignWare/ (several occurrences)
> 
> Remove space before comma.
> 
> > Signed-off-by: Elad Nachman <enachman@marvell.com>
> > ---
> > v4:
> >    1) Fix blank lines removal / addition
> > 
> >    2) Remove usage of variable with same name as dt binding property
> > 
> >  drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 53a16b8b6ac2..9773c110c733 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -735,8 +735,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
> >  void dw_pcie_iatu_detect(struct dw_pcie *pci)
> >  {
> >  	int max_region, ob, ib;
> > -	u32 val, min, dir;
> > +	u32 val, min, dir, ret;
> >  	u64 max;
> > +	struct device *dev = pci->dev;
> > +	struct device_node *np = dev->of_node;
> >  
> >  	val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
> >  	if (val == 0xFFFFFFFF) {
> > @@ -781,7 +783,13 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)
> >  		dw_pcie_writel_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT, 0xFFFFFFFF);
> >  		max = dw_pcie_readl_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT);
> >  	} else {
> > -		max = 0;
> > +		/* Allow dts override of region limit for older IP with above 32-bit support: */
> 
> Reflow comment to fit in 80 columns.
> 
> > +		ret = of_property_read_u32(np, "num-regionmask", &val);
> > +		if (!ret && val > 32) {
> > +			max = GENMASK(val - 33, 0);
> > +			dev_info(pci->dev, "Overriding region limit to %u bits\n", val);
> > +		} else
> > +			max = 0;
> >  	}
> >  
> >  	pci->num_ob_windows = ob;
> > -- 
> > 2.17.1
> > 
> 

Re: [PATCH v4 7/8] PCI: dwc: Introduce configurable DMA mask

[Rob Herring]

+Robin

On Mon, Mar 13, 2023 at 02:40:15PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Some devices, such as AC5 and AC5X have their physical DDR memory
> start at address 0x2_0000_0000. In order to have the DMA coherent
> allocation succeed later, a different DMA mask is required, as
> defined in the DT file for such SOCs, using dma-ranges.

I'm afraid this is not right. 'dma-ranges' in the PCI host bridge node 
applies to PCI devices (i.e. child node), not the host bridge itself. 
It's 'dma-ranges' in the parent node of the host bridge that applies 
here. The core code will set masks (ranges really now) based on bus 
restrictions. The mask for the device should only be based on the 
device's limits (i.e. the device is 32-bit only). 

I think you will need whatever solution comes out of this thread[1].

Rob

[1] https://lore.kernel.org/all/c014b074-6d7f-773b-533a-c0500e239ab8@arm.com/

Re: [PATCH v4 4/8] dt-bindings: PCI: dwc: Add dma-ranges, region mask

[Rob Herring]

On Mon, Mar 13, 2023 at 02:40:12PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add properties to support configurable DMA mask bits and region mask bits:
> 
>  1. configurable dma-ranges is needed for Marvell AC5/AC5X SOCs which
>     have their physical DDR memory start at address 0x2_0000_0000.
> 
>  2. Configurable region mask bits is needed for the Marvell Armada
>     7020/7040/8040 SOCs when the DT file places the PCIe window above the 4GB region.
>     The Synopsis Designware PCIe IP in these SOCs is too old to specify the
>     highest memory location supported by the PCIe, but practically supports
>     such locations. Allow these locations to be specified in the DT file.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
> v4:
>    1) Fix commit message and its formatting
> 
>    2) Replace num-dmamask with dma-ranges
> 
>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 5 +++++
>  Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml     | 6 ++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> index d87e13496834..3cb9af1aefeb 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -261,6 +261,11 @@ properties:
>  
>    dma-coherent: true
>  
> +  num-regionmask:
> +    description: |
> +      number of region limit mask bits to use, if different than default 32
> +    maximum: 64

This should be implied from the compatible string.

> +
>  additionalProperties: true
>  
>  ...
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> index 1a83f0f65f19..ed7ae2a14804 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> @@ -197,6 +197,12 @@ properties:
>        - contains:
>            const: msi
>  
> +  dma-ranges:
> +    description:
> +      Defines the DMA mask for devices which due to non-standard HW address
> +      assignment have their RAM starting address above the lower 32-bit region.
> +      Since this is a mask, only the size attribute of the dma-ranges is used.
> +

No need for this, it is already defined in pci-bus.yaml.

The description is wrong here anyways. The purpose is to translate 
inbound PCI addresses to parent bus addresses (and eventually CPU 
addresses).

Rob

Re: [PATCH v4 2/8] PCI: armada8k: Add AC5 SoC support

[Serge Semin]

On Mon, Mar 13, 2023 at 02:40:10PM +0200, Elad Nachman wrote:
> From: Raz Adashi <raza@marvell.com>
> 
> pcie-armada8k driver is utilized to serve also AC5.
> 
> Driver assumes interrupt mask registers are located
> in the same address inboth CPUs. This assumption is
> incorrect - fix it for AC5.
> 
> Co-developed-by: Yuval Shaia <yshaia@marvell.com>
> Signed-off-by: Yuval Shaia <yshaia@marvell.com>
> Signed-off-by: Raz Adashi <raza@marvell.com>
> Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
> ---
> v2:
>    1) fix W1 warnings which caused by unused leftover code
> 
>    2) Use one xlate function to translate ac5 dbi access. Also add
>       mode description in comments about this translation.
> 
>    3) Use correct name of Raz
> 
>    4) Use matching data to pass the SoC specific params (type & ops)
> 
>  drivers/pci/controller/dwc/pcie-armada8k.c | 145 +++++++++++++++++----
>  1 file changed, 120 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index 5c999e15c357..b9fb1375dc58 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -16,6 +16,7 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/pci.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> @@ -26,15 +27,26 @@
>  
>  #define ARMADA8K_PCIE_MAX_LANES PCIE_LNK_X4
>  
> +enum armada8k_pcie_type {
> +	ARMADA8K_PCIE_TYPE_A8K,
> +	ARMADA8K_PCIE_TYPE_AC5
> +};

AFAICS instead of if-else-based pattern you can just split up the
implementation into the dedicated functions.

> +
>  struct armada8k_pcie {
>  	struct dw_pcie *pci;
>  	struct clk *clk;
>  	struct clk *clk_reg;
>  	struct phy *phy[ARMADA8K_PCIE_MAX_LANES];
>  	unsigned int phy_count;
> +	enum armada8k_pcie_type pcie_type;
>  };
>  
> -#define PCIE_VENDOR_REGS_OFFSET		0x8000
> +struct armada8k_pcie_of_data {
> +	enum armada8k_pcie_type pcie_type;
> +	const struct dw_pcie_ops *pcie_ops;
> +};
> +
> +#define PCIE_VENDOR_REGS_OFFSET		0x8000	/* in ac5 is 0x10000 */
>  
>  #define PCIE_GLOBAL_CONTROL_REG		(PCIE_VENDOR_REGS_OFFSET + 0x0)
>  #define PCIE_APP_LTSSM_EN		BIT(2)
> @@ -48,10 +60,17 @@ struct armada8k_pcie {
>  
>  #define PCIE_GLOBAL_INT_CAUSE1_REG	(PCIE_VENDOR_REGS_OFFSET + 0x1C)
>  #define PCIE_GLOBAL_INT_MASK1_REG	(PCIE_VENDOR_REGS_OFFSET + 0x20)
> +#define PCIE_GLOBAL_INT_MASK2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x28)
>  #define PCIE_INT_A_ASSERT_MASK		BIT(9)
>  #define PCIE_INT_B_ASSERT_MASK		BIT(10)
>  #define PCIE_INT_C_ASSERT_MASK		BIT(11)
>  #define PCIE_INT_D_ASSERT_MASK		BIT(12)
> +#define PCIE_INT_A_ASSERT_MASK_AC5	BIT(12)
> +#define PCIE_INT_B_ASSERT_MASK_AC5	BIT(13)
> +#define PCIE_INT_C_ASSERT_MASK_AC5	BIT(14)
> +#define PCIE_INT_D_ASSERT_MASK_AC5	BIT(15)
> +
> +#define PCIE_ATU_ACCESS_MASK_AC5	GENMASK(21, 20)
>  
>  #define PCIE_ARCACHE_TRC_REG		(PCIE_VENDOR_REGS_OFFSET + 0x50)
>  #define PCIE_AWCACHE_TRC_REG		(PCIE_VENDOR_REGS_OFFSET + 0x54)
> @@ -169,6 +188,7 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	u32 reg;
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct armada8k_pcie *pcie = to_armada8k_pcie(pci);
>  
>  	if (!dw_pcie_link_up(pci)) {
>  		/* Disable LTSSM state machine to enable configuration */
> @@ -177,32 +197,41 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
>  		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, reg);
>  	}
>  
> -	/* Set the device to root complex mode */
> -	reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
> -	reg &= ~(PCIE_DEVICE_TYPE_MASK << PCIE_DEVICE_TYPE_SHIFT);
> -	reg |= PCIE_DEVICE_TYPE_RC << PCIE_DEVICE_TYPE_SHIFT;
> -	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, reg);
> +	if (pcie->pcie_type == ARMADA8K_PCIE_TYPE_A8K) {
> +		/* Set the device to root complex mode */
> +		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
> +		reg &= ~(PCIE_DEVICE_TYPE_MASK << PCIE_DEVICE_TYPE_SHIFT);
> +		reg |= PCIE_DEVICE_TYPE_RC << PCIE_DEVICE_TYPE_SHIFT;
> +		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, reg);
>  
> -	/* Set the PCIe master AxCache attributes */
> -	dw_pcie_writel_dbi(pci, PCIE_ARCACHE_TRC_REG, ARCACHE_DEFAULT_VALUE);
> -	dw_pcie_writel_dbi(pci, PCIE_AWCACHE_TRC_REG, AWCACHE_DEFAULT_VALUE);
> +		/* Set the PCIe master AxCache attributes */
> +		dw_pcie_writel_dbi(pci, PCIE_ARCACHE_TRC_REG, ARCACHE_DEFAULT_VALUE);
> +		dw_pcie_writel_dbi(pci, PCIE_AWCACHE_TRC_REG, AWCACHE_DEFAULT_VALUE);
>  
> -	/* Set the PCIe master AxDomain attributes */
> -	reg = dw_pcie_readl_dbi(pci, PCIE_ARUSER_REG);
> -	reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
> -	reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
> -	dw_pcie_writel_dbi(pci, PCIE_ARUSER_REG, reg);
> +		/* Set the PCIe master AxDomain attributes */
> +		reg = dw_pcie_readl_dbi(pci, PCIE_ARUSER_REG);
> +		reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
> +		reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
> +		dw_pcie_writel_dbi(pci, PCIE_ARUSER_REG, reg);
>  
> -	reg = dw_pcie_readl_dbi(pci, PCIE_AWUSER_REG);
> -	reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
> -	reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
> -	dw_pcie_writel_dbi(pci, PCIE_AWUSER_REG, reg);
> +		reg = dw_pcie_readl_dbi(pci, PCIE_AWUSER_REG);
> +		reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
> +		reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
> +		dw_pcie_writel_dbi(pci, PCIE_AWUSER_REG, reg);
> +	}
>  
>  	/* Enable INT A-D interrupts */
> -	reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG);
> -	reg |= PCIE_INT_A_ASSERT_MASK | PCIE_INT_B_ASSERT_MASK |
> -	       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
> -	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
> +	if (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5) {
> +		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG);
> +		reg |= PCIE_INT_A_ASSERT_MASK_AC5 | PCIE_INT_B_ASSERT_MASK_AC5 |
> +		       PCIE_INT_C_ASSERT_MASK_AC5 | PCIE_INT_D_ASSERT_MASK_AC5;
> +		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG, reg);
> +	} else {
> +		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG);
> +		reg |= PCIE_INT_A_ASSERT_MASK | PCIE_INT_B_ASSERT_MASK |
> +		       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
> +		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
> +	}
>  
>  	return 0;
>  }
> @@ -258,9 +287,61 @@ static int armada8k_add_pcie_port(struct armada8k_pcie *pcie,
>  	return 0;
>  }
>  
> -static const struct dw_pcie_ops dw_pcie_ops = {
> +static u32 ac5_xlate_dbi_reg(u32 reg)
> +{
> +	/* Handle AC5 ATU access */
> +	if ((reg & ~0xfffff) == PCIE_ATU_ACCESS_MASK_AC5) {
> +		reg &= 0xfffff;
> +		/* ATU registers offset is 0xC00 + 0x200 * n,
> +		 * from RFU registers.
> +		 */
> +		reg = 0xc000 | (0x200 * (reg >> 9)) | (reg & 0xff);

A custom ATU-base address can be specified instead of this
brain-cracking hack.

> +	} else if ((reg & 0xfffff000) == PCIE_VENDOR_REGS_OFFSET) {
> +		/* PCIe RFU registers in A8K are at offset 0x8000 from base
> +		 * (0xf2600000) while in AC5 offset is 0x10000 from base
> +		 * (0x800a0000) therefore need the addition of 0x8000.
> +		 */
> +		reg += PCIE_VENDOR_REGS_OFFSET;
> +	}

app/ulbi reg-space could be defined instead.

-Serge(y)

> +
> +	return reg;
> +}
> +
> +static u32 ac5_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> +			     u32 reg, size_t size)
> +{
> +	u32 val;
> +
> +	dw_pcie_read(base + ac5_xlate_dbi_reg(reg), size, &val);
> +	return val;
> +}
> +
> +static void ac5_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> +			       u32 reg, size_t size, u32 val)
> +{
> +	dw_pcie_write(base + ac5_xlate_dbi_reg(reg), size, val);
> +}
> +
> +static const struct dw_pcie_ops armada8k_dw_pcie_ops = {
> +	.link_up = armada8k_pcie_link_up,
> +	.start_link = armada8k_pcie_start_link,
> +};
> +
> +static const struct dw_pcie_ops ac5_dw_pcie_ops = {
>  	.link_up = armada8k_pcie_link_up,
>  	.start_link = armada8k_pcie_start_link,
> +	.read_dbi = ac5_pcie_read_dbi,
> +	.write_dbi = ac5_pcie_write_dbi,
> +};
> +
> +static const struct armada8k_pcie_of_data a8k_pcie_of_data = {
> +	.pcie_type = ARMADA8K_PCIE_TYPE_A8K,
> +	.pcie_ops = &armada8k_dw_pcie_ops,
> +};
> +
> +static const struct armada8k_pcie_of_data ac5_pcie_of_data = {
> +	.pcie_type = ARMADA8K_PCIE_TYPE_AC5,
> +	.pcie_ops = &ac5_dw_pcie_ops,
>  };
>  
>  static int armada8k_pcie_probe(struct platform_device *pdev)
> @@ -268,9 +349,15 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  	struct dw_pcie *pci;
>  	struct armada8k_pcie *pcie;
>  	struct device *dev = &pdev->dev;
> +	const struct armada8k_pcie_of_data *data;
>  	struct resource *base;
>  	int ret;
>  
> +	data = of_device_get_match_data(dev);
> +	if (!data)
> +		return -EINVAL;
> +
> +
>  	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>  	if (!pcie)
>  		return -ENOMEM;
> @@ -279,9 +366,10 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  	if (!pci)
>  		return -ENOMEM;
>  
> +	pci->ops = data->pcie_ops;
>  	pci->dev = dev;
> -	pci->ops = &dw_pcie_ops;
>  
> +	pcie->pcie_type = data->pcie_type;
>  	pcie->pci = pci;
>  
>  	pcie->clk = devm_clk_get(dev, NULL);
> @@ -334,7 +422,14 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id armada8k_pcie_of_match[] = {
> -	{ .compatible = "marvell,armada8k-pcie", },
> +	{
> +		.compatible = "marvell,armada8k-pcie",
> +		.data = &a8k_pcie_of_data,
> +	},
> +	{
> +		.compatible = "marvell,ac5-pcie",
> +		.data = &ac5_pcie_of_data,
> +	},
>  	{},
>  };
>  
> -- 
> 2.17.1
> 
> 

Re: [PATCH v4 3/8] PCI: armada8k: Add AC5 MSI support

[Serge Semin]

On Mon, Mar 13, 2023 at 02:40:11PM +0200, Elad Nachman wrote:
> From: Yuval Shaia <yshaia@marvell.com>
> 
> AC5 requires different handling for MSI as with armada8k.
> Fix it by:
> 
> 1. Enabling the relevant bits in init phase
> 2. Dispatch virtual IRQ handlers when MSI interrupts are received
> 
> Also enable/disable PCIE_APP_LTSSM for AC5.
> 
> Signed-off-by: Yuval Shaia <yshaia@marvell.com>
> Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
> ---
> v4:
>    Fix commit subject to be aligned with previous patch in series
> 
> v2:
>    1) fix W1 warnings which caused by unused leftover code
> 
>    2) fix type in "requieres" word in the description
> 
>  drivers/pci/controller/dwc/pcie-armada8k.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index b9fb1375dc58..02481ecadd25 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -50,6 +50,7 @@ struct armada8k_pcie_of_data {
>  
>  #define PCIE_GLOBAL_CONTROL_REG		(PCIE_VENDOR_REGS_OFFSET + 0x0)
>  #define PCIE_APP_LTSSM_EN		BIT(2)
> +#define PCIE_APP_LTSSM_EN_AC5		BIT(24)
>  #define PCIE_DEVICE_TYPE_SHIFT		4
>  #define PCIE_DEVICE_TYPE_MASK		0xF
>  #define PCIE_DEVICE_TYPE_RC		0x4 /* Root complex */
> @@ -69,6 +70,7 @@ struct armada8k_pcie_of_data {
>  #define PCIE_INT_B_ASSERT_MASK_AC5	BIT(13)
>  #define PCIE_INT_C_ASSERT_MASK_AC5	BIT(14)
>  #define PCIE_INT_D_ASSERT_MASK_AC5	BIT(15)
> +#define PCIE_MSI_MASK_AC5		BIT(11)
>  
>  #define PCIE_ATU_ACCESS_MASK_AC5	GENMASK(21, 20)
>  
> @@ -184,6 +186,16 @@ static int armada8k_pcie_start_link(struct dw_pcie *pci)
>  	return 0;
>  }
>  
> +static void ac5_pcie_msi_init(struct dw_pcie *pci)
> +{
> +	u32 val;
> +
> +	/* Set MSI bit in interrupt mask */
> +	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG);
> +	val |= PCIE_MSI_MASK_AC5;
> +	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, val);
> +}
> +
>  static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	u32 reg;
> @@ -193,7 +205,10 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
>  	if (!dw_pcie_link_up(pci)) {
>  		/* Disable LTSSM state machine to enable configuration */
>  		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);

> -		reg &= ~(PCIE_APP_LTSSM_EN);
> +		if (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5)
> +			reg &= ~(PCIE_APP_LTSSM_EN_AC5);
> +		else
> +			reg &= ~(PCIE_APP_LTSSM_EN);

This has nothing to do with MSIs.

-Serge(y)

>  		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, reg);
>  	}
>  
> @@ -233,6 +248,9 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
>  		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
>  	}
>  
> +	if (IS_ENABLED(CONFIG_PCI_MSI) && (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5))
> +		ac5_pcie_msi_init(pci);
> +
>  	return 0;
>  }
>  
> @@ -249,6 +267,8 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
>  	 */
>  	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG);
>  	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG, val);
> +	if ((PCIE_MSI_MASK_AC5 & val) && (pcie->pcie_type == ARMADA8K_PCIE_TYPE_AC5))
> +		dw_handle_msi_irq(&pci->pp);
>  
>  	return IRQ_HANDLED;
>  }
> -- 
> 2.17.1
> 
> 

Re: [PATCH v4 8/8] PCI: dwc: Introduce region limit from DT

[Serge Semin]

On Mon, Mar 13, 2023 at 02:40:16PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Allow dts override of region limit for SOCs with older Synopsis
> Designware PCIe IP but with greater than 32-bit address range support,
> such as the Armada 7020/7040/8040 family of SOCs by Marvell,
> when the DT file places the PCIe window above the 4GB region.
> The Synopsis Designware PCIe IP in these SOCs is too old to specify the
> highest memory location supported by the PCIe, but practically supports
> such locations. Allow these locations to be specified in the DT file.
> DT property is called num-regionmask , and can range between 33 and 64.

The implemented algorithm doesn't prevents you from specifying the
outbound MW base above 4GB. It prevents you from overflowing the limit
address which is of the 32-bits width only for the chips older v4.60a
and if the INCREASE_REGION_SIZE IP-core synthesize parameter is set to
zero.

In other words you must make sure that dma-ranges/ranges entries size
when combined with the source address doesn't cause the limit address
overflow (4GB boundary cross in your case). For instance, you want to
map 0x1F0000000 CPU-address region of 512MB size to 0x0 PCIe address. In
that case you'd specify the ranges property like this:
< ranges = <0x82000000 0 0 0x1 0xf0000000 0 0x20000000>;
The CPU-base address is ok since iATU always supports 64-bit base
addresses. But after you add 0x20000000 to 0x1f0000000 you'll get the
4GB boundary overflow (0x210000000) which can't be described with the
32-bit limit address CSR. In this particular case the maximum range
size you can specify is 0x10000000 (256MB).

Anyway judging by the patch content you do nothing but hacking the
ranges entries sanity check procedure which I don't think is a good
thing to do.

-Serge(y)

> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
> v4:
>    1) Fix blank lines removal / addition
> 
>    2) Remove usage of variable with same name as dt binding property
> 
>  drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 53a16b8b6ac2..9773c110c733 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -735,8 +735,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
>  void dw_pcie_iatu_detect(struct dw_pcie *pci)
>  {
>  	int max_region, ob, ib;
> -	u32 val, min, dir;
> +	u32 val, min, dir, ret;
>  	u64 max;
> +	struct device *dev = pci->dev;
> +	struct device_node *np = dev->of_node;
>  
>  	val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
>  	if (val == 0xFFFFFFFF) {
> @@ -781,7 +783,13 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)
>  		dw_pcie_writel_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT, 0xFFFFFFFF);
>  		max = dw_pcie_readl_atu(pci, dir, 0, PCIE_ATU_UPPER_LIMIT);
>  	} else {
> -		max = 0;
> +		/* Allow dts override of region limit for older IP with above 32-bit support: */
> +		ret = of_property_read_u32(np, "num-regionmask", &val);
> +		if (!ret && val > 32) {
> +			max = GENMASK(val - 33, 0);
> +			dev_info(pci->dev, "Overriding region limit to %u bits\n", val);
> +		} else
> +			max = 0;
>  	}
>  
>  	pci->num_ob_windows = ob;
> -- 
> 2.17.1
> 
> 

Re: [PATCH v4 7/8] PCI: dwc: Introduce configurable DMA mask

[Robin Murphy]

On 2023-03-17 18:23, Rob Herring wrote:
> +Robin
> 
> On Mon, Mar 13, 2023 at 02:40:15PM +0200, Elad Nachman wrote:
>> From: Elad Nachman <enachman@marvell.com>
>>
>> Some devices, such as AC5 and AC5X have their physical DDR memory
>> start at address 0x2_0000_0000. In order to have the DMA coherent
>> allocation succeed later, a different DMA mask is required, as
>> defined in the DT file for such SOCs, using dma-ranges.
> 
> I'm afraid this is not right. 'dma-ranges' in the PCI host bridge node
> applies to PCI devices (i.e. child node), not the host bridge itself.
> It's 'dma-ranges' in the parent node of the host bridge that applies
> here. The core code will set masks (ranges really now) based on bus
> restrictions. The mask for the device should only be based on the
> device's limits (i.e. the device is 32-bit only).
> 
> I think you will need whatever solution comes out of this thread[1].

Right, "make the allocation succeed later" is entirely missing the point 
of this code. The only reason we're doing that allocation at all is to 
reserve a 32-bit bus address. If it fails, it means we can't reliably 
support endpoints with only a 32-bit MSI capability. Using a bigger mask 
in order to successfully reserve a >32-bit bus address basically 
*guarantees* that you can't support endpoints with only a 32-bit MSI 
capability.

Thanks Rob for digging up that thread; the original idea there should 
still be fine, but the alternative suggestion from Serge at the end is 
potentially even better for this situation where it's down to the SoC's 
memory map rather than the kernel config. It just needs somebody with 
sufficient motivation and resources to write and test a patch :)

Robin.

> 
> Rob
> 
> [1] https://lore.kernel.org/all/c014b074-6d7f-773b-533a-c0500e239ab8@arm.com/