[PATCH 0/2] PCI: aardvark: Resource fixes

[PATCH 0/2] PCI: aardvark: Resource fixes

[Pali Rohár]

This patch series fixes configuring PCIe resources (IO and MEM) in
aardvark controller driver. It is required to initialize BARs on systems
with more cards, e.g. NVMe disks and WiFi AX cards.

Pali Rohár (2):
  PCI: aardvark: Configure PCIe resources from 'ranges' DT property
  arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

 .../dts/marvell/armada-3720-turris-mox.dts    |  17 ++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |  11 +-
 drivers/pci/controller/pci-aardvark.c         | 195 +++++++++++++++++-
 3 files changed, 220 insertions(+), 3 deletions(-)

-- 
2.20.1

[PATCH 0/2] PCI: aardvark: Resource fixes

[Pali Rohár]

This patch series fixes configuring PCIe resources (IO and MEM) in
aardvark controller driver. It is required to initialize BARs on systems
with more cards, e.g. NVMe disks and WiFi AX cards.

Pali Rohár (2):
  PCI: aardvark: Configure PCIe resources from 'ranges' DT property
  arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

 .../dts/marvell/armada-3720-turris-mox.dts    |  17 ++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |  11 +-
 drivers/pci/controller/pci-aardvark.c         | 195 +++++++++++++++++-
 3 files changed, 220 insertions(+), 3 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] PCI: aardvark: Configure PCIe resources from 'ranges' DT property

[Pali Rohár]

In commit 6df6ba974a55 ("PCI: aardvark: Remove PCIe outbound window
configuration") was removed aardvark PCIe outbound window configuration and
commit description said that was recommended solution by HW designers.

But that commit completely removed support for configuring PCIe IO
resources without removing PCIe IO 'ranges' from DTS files. After that
commit PCIe IO space started to be treated as PCIe MEM space and accessing
it just caused kernel crash.

Moreover implementation of PCIe outbound windows prior that commit was
incorrect. It completely ignored offset between CPU address and PCIe bus
address and expected that in DTS is CPU address always same as PCIe bus
address without doing any checks. Also it completely ignored size of every
PCIe resource specified in 'ranges' DTS property and expected that every
PCIe resource has size 128 MB (also for PCIe IO range). Again without any
check. Apparently none of PCIe resource has in DTS specified size of 128
MB. So it was completely broken and thanks to how aardvark mask works,
configuration was completely ignored.

This patch reverts back support for PCIe outbound window configuration but
implementation is a new without issues mentioned above. PCIe outbound
window is required when DTS specify in 'ranges' property non-zero offset
between CPU and PCIe address space. To address recommendation by HW
designers as specified in commit description of 6df6ba974a55, set default
outbound parameters as PCIe MEM access without translation and therefore
for this PCIe 'ranges' it is not needed to configure PCIe outbound window.
For PCIe IO space is needed to configure aardvark PCIe outbound window.

This patch fixes kernel crash when trying to access PCIe IO space.

Signed-off-by: Pali Rohár <pali@kernel.org>
Cc: stable@vger.kernel.org # 6df6ba974a55 ("PCI: aardvark: Remove PCIe outbound window configuration")
---
 drivers/pci/controller/pci-aardvark.c | 195 +++++++++++++++++++++++++-
 1 file changed, 194 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 3f3c72927afb..b4da496360f0 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -119,6 +119,46 @@
 #define PCIE_MSI_MASK_REG			(CONTROL_BASE_ADDR + 0x5C)
 #define PCIE_MSI_PAYLOAD_REG			(CONTROL_BASE_ADDR + 0x9C)
 
+/* PCIe window configuration */
+#define OB_WIN_BASE_ADDR			0x4c00
+#define OB_WIN_BLOCK_SIZE			0x20
+#define OB_WIN_COUNT				8
+#define OB_WIN_REG_ADDR(win, offset)		(OB_WIN_BASE_ADDR + \
+						 OB_WIN_BLOCK_SIZE * (win) + \
+						 (offset))
+#define OB_WIN_MATCH_LS(win)			OB_WIN_REG_ADDR(win, 0x00)
+#define     OB_WIN_ENABLE			BIT(0)
+#define OB_WIN_MATCH_MS(win)			OB_WIN_REG_ADDR(win, 0x04)
+#define OB_WIN_REMAP_LS(win)			OB_WIN_REG_ADDR(win, 0x08)
+#define OB_WIN_REMAP_MS(win)			OB_WIN_REG_ADDR(win, 0x0c)
+#define OB_WIN_MASK_LS(win)			OB_WIN_REG_ADDR(win, 0x10)
+#define OB_WIN_MASK_MS(win)			OB_WIN_REG_ADDR(win, 0x14)
+#define OB_WIN_ACTIONS(win)			OB_WIN_REG_ADDR(win, 0x18)
+#define OB_WIN_DEFAULT_ACTIONS			(OB_WIN_ACTIONS(OB_WIN_COUNT-1) + 0x4)
+#define     OB_WIN_FUNC_NUM_MASK		GENMASK(31, 24)
+#define     OB_WIN_FUNC_NUM_SHIFT		24
+#define     OB_WIN_FUNC_NUM_ENABLE		BIT(23)
+#define     OB_WIN_BUS_NUM_BITS_MASK		GENMASK(22, 20)
+#define     OB_WIN_BUS_NUM_BITS_SHIFT		20
+#define     OB_WIN_MSG_CODE_ENABLE		BIT(22)
+#define     OB_WIN_MSG_CODE_MASK		GENMASK(21, 14)
+#define     OB_WIN_MSG_CODE_SHIFT		14
+#define     OB_WIN_MSG_PAYLOAD_LEN		BIT(12)
+#define     OB_WIN_ATTR_ENABLE			BIT(11)
+#define     OB_WIN_ATTR_TC_MASK			GENMASK(10, 8)
+#define     OB_WIN_ATTR_TC_SHIFT		8
+#define     OB_WIN_ATTR_RELAXED			BIT(7)
+#define     OB_WIN_ATTR_NOSNOOP			BIT(6)
+#define     OB_WIN_ATTR_POISON			BIT(5)
+#define     OB_WIN_ATTR_IDO			BIT(4)
+#define     OB_WIN_TYPE_MASK			GENMASK(3, 0)
+#define     OB_WIN_TYPE_SHIFT			0
+#define     OB_WIN_TYPE_MEM			0x0
+#define     OB_WIN_TYPE_IO			0x4
+#define     OB_WIN_TYPE_CONFIG_TYPE0		0x8
+#define     OB_WIN_TYPE_CONFIG_TYPE1		0x9
+#define     OB_WIN_TYPE_MSG			0xc
+
 /* LMI registers base address and register offsets */
 #define LMI_BASE_ADDR				0x6000
 #define CFG_REG					(LMI_BASE_ADDR + 0x0)
@@ -183,6 +223,13 @@
 struct advk_pcie {
 	struct platform_device *pdev;
 	void __iomem *base;
+	struct {
+		phys_addr_t match;
+		phys_addr_t remap;
+		phys_addr_t mask;
+		u32 actions;
+	} wins[OB_WIN_COUNT];
+	u8 wins_count;
 	struct irq_domain *irq_domain;
 	struct irq_chip irq_chip;
 	struct irq_domain *msi_domain;
@@ -369,9 +416,39 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
 	dev_err(dev, "link never came up\n");
 }
 
+/*
+ * Set PCIe address window register which could be used for memory
+ * mapping.
+ */
+static void advk_pcie_set_ob_win(struct advk_pcie *pcie, u8 win_num,
+				 phys_addr_t match, phys_addr_t remap,
+				 phys_addr_t mask, u32 actions)
+{
+	advk_writel(pcie, OB_WIN_ENABLE |
+			  lower_32_bits(match), OB_WIN_MATCH_LS(win_num));
+	advk_writel(pcie, upper_32_bits(match), OB_WIN_MATCH_MS(win_num));
+	advk_writel(pcie, lower_32_bits(remap), OB_WIN_REMAP_LS(win_num));
+	advk_writel(pcie, upper_32_bits(remap), OB_WIN_REMAP_MS(win_num));
+	advk_writel(pcie, lower_32_bits(mask), OB_WIN_MASK_LS(win_num));
+	advk_writel(pcie, upper_32_bits(mask), OB_WIN_MASK_MS(win_num));
+	advk_writel(pcie, actions, OB_WIN_ACTIONS(win_num));
+}
+
+static void advk_pcie_disable_ob_win(struct advk_pcie *pcie, u8 win_num)
+{
+	advk_writel(pcie, 0, OB_WIN_MATCH_LS(win_num));
+	advk_writel(pcie, 0, OB_WIN_MATCH_MS(win_num));
+	advk_writel(pcie, 0, OB_WIN_REMAP_LS(win_num));
+	advk_writel(pcie, 0, OB_WIN_REMAP_MS(win_num));
+	advk_writel(pcie, 0, OB_WIN_MASK_LS(win_num));
+	advk_writel(pcie, 0, OB_WIN_MASK_MS(win_num));
+	advk_writel(pcie, 0, OB_WIN_ACTIONS(win_num));
+}
+
 static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 {
 	u32 reg;
+	int i;
 
 	/* Enable TX */
 	reg = advk_readl(pcie, PCIE_CORE_REF_CLK_REG);
@@ -440,15 +517,51 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 	reg = PCIE_IRQ_ALL_MASK & (~PCIE_IRQ_ENABLE_INTS_MASK);
 	advk_writel(pcie, reg, HOST_CTRL_INT_MASK_REG);
 
+	/*
+	 * Enable AXI address window location generation:
+	 * When it is enabled, the default outbound window
+	 * configurations (Default User Field: 0xD0074CFC)
+	 * are used to transparent address translation for
+	 * the outbound transactions. Thus, PCIe address
+	 * windows are not required for transparent memory
+	 * access when default outbound window configuration
+	 * is set for memory access.
+	 */
 	reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG);
 	reg |= PCIE_CORE_CTRL2_OB_WIN_ENABLE;
 	advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
 
