[PATCH v3 0/7] PCI: dwc: Add support for Marvell AC5 SoC

[PATCH v3 0/7] 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 legacy 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.

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 (4):
  dt-bindings: PCI: dwc: add DMA, region mask bits
  PCI: dwc: support AC5 Legacy PCIe interrupts
  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 MSI support for AC5 SoC

 .../devicetree/bindings/pci/pci-armada8k.txt  |   4 +-
 .../bindings/pci/snps,dw-pcie-common.yaml     |  10 +
 drivers/pci/controller/dwc/pcie-armada8k.c    | 184 +++++++++++++++---
 .../pci/controller/dwc/pcie-designware-host.c |  23 ++-
 drivers/pci/controller/dwc/pcie-designware.c  |  13 +-
 5 files changed, 197 insertions(+), 37 deletions(-)

-- 
2.17.1

[PATCH v3 1/7] 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 v3 2/7] 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 v3 3/7] PCI: armada8k: Add MSI support for AC5 SoC

[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>
---
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 v3 4/7] dt-bindings: PCI: dwc: add DMA, region mask bits

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Add properties to support configurable DMA mask bits
and region mask bits.
configurable DMA mask bits is needed for Marvell AC5/AC5X SOCs which
have their physical DDR memory start at address 0x2_0000_0000.
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.
First DT property is called num-dmamask,
and can range between 33 and 64.
Second DT property is called num-regionmask,
and can range between 33 and 64.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
 .../devicetree/bindings/pci/snps,dw-pcie-common.yaml   | 10 ++++++++++
 1 file changed, 10 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..a1b06ff19ca7 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -261,6 +261,16 @@ properties:
 
   dma-coherent: true
 
+  num-dmamask:
+    description: |
+      number of dma mask bits to use, if different than default 32
+    maximum: 64
+
+  num-regionmask:
+    description: |
+      number of region limit mask bits to use, if different than default 32
+    maximum: 64
+
 additionalProperties: true
 
 ...
-- 
2.17.1

Re: [PATCH v3 4/7] dt-bindings: PCI: dwc: add DMA, region mask bits

[Krzysztof Kozlowski]

On 23/02/2023 19:05, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add properties to support configurable DMA mask bits
> and region mask bits.
> configurable DMA mask bits is needed for Marvell AC5/AC5X SOCs which
> have their physical DDR memory start at address 0x2_0000_0000.
> 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.

This formatting is so bad it makes difficult to read. Make these proper
sentences with proper wrapping.


> First DT property is called num-dmamask,
> and can range between 33 and 64.

Wrong mapping and we see it in the code. No need to code it again in
commit msg. Especially that you already said it in the first sentence.

> Second DT property is called num-regionmask,
> and can range between 33 and 64.


> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml   | 10 ++++++++++
>  1 file changed, 10 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..a1b06ff19ca7 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -261,6 +261,16 @@ properties:
>  
>    dma-coherent: true
>  
> +  num-dmamask:
> +    description: |
> +      number of dma mask bits to use, if different than default 32

minimum: 33 (from commit msg)
default: 32... which does not make now sense...

> +    maximum: 64
> +
> +  num-regionmask:
> +    description: |
> +      number of region limit mask bits to use, if different than default 32
> +    maximum: 64
> +
>  additionalProperties: true
>  
>  ...

Best regards,
Krzysztof

Re: [PATCH v3 4/7] dt-bindings: PCI: dwc: add DMA, region mask bits

[Rob Herring]

On Thu, Feb 23, 2023 at 08:05:28PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add properties to support configurable DMA mask bits
> and region mask bits.
> configurable DMA mask bits is needed for Marvell AC5/AC5X SOCs which
> have their physical DDR memory start at address 0x2_0000_0000.
> 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.
> First DT property is called num-dmamask,
> and can range between 33 and 64.
> Second DT property is called num-regionmask,
> and can range between 33 and 64.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml   | 10 ++++++++++
>  1 file changed, 10 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..a1b06ff19ca7 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -261,6 +261,16 @@ properties:
>  
>    dma-coherent: true
>  
> +  num-dmamask:

Nope! There's already a defined way to define DMA/bus addresses and 
sizes in DT. That's dma-ranges.

Rob

[PATCH v3 5/7] PCI: dwc: support AC5 Legacy PCIe interrupts

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Support message emulation of Legacy 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>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 41 +++++++++++++++-------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 02481ecadd25..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)
@@ -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;
 }
@@ -307,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,
@@ -332,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 = {
@@ -418,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;
@@ -429,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

Re: [PATCH v3 5/7] PCI: dwc: support AC5 Legacy PCIe interrupts

[Bjorn Helgaas]

On Thu, Feb 23, 2023 at 08:05:29PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Support message emulation of Legacy 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>
> ---
>  drivers/pci/controller/dwc/pcie-armada8k.c | 41 +++++++++++++++-------
>  1 file changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index 02481ecadd25..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)
> @@ -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;
>  }
> @@ -307,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);

Is this change from 0xC00 to 0xC000 a bug fix?  This purports to only
add functionality.  A bug fix should be split to its own patch.

The armada8k_pcie_irq_handler() change above looks like it goes with
the commit log, but all this other stuff looks like separate logical
changes that should be in separate patches.

