2023-03-13 12:43:00

by Elad Nachman

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

From: Elad Nachman <[email protected]>

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



2023-03-13 12:43:05

by Elad Nachman

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

From: Yuval Shaia <[email protected]>

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 <[email protected]>
Signed-off-by: Vadym Kochan <[email protected]>
---
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


2023-03-13 12:43:09

by Elad Nachman

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

From: Vadym Kochan <[email protected]>

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

Signed-off-by: Vadym Kochan <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
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


2023-03-13 12:43:17

by Elad Nachman

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

From: Elad Nachman <[email protected]>

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 <[email protected]>
---
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


2023-03-13 12:43:21

by Elad Nachman

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

From: Elad Nachman <[email protected]>

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 <[email protected]>
---
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


2023-03-13 12:43:24

by Elad Nachman

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

From: Elad Nachman <[email protected]>

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

Signed-off-by: Elad Nachman <[email protected]>
---
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


2023-03-13 12:43:28

by Elad Nachman

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

From: Elad Nachman <[email protected]>

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 <[email protected]>
---
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


2023-03-13 19:22:46

by Bjorn Helgaas

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

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.

2023-03-13 19:48:09

by Bjorn Helgaas

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

[+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 <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>

2023-03-14 20:48:52

by Serge Semin

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

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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
> >
>


2023-03-17 18:30:46

by Rob Herring (Arm)

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

On Mon, Mar 13, 2023 at 02:40:12PM +0200, Elad Nachman wrote:
> From: Elad Nachman <[email protected]>
>
> 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 <[email protected]>
> ---
> 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

2023-03-22 23:29:12

by Serge Semin

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

On Mon, Mar 13, 2023 at 02:40:11PM +0200, Elad Nachman wrote:
> From: Yuval Shaia <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Vadym Kochan <[email protected]>
> ---
> 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
>
>

2023-03-23 00:15:54

by Serge Semin

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

On Mon, Mar 13, 2023 at 02:40:16PM +0200, Elad Nachman wrote:
> From: Elad Nachman <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
>