-	/* Bypass the address window mapping for PIO */
+	/*
+	 * Set memory access in Default User Field so it
+	 * is not required to configure PCIe address for
+	 * transparent memory access.
+	 */
+	advk_writel(pcie, OB_WIN_TYPE_MEM, OB_WIN_DEFAULT_ACTIONS);
+
+	/*
+	 * Bypass the address window mapping for PIO:
+	 * Since PIO access already contains all required
+	 * info over AXI interface by PIO registers, the
+	 * address window is not required.
+	 */
 	reg = advk_readl(pcie, PIO_CTRL);
 	reg |= PIO_CTRL_ADDR_WIN_DISABLE;
 	advk_writel(pcie, reg, PIO_CTRL);
 
+	/*
+	 * Configure PCIe address windows for non-memory or
+	 * non-transparent access as by default PCIe uses
+	 * transparent memory access.
+	 */
+	for (i = 0; i < pcie->wins_count; i++)
+		advk_pcie_set_ob_win(pcie, i,
+				     pcie->wins[i].match, pcie->wins[i].remap,
+				     pcie->wins[i].mask, pcie->wins[i].actions);
+
+	/* Disable remaining PCIe outbound windows */
+	for (i = pcie->wins_count; i < OB_WIN_COUNT; i++)
+		advk_pcie_disable_ob_win(pcie, i);
+
 	advk_pcie_train_link(pcie);
 
 	/*
@@ -1218,6 +1331,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct advk_pcie *pcie;
 	struct pci_host_bridge *bridge;
+	struct resource_entry *entry;
 	int ret, irq;
 
 	bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct advk_pcie));
@@ -1228,6 +1342,80 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	pcie->pdev = pdev;
 	platform_set_drvdata(pdev, pcie);
 
+	resource_list_for_each_entry(entry, &bridge->windows) {
+		resource_size_t start = entry->res->start;
+		resource_size_t size = resource_size(entry->res);
+		unsigned long type = resource_type(entry->res);
+		u64 win_size;
+
+		/*
+		 * Aardvark hardware allows to configure also PCIe window
+		 * for config type 0 and type 1 mapping, but driver uses
+		 * only PIO for issuing configuration transfers which does
+		 * not use PCIe window configuration.
+		 */
+		if (type != IORESOURCE_MEM && type != IORESOURCE_MEM_64 &&
+		    type != IORESOURCE_IO)
+			continue;
+
+		/*
+		 * Skip transparent memory resources. Default outbound access
+		 * configuration is set to transparent memory access so it
+		 * does not need window configuration.
+		 */
+		if ((type == IORESOURCE_MEM || type == IORESOURCE_MEM_64) &&
+		    entry->offset == 0)
+			continue;
+
+		/*
+		 * The n-th PCIe window is configured by tuple (match, remap, mask)
+		 * and an access to address A uses this window if A matches the
+		 * match with given mask.
+		 * So every PCIe window size must be a power of two and every start
+		 * address must be aligned to window size. Minimal size is 64 KiB
+		 * because lower 16 bits of mask must be zero. Remapped address
+		 * may have set only bits from the mask.
+		 */
+		while (pcie->wins_count < OB_WIN_COUNT && size > 0) {
+			/* Calculate the largest aligned window size */
+			win_size = (1ULL << (fls64(size)-1)) |
+				   (start ? (1ULL << __ffs64(start)) : 0);
+			win_size = 1ULL << __ffs64(win_size);
+			if (win_size < 0x10000)
+				break;
+
+			dev_dbg(dev,
+				"Configuring PCIe window %d: [0x%llx-0x%llx] as %lu\n",
+				pcie->wins_count, (unsigned long long)start,
+				(unsigned long long)start + win_size, type);
+
+			if (type == IORESOURCE_IO) {
+				pcie->wins[pcie->wins_count].actions = OB_WIN_TYPE_IO;
+				pcie->wins[pcie->wins_count].match = pci_pio_to_address(start);
+			} else {
+				pcie->wins[pcie->wins_count].actions = OB_WIN_TYPE_MEM;
+				pcie->wins[pcie->wins_count].match = start;
+			}
+			pcie->wins[pcie->wins_count].remap = start - entry->offset;
+			pcie->wins[pcie->wins_count].mask = ~(win_size - 1);
+
+			if (pcie->wins[pcie->wins_count].remap & (win_size - 1))
+				break;
+
+			start += win_size;
+			size -= win_size;
+			pcie->wins_count++;
+		}
+
+		if (size > 0) {
+			dev_err(&pcie->pdev->dev,
+				"Invalid PCIe region [0x%llx-0x%llx]\n",
+				(unsigned long long)entry->res->start,
+				(unsigned long long)entry->res->end + 1);
+			return -EINVAL;
+		}
+	}
+
 	pcie->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pcie->base))
 		return PTR_ERR(pcie->base);
@@ -1308,6 +1496,7 @@ static int advk_pcie_remove(struct platform_device *pdev)
 {
 	struct advk_pcie *pcie = platform_get_drvdata(pdev);
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+	int i;
 
 	pci_lock_rescan_remove();
 	pci_stop_root_bus(bridge->bus);
@@ -1317,6 +1506,10 @@ static int advk_pcie_remove(struct platform_device *pdev)
 	advk_pcie_remove_msi_irq_domain(pcie);
 	advk_pcie_remove_irq_domain(pcie);
 
+	/* Disable outbound address windows mapping */
+	for (i = 0; i < OB_WIN_COUNT; i++)
+		advk_pcie_disable_ob_win(pcie, i);
+
 	return 0;
 }
 
-- 
2.20.1

[PATCH 1/2] PCI: aardvark: Configure PCIe resources from 'ranges' DT property

[Pali Rohár]

In commit 6df6ba974a55 ("PCI: aardvark: Remove PCIe outbound window
configuration") was removed aardvark PCIe outbound window configuration and
commit description said that was recommended solution by HW designers.

But that commit completely removed support for configuring PCIe IO
resources without removing PCIe IO 'ranges' from DTS files. After that
commit PCIe IO space started to be treated as PCIe MEM space and accessing
it just caused kernel crash.

Moreover implementation of PCIe outbound windows prior that commit was
incorrect. It completely ignored offset between CPU address and PCIe bus
address and expected that in DTS is CPU address always same as PCIe bus
address without doing any checks. Also it completely ignored size of every
PCIe resource specified in 'ranges' DTS property and expected that every
PCIe resource has size 128 MB (also for PCIe IO range). Again without any
check. Apparently none of PCIe resource has in DTS specified size of 128
MB. So it was completely broken and thanks to how aardvark mask works,
configuration was completely ignored.

This patch reverts back support for PCIe outbound window configuration but
implementation is a new without issues mentioned above. PCIe outbound
window is required when DTS specify in 'ranges' property non-zero offset
between CPU and PCIe address space. To address recommendation by HW
designers as specified in commit description of 6df6ba974a55, set default
outbound parameters as PCIe MEM access without translation and therefore
for this PCIe 'ranges' it is not needed to configure PCIe outbound window.
For PCIe IO space is needed to configure aardvark PCIe outbound window.

This patch fixes kernel crash when trying to access PCIe IO space.

Signed-off-by: Pali Rohár <pali@kernel.org>
Cc: stable@vger.kernel.org # 6df6ba974a55 ("PCI: aardvark: Remove PCIe outbound window configuration")
---
 drivers/pci/controller/pci-aardvark.c | 195 +++++++++++++++++++++++++-
 1 file changed, 194 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 3f3c72927afb..b4da496360f0 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -119,6 +119,46 @@
 #define PCIE_MSI_MASK_REG			(CONTROL_BASE_ADDR + 0x5C)
 #define PCIE_MSI_PAYLOAD_REG			(CONTROL_BASE_ADDR + 0x9C)
 
+/* PCIe window configuration */
+#define OB_WIN_BASE_ADDR			0x4c00
+#define OB_WIN_BLOCK_SIZE			0x20
+#define OB_WIN_COUNT				8
+#define OB_WIN_REG_ADDR(win, offset)		(OB_WIN_BASE_ADDR + \
+						 OB_WIN_BLOCK_SIZE * (win) + \
+						 (offset))
+#define OB_WIN_MATCH_LS(win)			OB_WIN_REG_ADDR(win, 0x00)
+#define     OB_WIN_ENABLE			BIT(0)
+#define OB_WIN_MATCH_MS(win)			OB_WIN_REG_ADDR(win, 0x04)
+#define OB_WIN_REMAP_LS(win)			OB_WIN_REG_ADDR(win, 0x08)
+#define OB_WIN_REMAP_MS(win)			OB_WIN_REG_ADDR(win, 0x0c)
+#define OB_WIN_MASK_LS(win)			OB_WIN_REG_ADDR(win, 0x10)
+#define OB_WIN_MASK_MS(win)			OB_WIN_REG_ADDR(win, 0x14)
+#define OB_WIN_ACTIONS(win)			OB_WIN_REG_ADDR(win, 0x18)
+#define OB_WIN_DEFAULT_ACTIONS			(OB_WIN_ACTIONS(OB_WIN_COUNT-1) + 0x4)
+#define     OB_WIN_FUNC_NUM_MASK		GENMASK(31, 24)
+#define     OB_WIN_FUNC_NUM_SHIFT		24
+#define     OB_WIN_FUNC_NUM_ENABLE		BIT(23)
+#define     OB_WIN_BUS_NUM_BITS_MASK		GENMASK(22, 20)
+#define     OB_WIN_BUS_NUM_BITS_SHIFT		20
+#define     OB_WIN_MSG_CODE_ENABLE		BIT(22)
+#define     OB_WIN_MSG_CODE_MASK		GENMASK(21, 14)
+#define     OB_WIN_MSG_CODE_SHIFT		14
+#define     OB_WIN_MSG_PAYLOAD_LEN		BIT(12)
+#define     OB_WIN_ATTR_ENABLE			BIT(11)
+#define     OB_WIN_ATTR_TC_MASK			GENMASK(10, 8)
+#define     OB_WIN_ATTR_TC_SHIFT		8
+#define     OB_WIN_ATTR_RELAXED			BIT(7)
+#define     OB_WIN_ATTR_NOSNOOP			BIT(6)
+#define     OB_WIN_ATTR_POISON			BIT(5)
+#define     OB_WIN_ATTR_IDO			BIT(4)
+#define     OB_WIN_TYPE_MASK			GENMASK(3, 0)
+#define     OB_WIN_TYPE_SHIFT			0
+#define     OB_WIN_TYPE_MEM			0x0
+#define     OB_WIN_TYPE_IO			0x4
+#define     OB_WIN_TYPE_CONFIG_TYPE0		0x8
+#define     OB_WIN_TYPE_CONFIG_TYPE1		0x9
+#define     OB_WIN_TYPE_MSG			0xc
+
 /* LMI registers base address and register offsets */
 #define LMI_BASE_ADDR				0x6000
 #define CFG_REG					(LMI_BASE_ADDR + 0x0)