> +		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,
> @@ -332,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 = {
> @@ -418,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;
> @@ -429,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 v3 6/7] 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.
If not defined, fallback to 32-bit as previously done in the code.
DT property is called num-dmamask , and can range between 33 and 64.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
 .../pci/controller/dwc/pcie-designware-host.c | 23 ++++++++++++++-----
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 9952057c8819..ac851b065325 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -204,7 +204,6 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
 				    pp->msi_irq_chip,
 				    pp, handle_edge_irq,
 				    NULL, NULL);
-
 	return 0;
 }
 
@@ -250,7 +249,6 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
 		irq_domain_remove(pp->irq_domain);
 		return -ENOMEM;
 	}
-
 	return 0;
 }
 
@@ -325,10 +323,12 @@ 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;
 
 	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
 		pp->irq_mask[ctrl] = ~0;
@@ -367,18 +367,30 @@ 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.
+	 * 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_property_read_u32(np, "num-dmamask", &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");
+		num_dma_maskbits = 32;
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(num_dma_maskbits));
+	if (ret)
+		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);
@@ -420,7 +432,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 		dev_err(dev, "Missing *config* reg space\n");
 		return -ENODEV;
 	}
-
 	bridge = devm_pci_alloc_host_bridge(dev, 0);
 	if (!bridge)
 		return -ENOMEM;
-- 
2.17.1

Re: [PATCH v3 6/7] PCI: dwc: Introduce Configurable DMA mask

[Krzysztof Kozlowski]

On 23/02/2023 19:05, 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

There is no space before full stop and comma. Also fix wrapping.

> coherent allocation succeed later, a different DMA mask is
> required, as defined in the DT file for such SOCs.
> If not defined, fallback to 32-bit as previously done in the code.
> DT property is called num-dmamask , and can range between 33 and 64.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 23 ++++++++++++++-----
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 9952057c8819..ac851b065325 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -204,7 +204,6 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
>  				    pp->msi_irq_chip,
>  				    pp, handle_edge_irq,
>  				    NULL, NULL);
> -

How this is related to the commit?

>  	return 0;
>  }
>  
> @@ -250,7 +249,6 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
>  		irq_domain_remove(pp->irq_domain);
>  		return -ENOMEM;
>  	}
> -

Same problem... and later in the code as well.

Best regards,
Krzysztof

[PATCH v3 7/7] 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>
---
 drivers/pci/controller/dwc/pcie-designware.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 53a16b8b6ac2..429594e853ae 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -401,7 +401,6 @@ static void dw_pcie_writel_atu(struct dw_pcie *pci, u32 dir, u32 index,
 	int ret;
 
 	base = dw_pcie_select_atu(pci, dir, index);
-
 	if (pci->ops && pci->ops->write_dbi) {
 		pci->ops->write_dbi(pci, base, reg, 4, val);
 		return;
@@ -735,10 +734,13 @@ 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, num_region_maskbits;
 	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) {
 		dw_pcie_cap_set(pci, IATU_UNROLL);
 
@@ -781,7 +783,12 @@ 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", &num_region_maskbits);
+		if (!ret && num_region_maskbits > 32)
+			max = GENMASK(num_region_maskbits - 33, 0);
+		else
+			max = 0;
 	}
 
 	pci->num_ob_windows = ob;
-- 
2.17.1

Re: [PATCH v3 7/7] PCI: dwc: Introduce region limit from DT

[Krzysztof Kozlowski]

On 23/02/2023 19:05, 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.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 53a16b8b6ac2..429594e853ae 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -401,7 +401,6 @@ static void dw_pcie_writel_atu(struct dw_pcie *pci, u32 dir, u32 index,
>  	int ret;
>  
>  	base = dw_pcie_select_atu(pci, dir, index);
> -
>  	if (pci->ops && pci->ops->write_dbi) {
>  		pci->ops->write_dbi(pci, base, reg, 4, val);
>  		return;
> @@ -735,10 +734,13 @@ 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, num_region_maskbits;

No need to use num_region_maskbits in function scope.

>  	u64 max;
> +	struct device *dev = pci->dev;
> +	struct device_node *np = dev->of_node;
>  
>  	val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
> +

You need to fix this random changes in unrelated places...

Best regards,
Krzysztof

Re: [PATCH v3 0/7] PCI: dwc: Add support for Marvell AC5 SoC

[Bjorn Helgaas]

On Thu, Feb 23, 2023 at 08:05:24PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add support for AC5 SoC with MSI and in message emulated legacy 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.
> ...

> Elad Nachman (4):
>   dt-bindings: PCI: dwc: add DMA, region mask bits
>   PCI: dwc: support AC5 Legacy PCIe interrupts
>   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 MSI support for AC5 SoC

Capitalize subject consistently.  Use consistent driver tags.  Use
parallel sentence structure.

s/add DMA/Add DMA/
s/PCI: dwc: support/PCI: armada8k: Support/
  (this particular patch only affects armada8k, so don't label it "dwc")
s/support/Support/
s/Configurable/configurable/
s/Add MSI support for AC5 SoC/Add AC5 MSI support/
  (parallel to "Add AC5 SoC support")

The PCIe spec doesn't really use "legacy" when defining the interrupt
model.  I think you're referring to INTx, which it *does* use and is
more specific.  If so, please say "INTx interrupts" instead of "legacy
PCIe interrupts".