@@ -183,6 +223,13 @@
 struct advk_pcie {
 	struct platform_device *pdev;
 	void __iomem *base;
+	struct {
+		phys_addr_t match;
+		phys_addr_t remap;
+		phys_addr_t mask;
+		u32 actions;
+	} wins[OB_WIN_COUNT];
+	u8 wins_count;
 	struct irq_domain *irq_domain;
 	struct irq_chip irq_chip;
 	struct irq_domain *msi_domain;
@@ -369,9 +416,39 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
 	dev_err(dev, "link never came up\n");
 }
 
+/*
+ * Set PCIe address window register which could be used for memory
+ * mapping.
+ */
+static void advk_pcie_set_ob_win(struct advk_pcie *pcie, u8 win_num,
+				 phys_addr_t match, phys_addr_t remap,
+				 phys_addr_t mask, u32 actions)
+{
+	advk_writel(pcie, OB_WIN_ENABLE |
+			  lower_32_bits(match), OB_WIN_MATCH_LS(win_num));
+	advk_writel(pcie, upper_32_bits(match), OB_WIN_MATCH_MS(win_num));
+	advk_writel(pcie, lower_32_bits(remap), OB_WIN_REMAP_LS(win_num));
+	advk_writel(pcie, upper_32_bits(remap), OB_WIN_REMAP_MS(win_num));
+	advk_writel(pcie, lower_32_bits(mask), OB_WIN_MASK_LS(win_num));
+	advk_writel(pcie, upper_32_bits(mask), OB_WIN_MASK_MS(win_num));
+	advk_writel(pcie, actions, OB_WIN_ACTIONS(win_num));
+}
+
+static void advk_pcie_disable_ob_win(struct advk_pcie *pcie, u8 win_num)
+{
+	advk_writel(pcie, 0, OB_WIN_MATCH_LS(win_num));
+	advk_writel(pcie, 0, OB_WIN_MATCH_MS(win_num));
+	advk_writel(pcie, 0, OB_WIN_REMAP_LS(win_num));
+	advk_writel(pcie, 0, OB_WIN_REMAP_MS(win_num));
+	advk_writel(pcie, 0, OB_WIN_MASK_LS(win_num));
+	advk_writel(pcie, 0, OB_WIN_MASK_MS(win_num));
+	advk_writel(pcie, 0, OB_WIN_ACTIONS(win_num));
+}
+
 static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 {
 	u32 reg;
+	int i;
 
 	/* Enable TX */
 	reg = advk_readl(pcie, PCIE_CORE_REF_CLK_REG);
@@ -440,15 +517,51 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 	reg = PCIE_IRQ_ALL_MASK & (~PCIE_IRQ_ENABLE_INTS_MASK);
 	advk_writel(pcie, reg, HOST_CTRL_INT_MASK_REG);
 
+	/*
+	 * Enable AXI address window location generation:
+	 * When it is enabled, the default outbound window
+	 * configurations (Default User Field: 0xD0074CFC)
+	 * are used to transparent address translation for
+	 * the outbound transactions. Thus, PCIe address
+	 * windows are not required for transparent memory
+	 * access when default outbound window configuration
+	 * is set for memory access.
+	 */
 	reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG);
 	reg |= PCIE_CORE_CTRL2_OB_WIN_ENABLE;
 	advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
 
-	/* Bypass the address window mapping for PIO */
+	/*
+	 * Set memory access in Default User Field so it
+	 * is not required to configure PCIe address for
+	 * transparent memory access.
+	 */
+	advk_writel(pcie, OB_WIN_TYPE_MEM, OB_WIN_DEFAULT_ACTIONS);
+
+	/*
+	 * Bypass the address window mapping for PIO:
+	 * Since PIO access already contains all required
+	 * info over AXI interface by PIO registers, the
+	 * address window is not required.
+	 */
 	reg = advk_readl(pcie, PIO_CTRL);
 	reg |= PIO_CTRL_ADDR_WIN_DISABLE;
 	advk_writel(pcie, reg, PIO_CTRL);
 
+	/*
+	 * Configure PCIe address windows for non-memory or
+	 * non-transparent access as by default PCIe uses
+	 * transparent memory access.
+	 */
+	for (i = 0; i < pcie->wins_count; i++)
+		advk_pcie_set_ob_win(pcie, i,
+				     pcie->wins[i].match, pcie->wins[i].remap,
+				     pcie->wins[i].mask, pcie->wins[i].actions);
+
+	/* Disable remaining PCIe outbound windows */
+	for (i = pcie->wins_count; i < OB_WIN_COUNT; i++)
+		advk_pcie_disable_ob_win(pcie, i);
+
 	advk_pcie_train_link(pcie);
 
 	/*
@@ -1218,6 +1331,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct advk_pcie *pcie;
 	struct pci_host_bridge *bridge;
+	struct resource_entry *entry;
 	int ret, irq;
 
 	bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct advk_pcie));
@@ -1228,6 +1342,80 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	pcie->pdev = pdev;
 	platform_set_drvdata(pdev, pcie);
 
+	resource_list_for_each_entry(entry, &bridge->windows) {
+		resource_size_t start = entry->res->start;
+		resource_size_t size = resource_size(entry->res);
+		unsigned long type = resource_type(entry->res);
+		u64 win_size;
+
+		/*
+		 * Aardvark hardware allows to configure also PCIe window
+		 * for config type 0 and type 1 mapping, but driver uses
+		 * only PIO for issuing configuration transfers which does
+		 * not use PCIe window configuration.
+		 */
+		if (type != IORESOURCE_MEM && type != IORESOURCE_MEM_64 &&
+		    type != IORESOURCE_IO)
+			continue;
+
+		/*
+		 * Skip transparent memory resources. Default outbound access
+		 * configuration is set to transparent memory access so it
+		 * does not need window configuration.
+		 */
+		if ((type == IORESOURCE_MEM || type == IORESOURCE_MEM_64) &&
+		    entry->offset == 0)
+			continue;
+
+		/*
+		 * The n-th PCIe window is configured by tuple (match, remap, mask)
+		 * and an access to address A uses this window if A matches the
+		 * match with given mask.
+		 * So every PCIe window size must be a power of two and every start
+		 * address must be aligned to window size. Minimal size is 64 KiB
+		 * because lower 16 bits of mask must be zero. Remapped address
+		 * may have set only bits from the mask.
+		 */
+		while (pcie->wins_count < OB_WIN_COUNT && size > 0) {
+			/* Calculate the largest aligned window size */
+			win_size = (1ULL << (fls64(size)-1)) |
+				   (start ? (1ULL << __ffs64(start)) : 0);
+			win_size = 1ULL << __ffs64(win_size);
+			if (win_size < 0x10000)
+				break;
+
+			dev_dbg(dev,
+				"Configuring PCIe window %d: [0x%llx-0x%llx] as %lu\n",
+				pcie->wins_count, (unsigned long long)start,
+				(unsigned long long)start + win_size, type);
+
+			if (type == IORESOURCE_IO) {
+				pcie->wins[pcie->wins_count].actions = OB_WIN_TYPE_IO;
+				pcie->wins[pcie->wins_count].match = pci_pio_to_address(start);
+			} else {
+				pcie->wins[pcie->wins_count].actions = OB_WIN_TYPE_MEM;
+				pcie->wins[pcie->wins_count].match = start;
+			}
+			pcie->wins[pcie->wins_count].remap = start - entry->offset;
+			pcie->wins[pcie->wins_count].mask = ~(win_size - 1);
+
+			if (pcie->wins[pcie->wins_count].remap & (win_size - 1))
+				break;
+
+			start += win_size;
+			size -= win_size;
+			pcie->wins_count++;
+		}
+
+		if (size > 0) {
+			dev_err(&pcie->pdev->dev,
+				"Invalid PCIe region [0x%llx-0x%llx]\n",
+				(unsigned long long)entry->res->start,
+				(unsigned long long)entry->res->end + 1);
+			return -EINVAL;
+		}
+	}
+
 	pcie->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pcie->base))
 		return PTR_ERR(pcie->base);
@@ -1308,6 +1496,7 @@ static int advk_pcie_remove(struct platform_device *pdev)
 {
 	struct advk_pcie *pcie = platform_get_drvdata(pdev);
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+	int i;
 
 	pci_lock_rescan_remove();
 	pci_stop_root_bus(bridge->bus);
@@ -1317,6 +1506,10 @@ static int advk_pcie_remove(struct platform_device *pdev)
 	advk_pcie_remove_msi_irq_domain(pcie);
 	advk_pcie_remove_irq_domain(pcie);
 
+	/* Disable outbound address windows mapping */
+	for (i = 0; i < OB_WIN_COUNT; i++)
+		advk_pcie_disable_ob_win(pcie, i);
+
 	return 0;
 }
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

Current PCIe MEM space of size 16 MB is not enough for some combination
of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
allocate more PCIe BARs for more PCIe cards.

Without this change some combination of PCIe cards cannot be used and
kernel show error messages in dmesg during initialization:

    pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
    pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
    pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
    pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
    pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
    pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
    pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
    pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
    pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
    pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
    pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]

Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
crashes during loading of kernel DTB file. This bug is present only in
U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
affected by this bug. Bug is fixed in U-Boot version 2021.07.

To not break booting new kernels on existing versions of U-Boot on Turris
Mox, use first 16 MB range for IO and second range with rest of PCIe window
for MEM.

Signed-off-by: Pali Rohár <pali@kernel.org>
Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
---
 .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 53e817c5f6f3..86b3025f174b 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -134,6 +134,23 @@
 	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
 	status = "okay";
 	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
+	/*
+	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
+	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
+	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
+	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
+	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
+	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
+	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
+	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
+	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
+	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
+	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
+	 */
+	#address-cells = <3>;
+	#size-cells = <2>;
+	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
+		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
 
 	/* enabled by U-Boot if PCIe module is present */
 	status = "disabled";
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 7a2df148c6a3..dac3007f2ac1 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -488,8 +488,15 @@
 			#interrupt-cells = <1>;
 			msi-parent = <&pcie0>;
 			msi-controller;
-			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
-				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
+			/*
+			 * The 128 MiB address range [0xe8000000-0xf0000000] is
+			 * dedicated for PCIe and can be assigned to 8 windows
+			 * with size a power of two. Use one 64 KiB window for
+			 * IO at the end and the remaining seven windows
+			 * (totaling 127 MiB) for MEM.
+			 */
+			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
+				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
 			interrupt-map-mask = <0 0 0 7>;
 			interrupt-map = <0 0 0 1 &pcie_intc 0>,
 					<0 0 0 2 &pcie_intc 1>,
-- 
2.20.1

[PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

Current PCIe MEM space of size 16 MB is not enough for some combination
of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
allocate more PCIe BARs for more PCIe cards.

Without this change some combination of PCIe cards cannot be used and
kernel show error messages in dmesg during initialization:

    pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
    pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
    pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
    pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
    pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
    pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
    pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
    pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
    pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
    pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
    pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]

Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
crashes during loading of kernel DTB file. This bug is present only in
U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
affected by this bug. Bug is fixed in U-Boot version 2021.07.

To not break booting new kernels on existing versions of U-Boot on Turris
Mox, use first 16 MB range for IO and second range with rest of PCIe window
for MEM.

Signed-off-by: Pali Rohár <pali@kernel.org>
Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
---
 .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 53e817c5f6f3..86b3025f174b 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -134,6 +134,23 @@
 	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
 	status = "okay";
 	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
+	/*
+	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
+	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
+	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
+	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
+	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
+	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
+	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
+	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
+	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
+	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
+	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
+	 */
+	#address-cells = <3>;
+	#size-cells = <2>;
+	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
+		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
 
 	/* enabled by U-Boot if PCIe module is present */
 	status = "disabled";
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 7a2df148c6a3..dac3007f2ac1 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -488,8 +488,15 @@
 			#interrupt-cells = <1>;
 			msi-parent = <&pcie0>;
 			msi-controller;
-			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
-				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
+			/*
+			 * The 128 MiB address range [0xe8000000-0xf0000000] is
+			 * dedicated for PCIe and can be assigned to 8 windows
+			 * with size a power of two. Use one 64 KiB window for
+			 * IO at the end and the remaining seven windows
+			 * (totaling 127 MiB) for MEM.
+			 */
+			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
+				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
 			interrupt-map-mask = <0 0 0 7>;
 			interrupt-map = <0 0 0 1 &pcie_intc 0>,
 					<0 0 0 2 &pcie_intc 1>,
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Gregory CLEMENT]

Hello Pali,

> Current PCIe MEM space of size 16 MB is not enough for some combination
> of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> allocate more PCIe BARs for more PCIe cards.
>
> Without this change some combination of PCIe cards cannot be used and
> kernel show error messages in dmesg during initialization:
>
>     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
>     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
>     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
>     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
>     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
>     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
>     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
>     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
>     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
>     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
>     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
>
> Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> crashes during loading of kernel DTB file. This bug is present only in
> U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> affected by this bug. Bug is fixed in U-Boot version 2021.07.
>
> To not break booting new kernels on existing versions of U-Boot on Turris
> Mox, use first 16 MB range for IO and second range with rest of PCIe window
> for MEM.

Is there any depencey with the firs patch of this series ?

What happend if this patch is applied without the other ?
Could you test it to see if any regression occure ?

Thanks,

Grégory

>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> ---
>  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 53e817c5f6f3..86b3025f174b 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -134,6 +134,23 @@
>  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
>  	status = "okay";
>  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> +	/*
> +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> +	 */
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
>  
>  	/* enabled by U-Boot if PCIe module is present */
>  	status = "disabled";
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 7a2df148c6a3..dac3007f2ac1 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -488,8 +488,15 @@
>  			#interrupt-cells = <1>;
>  			msi-parent = <&pcie0>;
>  			msi-controller;
> -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> +			/*
> +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> +			 * dedicated for PCIe and can be assigned to 8 windows
> +			 * with size a power of two. Use one 64 KiB window for
> +			 * IO at the end and the remaining seven windows
> +			 * (totaling 127 MiB) for MEM.
> +			 */
> +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
>  			interrupt-map-mask = <0 0 0 7>;
>  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
>  					<0 0 0 2 &pcie_intc 1>,
> -- 
> 2.20.1
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Gregory CLEMENT]

Hello Pali,

> Current PCIe MEM space of size 16 MB is not enough for some combination
> of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> allocate more PCIe BARs for more PCIe cards.
>
> Without this change some combination of PCIe cards cannot be used and
> kernel show error messages in dmesg during initialization:
>
>     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
>     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
>     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
>     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
>     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
>     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
>     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
>     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
>     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
>     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
>     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
>
> Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> crashes during loading of kernel DTB file. This bug is present only in
> U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> affected by this bug. Bug is fixed in U-Boot version 2021.07.
>
> To not break booting new kernels on existing versions of U-Boot on Turris
> Mox, use first 16 MB range for IO and second range with rest of PCIe window
> for MEM.

Is there any depencey with the firs patch of this series ?

What happend if this patch is applied without the other ?
Could you test it to see if any regression occure ?

Thanks,

Grégory

>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> ---
>  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 53e817c5f6f3..86b3025f174b 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -134,6 +134,23 @@
>  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
>  	status = "okay";
>  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> +	/*
> +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> +	 */
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
>  
>  	/* enabled by U-Boot if PCIe module is present */
>  	status = "disabled";
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 7a2df148c6a3..dac3007f2ac1 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -488,8 +488,15 @@
>  			#interrupt-cells = <1>;
>  			msi-parent = <&pcie0>;
>  			msi-controller;
> -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> +			/*
> +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> +			 * dedicated for PCIe and can be assigned to 8 windows
> +			 * with size a power of two. Use one 64 KiB window for
> +			 * IO at the end and the remaining seven windows
> +			 * (totaling 127 MiB) for MEM.
> +			 */
> +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
>  			interrupt-map-mask = <0 0 0 7>;
>  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
>  					<0 0 0 2 &pcie_intc 1>,
> -- 
> 2.20.1
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> Hello Pali,
> 
> > Current PCIe MEM space of size 16 MB is not enough for some combination
> > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > allocate more PCIe BARs for more PCIe cards.
> >
> > Without this change some combination of PCIe cards cannot be used and
> > kernel show error messages in dmesg during initialization:
> >
> >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> >
> > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > crashes during loading of kernel DTB file. This bug is present only in
> > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> >
> > To not break booting new kernels on existing versions of U-Boot on Turris
> > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > for MEM.
> 
> Is there any depencey with the firs patch of this series ?
> 
> What happend if this patch is applied without the other ?

First patch is fixing reading and setting ranges configuration from DTS.
Without first patch memory windows stays as they were in bootloader or
in its default configuration. Which is that all 128 MB are transparently
mapped to PCIe MEM space.

Therefore this second DTS patch does not fixes issue with IO space
(kernel still crashes when accessing it). But allows to use all PCIe MEM
space (due to bootloader / default configuration) and therefore allows
to use more PCIe cards (which needs more PCIe MEM space).

> Could you test it to see if any regression occure ?
> 
> Thanks,
> 
> Grégory
> 
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > ---
> >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > index 53e817c5f6f3..86b3025f174b 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > @@ -134,6 +134,23 @@
> >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> >  	status = "okay";
> >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > +	/*
> > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > +	 */
> > +	#address-cells = <3>;
> > +	#size-cells = <2>;
> > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> >  
> >  	/* enabled by U-Boot if PCIe module is present */
> >  	status = "disabled";
> > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > index 7a2df148c6a3..dac3007f2ac1 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > @@ -488,8 +488,15 @@
> >  			#interrupt-cells = <1>;
> >  			msi-parent = <&pcie0>;
> >  			msi-controller;
> > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > +			/*
> > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > +			 * dedicated for PCIe and can be assigned to 8 windows
> > +			 * with size a power of two. Use one 64 KiB window for
> > +			 * IO at the end and the remaining seven windows
> > +			 * (totaling 127 MiB) for MEM.
> > +			 */
> > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> >  			interrupt-map-mask = <0 0 0 7>;
> >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> >  					<0 0 0 2 &pcie_intc 1>,
> > -- 
> > 2.20.1
> >
> 
> -- 
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> Hello Pali,
> 
> > Current PCIe MEM space of size 16 MB is not enough for some combination
> > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > allocate more PCIe BARs for more PCIe cards.
> >
> > Without this change some combination of PCIe cards cannot be used and
> > kernel show error messages in dmesg during initialization:
> >
> >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> >
> > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > crashes during loading of kernel DTB file. This bug is present only in
> > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> >
> > To not break booting new kernels on existing versions of U-Boot on Turris
> > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > for MEM.
> 
> Is there any depencey with the firs patch of this series ?
> 
> What happend if this patch is applied without the other ?

First patch is fixing reading and setting ranges configuration from DTS.
Without first patch memory windows stays as they were in bootloader or
in its default configuration. Which is that all 128 MB are transparently
mapped to PCIe MEM space.

Therefore this second DTS patch does not fixes issue with IO space
(kernel still crashes when accessing it). But allows to use all PCIe MEM
space (due to bootloader / default configuration) and therefore allows
to use more PCIe cards (which needs more PCIe MEM space).

> Could you test it to see if any regression occure ?
> 
> Thanks,
> 
> Grégory
> 
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > ---
> >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > index 53e817c5f6f3..86b3025f174b 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > @@ -134,6 +134,23 @@
> >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> >  	status = "okay";
> >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > +	/*
> > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > +	 */
> > +	#address-cells = <3>;
> > +	#size-cells = <2>;
> > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> >  
> >  	/* enabled by U-Boot if PCIe module is present */
> >  	status = "disabled";
> > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > index 7a2df148c6a3..dac3007f2ac1 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > @@ -488,8 +488,15 @@
> >  			#interrupt-cells = <1>;
> >  			msi-parent = <&pcie0>;
> >  			msi-controller;
> > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > +			/*
> > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > +			 * dedicated for PCIe and can be assigned to 8 windows
> > +			 * with size a power of two. Use one 64 KiB window for
> > +			 * IO at the end and the remaining seven windows
> > +			 * (totaling 127 MiB) for MEM.
> > +			 */
> > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> >  			interrupt-map-mask = <0 0 0 7>;
> >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> >  					<0 0 0 2 &pcie_intc 1>,
> > -- 
> > 2.20.1
> >
> 
> -- 
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Lorenzo Pieralisi]

On Fri, Jul 23, 2021 at 04:12:04PM +0200, Pali Rohár wrote:
> On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> > Hello Pali,
> > 
> > > Current PCIe MEM space of size 16 MB is not enough for some combination
> > > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > > allocate more PCIe BARs for more PCIe cards.
> > >
> > > Without this change some combination of PCIe cards cannot be used and
> > > kernel show error messages in dmesg during initialization:
> > >
> > >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> > >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> > >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> > >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> > >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> > >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> > >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> > >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> > >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> > >
> > > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > > crashes during loading of kernel DTB file. This bug is present only in
> > > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> > >
> > > To not break booting new kernels on existing versions of U-Boot on Turris
> > > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > > for MEM.
> > 
> > Is there any depencey with the firs patch of this series ?
> > 
> > What happend if this patch is applied without the other ?
> 
> First patch is fixing reading and setting ranges configuration from DTS.
> Without first patch memory windows stays as they were in bootloader or
> in its default configuration. Which is that all 128 MB are transparently
> mapped to PCIe MEM space.
> 
> Therefore this second DTS patch does not fixes issue with IO space
> (kernel still crashes when accessing it). But allows to use all PCIe MEM
> space (due to bootloader / default configuration) and therefore allows
> to use more PCIe cards (which needs more PCIe MEM space).

So, the two patches are decoupled then ? We are not taking dts changes
through the PCI tree.

Besides: these dts patches are a nightmare for backward compatibility,
hopefully Rob can shed some light on whether what you are doing here
is advisable and how to sync the changes with kernel changes.

Lorenzo

> > Could you test it to see if any regression occure ?
> > 
> > Thanks,
> > 
> > Grégory
> > 
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > > ---
> > >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> > >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > index 53e817c5f6f3..86b3025f174b 100644
> > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > @@ -134,6 +134,23 @@
> > >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> > >  	status = "okay";
> > >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > > +	/*
> > > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > > +	 */
> > > +	#address-cells = <3>;
> > > +	#size-cells = <2>;
> > > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> > >  
> > >  	/* enabled by U-Boot if PCIe module is present */
> > >  	status = "disabled";
> > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > index 7a2df148c6a3..dac3007f2ac1 100644
> > > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > @@ -488,8 +488,15 @@
> > >  			#interrupt-cells = <1>;
> > >  			msi-parent = <&pcie0>;
> > >  			msi-controller;
> > > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > > +			/*
> > > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > > +			 * dedicated for PCIe and can be assigned to 8 windows
> > > +			 * with size a power of two. Use one 64 KiB window for
> > > +			 * IO at the end and the remaining seven windows
> > > +			 * (totaling 127 MiB) for MEM.
> > > +			 */
> > > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> > >  			interrupt-map-mask = <0 0 0 7>;
> > >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > >  					<0 0 0 2 &pcie_intc 1>,
> > > -- 
> > > 2.20.1
> > >
> > 
> > -- 
> > Gregory Clement, Bootlin
> > Embedded Linux and Kernel engineering
> > http://bootlin.com

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Lorenzo Pieralisi]

On Fri, Jul 23, 2021 at 04:12:04PM +0200, Pali Rohár wrote:
> On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> > Hello Pali,
> > 
> > > Current PCIe MEM space of size 16 MB is not enough for some combination
> > > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > > allocate more PCIe BARs for more PCIe cards.
> > >
> > > Without this change some combination of PCIe cards cannot be used and
> > > kernel show error messages in dmesg during initialization:
> > >
> > >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> > >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> > >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> > >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> > >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> > >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> > >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> > >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> > >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> > >
> > > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > > crashes during loading of kernel DTB file. This bug is present only in
> > > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> > >
> > > To not break booting new kernels on existing versions of U-Boot on Turris
> > > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > > for MEM.
> > 
> > Is there any depencey with the firs patch of this series ?
> > 
> > What happend if this patch is applied without the other ?
> 
> First patch is fixing reading and setting ranges configuration from DTS.
> Without first patch memory windows stays as they were in bootloader or
> in its default configuration. Which is that all 128 MB are transparently
> mapped to PCIe MEM space.
> 
> Therefore this second DTS patch does not fixes issue with IO space
> (kernel still crashes when accessing it). But allows to use all PCIe MEM
> space (due to bootloader / default configuration) and therefore allows
> to use more PCIe cards (which needs more PCIe MEM space).

So, the two patches are decoupled then ? We are not taking dts changes
through the PCI tree.

Besides: these dts patches are a nightmare for backward compatibility,
hopefully Rob can shed some light on whether what you are doing here
is advisable and how to sync the changes with kernel changes.

Lorenzo

> > Could you test it to see if any regression occure ?
> > 
> > Thanks,
> > 
> > Grégory
> > 
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > > ---
> > >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> > >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > index 53e817c5f6f3..86b3025f174b 100644
> > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > @@ -134,6 +134,23 @@
> > >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> > >  	status = "okay";
> > >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > > +	/*
> > > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > > +	 */
> > > +	#address-cells = <3>;
> > > +	#size-cells = <2>;
> > > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> > >  
> > >  	/* enabled by U-Boot if PCIe module is present */
> > >  	status = "disabled";
> > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > index 7a2df148c6a3..dac3007f2ac1 100644
> > > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > @@ -488,8 +488,15 @@
> > >  			#interrupt-cells = <1>;
> > >  			msi-parent = <&pcie0>;
> > >  			msi-controller;
> > > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > > +			/*
> > > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > > +			 * dedicated for PCIe and can be assigned to 8 windows
> > > +			 * with size a power of two. Use one 64 KiB window for
> > > +			 * IO at the end and the remaining seven windows
> > > +			 * (totaling 127 MiB) for MEM.
> > > +			 */
> > > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> > >  			interrupt-map-mask = <0 0 0 7>;
> > >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > >  					<0 0 0 2 &pcie_intc 1>,
> > > -- 
> > > 2.20.1
> > >
> > 
> > -- 
> > Gregory Clement, Bootlin
> > Embedded Linux and Kernel engineering
> > http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

On Friday 23 July 2021 16:52:47 Lorenzo Pieralisi wrote:
> On Fri, Jul 23, 2021 at 04:12:04PM +0200, Pali Rohár wrote:
> > On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> > > Hello Pali,
> > > 
> > > > Current PCIe MEM space of size 16 MB is not enough for some combination
> > > > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > > > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > > > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > > > allocate more PCIe BARs for more PCIe cards.
> > > >
> > > > Without this change some combination of PCIe cards cannot be used and
> > > > kernel show error messages in dmesg during initialization:
> > > >
> > > >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> > > >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> > > >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> > > >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> > > >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> > > >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> > > >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> > > >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> > > >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> > > >
> > > > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > > > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > > > crashes during loading of kernel DTB file. This bug is present only in
> > > > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > > > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> > > >
> > > > To not break booting new kernels on existing versions of U-Boot on Turris
> > > > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > > > for MEM.
> > > 
> > > Is there any depencey with the firs patch of this series ?
> > > 
> > > What happend if this patch is applied without the other ?
> > 
> > First patch is fixing reading and setting ranges configuration from DTS.
> > Without first patch memory windows stays as they were in bootloader or
> > in its default configuration. Which is that all 128 MB are transparently
> > mapped to PCIe MEM space.
> > 
> > Therefore this second DTS patch does not fixes issue with IO space
> > (kernel still crashes when accessing it). But allows to use all PCIe MEM
> > space (due to bootloader / default configuration) and therefore allows
> > to use more PCIe cards (which needs more PCIe MEM space).
> 
> So, the two patches are decoupled then ? We are not taking dts changes
> through the PCI tree.

Well, both patches are required to fix setup and use of PCIe ranges on
A3720 platforms. But I would say they are independent and you can apply
them in any order. So Gregory, feel free to take DTS change in your tree
and Lorenzo can review other patch.

I sent these two patches in one series as they are fixing one common
issue. It is common that for fixing one common issue it is required to
touch more subsystems / trees.

> Besides: these dts patches are a nightmare for backward compatibility,
> hopefully Rob can shed some light on whether what you are doing here
> is advisable and how to sync the changes with kernel changes.

As written in comment for armada-3720-turris-mox.dts file, there are
specific requirements what needs to be put into ranges section. And
version of this file without applying this patch and also version of
this file with applied patch matches these requirements.

So I would say that this DTS change is backward and also forward
compatible.

But I agree that DTS changes are lot of time nightmare...

> Lorenzo
> 
> > > Could you test it to see if any regression occure ?
> > > 
> > > Thanks,
> > > 
> > > Grégory
> > > 
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > > > ---
> > > >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> > > >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> > > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > index 53e817c5f6f3..86b3025f174b 100644
> > > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > @@ -134,6 +134,23 @@
> > > >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> > > >  	status = "okay";
> > > >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > > > +	/*
> > > > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > > > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > > > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > > > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > > > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > > > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > > > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > > > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > > > +	 */
> > > > +	#address-cells = <3>;
> > > > +	#size-cells = <2>;
> > > > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > > > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> > > >  
> > > >  	/* enabled by U-Boot if PCIe module is present */
> > > >  	status = "disabled";
> > > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > index 7a2df148c6a3..dac3007f2ac1 100644
> > > > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > @@ -488,8 +488,15 @@
> > > >  			#interrupt-cells = <1>;
> > > >  			msi-parent = <&pcie0>;
> > > >  			msi-controller;
> > > > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > > > +			/*
> > > > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > > > +			 * dedicated for PCIe and can be assigned to 8 windows
> > > > +			 * with size a power of two. Use one 64 KiB window for
> > > > +			 * IO at the end and the remaining seven windows
> > > > +			 * (totaling 127 MiB) for MEM.
> > > > +			 */
> > > > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > > > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> > > >  			interrupt-map-mask = <0 0 0 7>;
> > > >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > > >  					<0 0 0 2 &pcie_intc 1>,
> > > > -- 
> > > > 2.20.1
> > > >
> > > 
> > > -- 
> > > Gregory Clement, Bootlin
> > > Embedded Linux and Kernel engineering
> > > http://bootlin.com

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

On Friday 23 July 2021 16:52:47 Lorenzo Pieralisi wrote:
> On Fri, Jul 23, 2021 at 04:12:04PM +0200, Pali Rohár wrote:
> > On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> > > Hello Pali,
> > > 
> > > > Current PCIe MEM space of size 16 MB is not enough for some combination
> > > > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > > > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > > > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > > > allocate more PCIe BARs for more PCIe cards.
> > > >
> > > > Without this change some combination of PCIe cards cannot be used and
> > > > kernel show error messages in dmesg during initialization:
> > > >
> > > >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> > > >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> > > >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> > > >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> > > >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> > > >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> > > >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> > > >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> > > >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> > > >
> > > > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > > > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > > > crashes during loading of kernel DTB file. This bug is present only in
> > > > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > > > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> > > >
> > > > To not break booting new kernels on existing versions of U-Boot on Turris
> > > > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > > > for MEM.
> > > 
> > > Is there any depencey with the firs patch of this series ?
> > > 
> > > What happend if this patch is applied without the other ?
> > 
> > First patch is fixing reading and setting ranges configuration from DTS.
> > Without first patch memory windows stays as they were in bootloader or
> > in its default configuration. Which is that all 128 MB are transparently
> > mapped to PCIe MEM space.
> > 
> > Therefore this second DTS patch does not fixes issue with IO space
> > (kernel still crashes when accessing it). But allows to use all PCIe MEM
> > space (due to bootloader / default configuration) and therefore allows
> > to use more PCIe cards (which needs more PCIe MEM space).
> 
> So, the two patches are decoupled then ? We are not taking dts changes
> through the PCI tree.

Well, both patches are required to fix setup and use of PCIe ranges on
A3720 platforms. But I would say they are independent and you can apply
them in any order. So Gregory, feel free to take DTS change in your tree
and Lorenzo can review other patch.

I sent these two patches in one series as they are fixing one common
issue. It is common that for fixing one common issue it is required to
touch more subsystems / trees.

> Besides: these dts patches are a nightmare for backward compatibility,
> hopefully Rob can shed some light on whether what you are doing here
> is advisable and how to sync the changes with kernel changes.

As written in comment for armada-3720-turris-mox.dts file, there are
specific requirements what needs to be put into ranges section. And
version of this file without applying this patch and also version of
this file with applied patch matches these requirements.

So I would say that this DTS change is backward and also forward
compatible.

But I agree that DTS changes are lot of time nightmare...

> Lorenzo
> 
> > > Could you test it to see if any regression occure ?
> > > 
> > > Thanks,
> > > 
> > > Grégory
> > > 
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > > > ---
> > > >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> > > >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> > > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > index 53e817c5f6f3..86b3025f174b 100644
> > > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > @@ -134,6 +134,23 @@
> > > >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> > > >  	status = "okay";
> > > >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > > > +	/*
> > > > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > > > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > > > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > > > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > > > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > > > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > > > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > > > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > > > +	 */
> > > > +	#address-cells = <3>;
> > > > +	#size-cells = <2>;
> > > > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > > > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> > > >  
> > > >  	/* enabled by U-Boot if PCIe module is present */
> > > >  	status = "disabled";
> > > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > index 7a2df148c6a3..dac3007f2ac1 100644
> > > > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > @@ -488,8 +488,15 @@
> > > >  			#interrupt-cells = <1>;
> > > >  			msi-parent = <&pcie0>;
> > > >  			msi-controller;
> > > > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > > > +			/*
> > > > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > > > +			 * dedicated for PCIe and can be assigned to 8 windows
> > > > +			 * with size a power of two. Use one 64 KiB window for
> > > > +			 * IO at the end and the remaining seven windows
> > > > +			 * (totaling 127 MiB) for MEM.
> > > > +			 */
> > > > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > > > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> > > >  			interrupt-map-mask = <0 0 0 7>;
> > > >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > > >  					<0 0 0 2 &pcie_intc 1>,
> > > > -- 
> > > > 2.20.1
> > > >
> > > 
> > > -- 
> > > Gregory Clement, Bootlin
> > > Embedded Linux and Kernel engineering
> > > http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

On Friday 23 July 2021 18:45:12 Pali Rohár wrote:
> On Friday 23 July 2021 16:52:47 Lorenzo Pieralisi wrote:
> > On Fri, Jul 23, 2021 at 04:12:04PM +0200, Pali Rohár wrote:
> > > On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> > > > Hello Pali,
> > > > 
> > > > > Current PCIe MEM space of size 16 MB is not enough for some combination
> > > > > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > > > > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > > > > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > > > > allocate more PCIe BARs for more PCIe cards.
> > > > >
> > > > > Without this change some combination of PCIe cards cannot be used and
> > > > > kernel show error messages in dmesg during initialization:
> > > > >
> > > > >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> > > > >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > > >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> > > > >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> > > > >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > > >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> > > > >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> > > > >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> > > > >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> > > > >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> > > > >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> > > > >
> > > > > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > > > > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > > > > crashes during loading of kernel DTB file. This bug is present only in
> > > > > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > > > > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> > > > >
> > > > > To not break booting new kernels on existing versions of U-Boot on Turris
> > > > > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > > > > for MEM.
> > > > 
> > > > Is there any depencey with the firs patch of this series ?
> > > > 
> > > > What happend if this patch is applied without the other ?
> > > 
> > > First patch is fixing reading and setting ranges configuration from DTS.
> > > Without first patch memory windows stays as they were in bootloader or
> > > in its default configuration. Which is that all 128 MB are transparently
> > > mapped to PCIe MEM space.
> > > 
> > > Therefore this second DTS patch does not fixes issue with IO space
> > > (kernel still crashes when accessing it). But allows to use all PCIe MEM
> > > space (due to bootloader / default configuration) and therefore allows
> > > to use more PCIe cards (which needs more PCIe MEM space).
> > 
> > So, the two patches are decoupled then ? We are not taking dts changes
> > through the PCI tree.
> 
> Well, both patches are required to fix setup and use of PCIe ranges on
> A3720 platforms. But I would say they are independent and you can apply
> them in any order. So Gregory, feel free to take DTS change in your tree
> and Lorenzo can review other patch.

Lorenzo, so will you take driver change via PCI tree?

And Gregory, DTS change via DTS tree?

> I sent these two patches in one series as they are fixing one common
> issue. It is common that for fixing one common issue it is required to
> touch more subsystems / trees.
> 
> > Besides: these dts patches are a nightmare for backward compatibility,
> > hopefully Rob can shed some light on whether what you are doing here
> > is advisable and how to sync the changes with kernel changes.
> 
> As written in comment for armada-3720-turris-mox.dts file, there are
> specific requirements what needs to be put into ranges section. And
> version of this file without applying this patch and also version of
> this file with applied patch matches these requirements.
> 
> So I would say that this DTS change is backward and also forward
> compatible.
> 
> But I agree that DTS changes are lot of time nightmare...
> 
> > Lorenzo
> > 
> > > > Could you test it to see if any regression occure ?
> > > > 
> > > > Thanks,
> > > > 
> > > > Grégory
> > > > 
> > > > >
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > > > > ---
> > > > >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> > > > >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> > > > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > index 53e817c5f6f3..86b3025f174b 100644
> > > > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > @@ -134,6 +134,23 @@
> > > > >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> > > > >  	status = "okay";
> > > > >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > > > > +	/*
> > > > > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > > > > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > > > > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > > > > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > > > > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > > > > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > > > > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > > > > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > > > > +	 */
> > > > > +	#address-cells = <3>;
> > > > > +	#size-cells = <2>;
> > > > > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > > > > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> > > > >  
> > > > >  	/* enabled by U-Boot if PCIe module is present */
> > > > >  	status = "disabled";
> > > > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > index 7a2df148c6a3..dac3007f2ac1 100644
> > > > > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > @@ -488,8 +488,15 @@
> > > > >  			#interrupt-cells = <1>;
> > > > >  			msi-parent = <&pcie0>;
> > > > >  			msi-controller;
> > > > > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > > > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > > > > +			/*
> > > > > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > > > > +			 * dedicated for PCIe and can be assigned to 8 windows
> > > > > +			 * with size a power of two. Use one 64 KiB window for
> > > > > +			 * IO at the end and the remaining seven windows
> > > > > +			 * (totaling 127 MiB) for MEM.
> > > > > +			 */
> > > > > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > > > > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> > > > >  			interrupt-map-mask = <0 0 0 7>;
> > > > >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > > > >  					<0 0 0 2 &pcie_intc 1>,
> > > > > -- 
> > > > > 2.20.1
> > > > >
> > > > 
> > > > -- 
> > > > Gregory Clement, Bootlin
> > > > Embedded Linux and Kernel engineering
> > > > http://bootlin.com

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

On Friday 23 July 2021 18:45:12 Pali Rohár wrote:
> On Friday 23 July 2021 16:52:47 Lorenzo Pieralisi wrote:
> > On Fri, Jul 23, 2021 at 04:12:04PM +0200, Pali Rohár wrote:
> > > On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> > > > Hello Pali,
> > > > 
> > > > > Current PCIe MEM space of size 16 MB is not enough for some combination
> > > > > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > > > > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > > > > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > > > > allocate more PCIe BARs for more PCIe cards.
> > > > >
> > > > > Without this change some combination of PCIe cards cannot be used and
> > > > > kernel show error messages in dmesg during initialization:
> > > > >
> > > > >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> > > > >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > > >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> > > > >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> > > > >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > > >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> > > > >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> > > > >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> > > > >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> > > > >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> > > > >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> > > > >
> > > > > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > > > > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > > > > crashes during loading of kernel DTB file. This bug is present only in
> > > > > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > > > > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> > > > >
> > > > > To not break booting new kernels on existing versions of U-Boot on Turris
> > > > > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > > > > for MEM.
> > > > 
> > > > Is there any depencey with the firs patch of this series ?
> > > > 
> > > > What happend if this patch is applied without the other ?
> > > 
> > > First patch is fixing reading and setting ranges configuration from DTS.
> > > Without first patch memory windows stays as they were in bootloader or
> > > in its default configuration. Which is that all 128 MB are transparently
> > > mapped to PCIe MEM space.
> > > 
> > > Therefore this second DTS patch does not fixes issue with IO space
> > > (kernel still crashes when accessing it). But allows to use all PCIe MEM
> > > space (due to bootloader / default configuration) and therefore allows
> > > to use more PCIe cards (which needs more PCIe MEM space).
> > 
> > So, the two patches are decoupled then ? We are not taking dts changes
> > through the PCI tree.
> 
> Well, both patches are required to fix setup and use of PCIe ranges on
> A3720 platforms. But I would say they are independent and you can apply
> them in any order. So Gregory, feel free to take DTS change in your tree
> and Lorenzo can review other patch.

Lorenzo, so will you take driver change via PCI tree?

And Gregory, DTS change via DTS tree?

> I sent these two patches in one series as they are fixing one common
> issue. It is common that for fixing one common issue it is required to
> touch more subsystems / trees.
> 
> > Besides: these dts patches are a nightmare for backward compatibility,
> > hopefully Rob can shed some light on whether what you are doing here
> > is advisable and how to sync the changes with kernel changes.
> 
> As written in comment for armada-3720-turris-mox.dts file, there are
> specific requirements what needs to be put into ranges section. And
> version of this file without applying this patch and also version of
> this file with applied patch matches these requirements.
> 
> So I would say that this DTS change is backward and also forward
> compatible.
> 
> But I agree that DTS changes are lot of time nightmare...
> 
> > Lorenzo
> > 
> > > > Could you test it to see if any regression occure ?
> > > > 
> > > > Thanks,
> > > > 
> > > > Grégory
> > > > 
> > > > >
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > > > > ---
> > > > >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> > > > >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> > > > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > index 53e817c5f6f3..86b3025f174b 100644
> > > > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > @@ -134,6 +134,23 @@
> > > > >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> > > > >  	status = "okay";
> > > > >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > > > > +	/*
> > > > > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > > > > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > > > > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > > > > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > > > > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > > > > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > > > > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > > > > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > > > > +	 */
> > > > > +	#address-cells = <3>;
> > > > > +	#size-cells = <2>;
> > > > > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > > > > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> > > > >  
> > > > >  	/* enabled by U-Boot if PCIe module is present */
> > > > >  	status = "disabled";
> > > > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > index 7a2df148c6a3..dac3007f2ac1 100644
> > > > > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > @@ -488,8 +488,15 @@
> > > > >  			#interrupt-cells = <1>;
> > > > >  			msi-parent = <&pcie0>;
> > > > >  			msi-controller;
> > > > > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > > > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > > > > +			/*
> > > > > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > > > > +			 * dedicated for PCIe and can be assigned to 8 windows
> > > > > +			 * with size a power of two. Use one 64 KiB window for
> > > > > +			 * IO at the end and the remaining seven windows
> > > > > +			 * (totaling 127 MiB) for MEM.
> > > > > +			 */
> > > > > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > > > > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> > > > >  			interrupt-map-mask = <0 0 0 7>;
> > > > >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > > > >  					<0 0 0 2 &pcie_intc 1>,
> > > > > -- 
> > > > > 2.20.1
> > > > >
> > > > 
> > > > -- 
> > > > Gregory Clement, Bootlin
> > > > Embedded Linux and Kernel engineering
> > > > http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

On Saturday 07 August 2021 13:35:36 Pali Rohár wrote:
> On Friday 23 July 2021 18:45:12 Pali Rohár wrote:
> > On Friday 23 July 2021 16:52:47 Lorenzo Pieralisi wrote:
> > > On Fri, Jul 23, 2021 at 04:12:04PM +0200, Pali Rohár wrote:
> > > > On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> > > > > Hello Pali,
> > > > > 
> > > > > > Current PCIe MEM space of size 16 MB is not enough for some combination
> > > > > > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > > > > > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > > > > > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > > > > > allocate more PCIe BARs for more PCIe cards.
> > > > > >
> > > > > > Without this change some combination of PCIe cards cannot be used and
> > > > > > kernel show error messages in dmesg during initialization:
> > > > > >
> > > > > >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> > > > > >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > > > >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> > > > > >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> > > > > >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > > > >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> > > > > >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> > > > > >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> > > > > >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> > > > > >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> > > > > >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> > > > > >
> > > > > > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > > > > > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > > > > > crashes during loading of kernel DTB file. This bug is present only in
> > > > > > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > > > > > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> > > > > >
> > > > > > To not break booting new kernels on existing versions of U-Boot on Turris
> > > > > > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > > > > > for MEM.
> > > > > 
> > > > > Is there any depencey with the firs patch of this series ?
> > > > > 
> > > > > What happend if this patch is applied without the other ?
> > > > 
> > > > First patch is fixing reading and setting ranges configuration from DTS.
> > > > Without first patch memory windows stays as they were in bootloader or
> > > > in its default configuration. Which is that all 128 MB are transparently
> > > > mapped to PCIe MEM space.
> > > > 
> > > > Therefore this second DTS patch does not fixes issue with IO space
> > > > (kernel still crashes when accessing it). But allows to use all PCIe MEM
> > > > space (due to bootloader / default configuration) and therefore allows
> > > > to use more PCIe cards (which needs more PCIe MEM space).
> > > 
> > > So, the two patches are decoupled then ? We are not taking dts changes
> > > through the PCI tree.
> > 
> > Well, both patches are required to fix setup and use of PCIe ranges on
> > A3720 platforms. But I would say they are independent and you can apply
> > them in any order. So Gregory, feel free to take DTS change in your tree
> > and Lorenzo can review other patch.
> 
> Lorenzo, so will you take driver change via PCI tree?
> 
> And Gregory, DTS change via DTS tree?

PING?

> > I sent these two patches in one series as they are fixing one common
> > issue. It is common that for fixing one common issue it is required to
> > touch more subsystems / trees.
> > 
> > > Besides: these dts patches are a nightmare for backward compatibility,
> > > hopefully Rob can shed some light on whether what you are doing here
> > > is advisable and how to sync the changes with kernel changes.
> > 
> > As written in comment for armada-3720-turris-mox.dts file, there are
> > specific requirements what needs to be put into ranges section. And
> > version of this file without applying this patch and also version of
> > this file with applied patch matches these requirements.
> > 
> > So I would say that this DTS change is backward and also forward
> > compatible.
> > 
> > But I agree that DTS changes are lot of time nightmare...
> > 
> > > Lorenzo
> > > 
> > > > > Could you test it to see if any regression occure ?
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > Grégory
> > > > > 
> > > > > >
> > > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > > > > > ---
> > > > > >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> > > > > >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> > > > > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > > index 53e817c5f6f3..86b3025f174b 100644
> > > > > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > > @@ -134,6 +134,23 @@
> > > > > >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> > > > > >  	status = "okay";
> > > > > >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > > > > > +	/*
> > > > > > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > > > > > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > > > > > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > > > > > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > > > > > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > > > > > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > > > > > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > > > > > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > > > > > +	 */
> > > > > > +	#address-cells = <3>;
> > > > > > +	#size-cells = <2>;
> > > > > > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > > > > > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> > > > > >  
> > > > > >  	/* enabled by U-Boot if PCIe module is present */
> > > > > >  	status = "disabled";
> > > > > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > > index 7a2df148c6a3..dac3007f2ac1 100644
> > > > > > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > > @@ -488,8 +488,15 @@
> > > > > >  			#interrupt-cells = <1>;
> > > > > >  			msi-parent = <&pcie0>;
> > > > > >  			msi-controller;
> > > > > > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > > > > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > > > > > +			/*
> > > > > > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > > > > > +			 * dedicated for PCIe and can be assigned to 8 windows
> > > > > > +			 * with size a power of two. Use one 64 KiB window for
> > > > > > +			 * IO at the end and the remaining seven windows
> > > > > > +			 * (totaling 127 MiB) for MEM.
> > > > > > +			 */
> > > > > > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > > > > > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> > > > > >  			interrupt-map-mask = <0 0 0 7>;
> > > > > >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > > > > >  					<0 0 0 2 &pcie_intc 1>,
> > > > > > -- 
> > > > > > 2.20.1
> > > > > >
> > > > > 
> > > > > -- 
> > > > > Gregory Clement, Bootlin
> > > > > Embedded Linux and Kernel engineering
> > > > > http://bootlin.com

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Pali Rohár]

On Saturday 07 August 2021 13:35:36 Pali Rohár wrote:
> On Friday 23 July 2021 18:45:12 Pali Rohár wrote:
> > On Friday 23 July 2021 16:52:47 Lorenzo Pieralisi wrote:
> > > On Fri, Jul 23, 2021 at 04:12:04PM +0200, Pali Rohár wrote:
> > > > On Friday 23 July 2021 14:52:25 Gregory CLEMENT wrote:
> > > > > Hello Pali,
> > > > > 
> > > > > > Current PCIe MEM space of size 16 MB is not enough for some combination
> > > > > > of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> > > > > > Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> > > > > > so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> > > > > > allocate more PCIe BARs for more PCIe cards.
> > > > > >
> > > > > > Without this change some combination of PCIe cards cannot be used and
> > > > > > kernel show error messages in dmesg during initialization:
> > > > > >
> > > > > >     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
> > > > > >     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > > > >     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
> > > > > >     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
> > > > > >     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
> > > > > >     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
> > > > > >     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
> > > > > >     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
> > > > > >     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
> > > > > >     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
> > > > > >     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
> > > > > >
> > > > > > Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> > > > > > kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> > > > > > crashes during loading of kernel DTB file. This bug is present only in
> > > > > > U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> > > > > > affected by this bug. Bug is fixed in U-Boot version 2021.07.
> > > > > >
> > > > > > To not break booting new kernels on existing versions of U-Boot on Turris
> > > > > > Mox, use first 16 MB range for IO and second range with rest of PCIe window
> > > > > > for MEM.
> > > > > 
> > > > > Is there any depencey with the firs patch of this series ?
> > > > > 
> > > > > What happend if this patch is applied without the other ?
> > > > 
> > > > First patch is fixing reading and setting ranges configuration from DTS.
> > > > Without first patch memory windows stays as they were in bootloader or
> > > > in its default configuration. Which is that all 128 MB are transparently
> > > > mapped to PCIe MEM space.
> > > > 
> > > > Therefore this second DTS patch does not fixes issue with IO space
> > > > (kernel still crashes when accessing it). But allows to use all PCIe MEM
> > > > space (due to bootloader / default configuration) and therefore allows
> > > > to use more PCIe cards (which needs more PCIe MEM space).
> > > 
> > > So, the two patches are decoupled then ? We are not taking dts changes
> > > through the PCI tree.
> > 
> > Well, both patches are required to fix setup and use of PCIe ranges on
> > A3720 platforms. But I would say they are independent and you can apply
> > them in any order. So Gregory, feel free to take DTS change in your tree
> > and Lorenzo can review other patch.
> 
> Lorenzo, so will you take driver change via PCI tree?
> 
> And Gregory, DTS change via DTS tree?

PING?

> > I sent these two patches in one series as they are fixing one common
> > issue. It is common that for fixing one common issue it is required to
> > touch more subsystems / trees.
> > 
> > > Besides: these dts patches are a nightmare for backward compatibility,
> > > hopefully Rob can shed some light on whether what you are doing here
> > > is advisable and how to sync the changes with kernel changes.
> > 
> > As written in comment for armada-3720-turris-mox.dts file, there are
> > specific requirements what needs to be put into ranges section. And
> > version of this file without applying this patch and also version of
> > this file with applied patch matches these requirements.
> > 
> > So I would say that this DTS change is backward and also forward
> > compatible.
> > 
> > But I agree that DTS changes are lot of time nightmare...
> > 
> > > Lorenzo
> > > 
> > > > > Could you test it to see if any regression occure ?
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > Grégory
> > > > > 
> > > > > >
> > > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > > > > > ---
> > > > > >  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
> > > > > >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
> > > > > >  2 files changed, 26 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > > index 53e817c5f6f3..86b3025f174b 100644
> > > > > > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > > > > > @@ -134,6 +134,23 @@
> > > > > >  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
> > > > > >  	status = "okay";
> > > > > >  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> > > > > > +	/*
> > > > > > +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > > > > > +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > > > > > +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > > > > > +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > > > > > +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > > > > > +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> > > > > > +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > > > > > +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > > > > > +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > > > > > +	 */
> > > > > > +	#address-cells = <3>;
> > > > > > +	#size-cells = <2>;
> > > > > > +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> > > > > > +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
> > > > > >  
> > > > > >  	/* enabled by U-Boot if PCIe module is present */
> > > > > >  	status = "disabled";
> > > > > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > > index 7a2df148c6a3..dac3007f2ac1 100644
> > > > > > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > > > > > @@ -488,8 +488,15 @@
> > > > > >  			#interrupt-cells = <1>;
> > > > > >  			msi-parent = <&pcie0>;
> > > > > >  			msi-controller;
> > > > > > -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > > > > -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > > > > > +			/*
> > > > > > +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> > > > > > +			 * dedicated for PCIe and can be assigned to 8 windows
> > > > > > +			 * with size a power of two. Use one 64 KiB window for
> > > > > > +			 * IO at the end and the remaining seven windows
> > > > > > +			 * (totaling 127 MiB) for MEM.
> > > > > > +			 */
> > > > > > +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> > > > > > +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
> > > > > >  			interrupt-map-mask = <0 0 0 7>;
> > > > > >  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > > > > >  					<0 0 0 2 &pcie_intc 1>,
> > > > > > -- 
> > > > > > 2.20.1
> > > > > >
> > > > > 
> > > > > -- 
> > > > > Gregory Clement, Bootlin
> > > > > Embedded Linux and Kernel engineering
> > > > > http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Gregory CLEMENT]

Pali Rohár <pali@kernel.org> writes:

> Current PCIe MEM space of size 16 MB is not enough for some combination
> of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> allocate more PCIe BARs for more PCIe cards.
>
> Without this change some combination of PCIe cards cannot be used and
> kernel show error messages in dmesg during initialization:
>
>     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
>     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
>     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
>     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
>     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
>     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
>     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
>     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
>     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
>     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
>     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
>
> Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> crashes during loading of kernel DTB file. This bug is present only in
> U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> affected by this bug. Bug is fixed in U-Boot version 2021.07.
>
> To not break booting new kernels on existing versions of U-Boot on Turris
> Mox, use first 16 MB range for IO and second range with rest of PCIe window
> for MEM.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied on mvebu/dt64

Thanks,

Gregory

> Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> ---
>  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 53e817c5f6f3..86b3025f174b 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -134,6 +134,23 @@
>  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
>  	status = "okay";
>  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> +	/*
> +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> +	 */
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
>  
>  	/* enabled by U-Boot if PCIe module is present */
>  	status = "disabled";
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 7a2df148c6a3..dac3007f2ac1 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -488,8 +488,15 @@
>  			#interrupt-cells = <1>;
>  			msi-parent = <&pcie0>;
>  			msi-controller;
> -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> +			/*
> +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> +			 * dedicated for PCIe and can be assigned to 8 windows
> +			 * with size a power of two. Use one 64 KiB window for
> +			 * IO at the end and the remaining seven windows
> +			 * (totaling 127 MiB) for MEM.
> +			 */
> +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
>  			interrupt-map-mask = <0 0 0 7>;
>  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
>  					<0 0 0 2 &pcie_intc 1>,
> -- 
> 2.20.1
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space

[Gregory CLEMENT]

Pali Rohár <pali@kernel.org> writes:

> Current PCIe MEM space of size 16 MB is not enough for some combination
> of PCIe cards (e.g. NVMe disk together with ath11k wifi card). ARM Trusted
> Firmware for Armada 3700 platform already assigns 128 MB for PCIe window,
> so extend PCIe MEM space to the end of 128 MB PCIe window which allows to
> allocate more PCIe BARs for more PCIe cards.
>
> Without this change some combination of PCIe cards cannot be used and
> kernel show error messages in dmesg during initialization:
>
>     pci 0000:00:00.0: BAR 8: no space for [mem size 0x01800000]
>     pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01800000]
>     pci 0000:00:00.0: BAR 6: assigned [mem 0xe8000000-0xe80007ff pref]
>     pci 0000:01:00.0: BAR 8: no space for [mem size 0x01800000]
>     pci 0000:01:00.0: BAR 8: failed to assign [mem size 0x01800000]
>     pci 0000:02:03.0: BAR 8: no space for [mem size 0x01000000]
>     pci 0000:02:03.0: BAR 8: failed to assign [mem size 0x01000000]
>     pci 0000:02:07.0: BAR 8: no space for [mem size 0x00100000]
>     pci 0000:02:07.0: BAR 8: failed to assign [mem size 0x00100000]
>     pci 0000:03:00.0: BAR 0: no space for [mem size 0x01000000 64bit]
>     pci 0000:03:00.0: BAR 0: failed to assign [mem size 0x01000000 64bit]
>
> Due to bugs in U-Boot port for Turris Mox, the second range in Turris Mox
> kernel DTS file for PCIe must start at 16 MB offset. Otherwise U-Boot
> crashes during loading of kernel DTB file. This bug is present only in
> U-Boot code for Turris Mox and therefore other Armada 3700 devices are not
> affected by this bug. Bug is fixed in U-Boot version 2021.07.
>
> To not break booting new kernels on existing versions of U-Boot on Turris
> Mox, use first 16 MB range for IO and second range with rest of PCIe window
> for MEM.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied on mvebu/dt64

Thanks,

Gregory

> Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> ---
>  .../boot/dts/marvell/armada-3720-turris-mox.dts | 17 +++++++++++++++++
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi    | 11 +++++++++--
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 53e817c5f6f3..86b3025f174b 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -134,6 +134,23 @@
>  	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
>  	status = "okay";
>  	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
> +	/*
> +	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> +	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> +	 * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> +	 * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> +	 * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> +	 * for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
> +	 * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> +	 * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> +	 * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> +	 */
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	ranges = <0x81000000 0 0xe8000000   0 0xe8000000   0 0x01000000   /* Port 0 IO */
> +		  0x82000000 0 0xe9000000   0 0xe9000000   0 0x07000000>; /* Port 0 MEM */
>  
>  	/* enabled by U-Boot if PCIe module is present */
>  	status = "disabled";
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 7a2df148c6a3..dac3007f2ac1 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -488,8 +488,15 @@
>  			#interrupt-cells = <1>;
>  			msi-parent = <&pcie0>;
>  			msi-controller;
> -			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> -				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> +			/*
> +			 * The 128 MiB address range [0xe8000000-0xf0000000] is
> +			 * dedicated for PCIe and can be assigned to 8 windows
> +			 * with size a power of two. Use one 64 KiB window for
> +			 * IO at the end and the remaining seven windows
> +			 * (totaling 127 MiB) for MEM.
> +			 */
> +			ranges = <0x82000000 0 0xe8000000   0 0xe8000000   0 0x07f00000   /* Port 0 MEM */
> +				  0x81000000 0 0xefff0000   0 0xefff0000   0 0x00010000>; /* Port 0 IO */
>  			interrupt-map-mask = <0 0 0 7>;
>  			interrupt-map = <0 0 0 1 &pcie_intc 0>,
>  					<0 0 0 2 &pcie_intc 1>,
> -- 
> 2.20.1
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] PCI: aardvark: Resource fixes

[Lorenzo Pieralisi]

On Thu, 24 Jun 2021 23:55:44 +0200, Pali Rohár wrote:
> This patch series fixes configuring PCIe resources (IO and MEM) in
> aardvark controller driver. It is required to initialize BARs on systems
> with more cards, e.g. NVMe disks and WiFi AX cards.
> 
> Pali Rohár (2):
>   PCI: aardvark: Configure PCIe resources from 'ranges' DT property
>   arm64: dts: marvell: armada-37xx: Extend PCIe MEM space
> 
> [...]

Applied to pci/aardvark, thanks!

[1/1] PCI: aardvark: Configure PCIe resources from 'ranges' DT property
      https://git.kernel.org/lpieralisi/pci/c/64f160e19e

Thanks,
Lorenzo

Re: [PATCH 0/2] PCI: aardvark: Resource fixes

[Lorenzo Pieralisi]

On Thu, 24 Jun 2021 23:55:44 +0200, Pali Rohár wrote:
> This patch series fixes configuring PCIe resources (IO and MEM) in
> aardvark controller driver. It is required to initialize BARs on systems
> with more cards, e.g. NVMe disks and WiFi AX cards.
> 
> Pali Rohár (2):
>   PCI: aardvark: Configure PCIe resources from 'ranges' DT property
>   arm64: dts: marvell: armada-37xx: Extend PCIe MEM space
> 
> [...]

Applied to pci/aardvark, thanks!

[1/1] PCI: aardvark: Configure PCIe resources from 'ranges' DT property
      https://git.kernel.org/lpieralisi/pci/c/64f160e19e

Thanks,
Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel