* [PATCH 0/2] PCI: aardvark: Resource fixes
@ 2021-06-24 21:55 Pali Rohár
2021-06-24 21:55 ` [PATCH 1/2] PCI: aardvark: Configure PCIe resources from 'ranges' DT property Pali Rohár
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Pali Rohár @ 2021-06-24 21:55 UTC (permalink / raw)
To: Lorenzo Pieralisi, Thomas Petazzoni, Bjorn Helgaas, Rob Herring,
Gregory Clement
Cc: Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak,
Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] PCI: aardvark: Configure PCIe resources from 'ranges' DT property 2021-06-24 21:55 [PATCH 0/2] PCI: aardvark: Resource fixes Pali Rohár @ 2021-06-24 21:55 ` Pali Rohár 2021-06-24 21:55 ` [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space Pali Rohár 2021-08-20 12:41 ` [PATCH 0/2] PCI: aardvark: Resource fixes Lorenzo Pieralisi 2 siblings, 0 replies; 11+ messages in thread From: Pali Rohár @ 2021-06-24 21:55 UTC (permalink / raw) To: Lorenzo Pieralisi, Thomas Petazzoni, Bjorn Helgaas, Rob Herring, Gregory Clement Cc: Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space 2021-06-24 21:55 [PATCH 0/2] PCI: aardvark: Resource fixes Pali Rohár 2021-06-24 21:55 ` [PATCH 1/2] PCI: aardvark: Configure PCIe resources from 'ranges' DT property Pali Rohár @ 2021-06-24 21:55 ` Pali Rohár 2021-07-23 12:52 ` Gregory CLEMENT 2021-08-17 16:39 ` Gregory CLEMENT 2021-08-20 12:41 ` [PATCH 0/2] PCI: aardvark: Resource fixes Lorenzo Pieralisi 2 siblings, 2 replies; 11+ messages in thread From: Pali Rohár @ 2021-06-24 21:55 UTC (permalink / raw) To: Lorenzo Pieralisi, Thomas Petazzoni, Bjorn Helgaas, Rob Herring, Gregory Clement Cc: Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space 2021-06-24 21:55 ` [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space Pali Rohár @ 2021-07-23 12:52 ` Gregory CLEMENT 2021-07-23 14:12 ` Pali Rohár 2021-08-17 16:39 ` Gregory CLEMENT 1 sibling, 1 reply; 11+ messages in thread From: Gregory CLEMENT @ 2021-07-23 12:52 UTC (permalink / raw) To: Pali Rohár, Lorenzo Pieralisi, Thomas Petazzoni, Bjorn Helgaas, Rob Herring Cc: Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space 2021-07-23 12:52 ` Gregory CLEMENT @ 2021-07-23 14:12 ` Pali Rohár 2021-07-23 15:52 ` Lorenzo Pieralisi 0 siblings, 1 reply; 11+ messages in thread From: Pali Rohár @ 2021-07-23 14:12 UTC (permalink / raw) To: Gregory CLEMENT Cc: Lorenzo Pieralisi, Thomas Petazzoni, Bjorn Helgaas, Rob Herring, Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space 2021-07-23 14:12 ` Pali Rohár @ 2021-07-23 15:52 ` Lorenzo Pieralisi 2021-07-23 16:45 ` Pali Rohár 0 siblings, 1 reply; 11+ messages in thread From: Lorenzo Pieralisi @ 2021-07-23 15:52 UTC (permalink / raw) To: Pali Rohár, robh Cc: Gregory CLEMENT, Thomas Petazzoni, Bjorn Helgaas, Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space 2021-07-23 15:52 ` Lorenzo Pieralisi @ 2021-07-23 16:45 ` Pali Rohár 2021-08-07 11:35 ` Pali Rohár 0 siblings, 1 reply; 11+ messages in thread From: Pali Rohár @ 2021-07-23 16:45 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: robh, Gregory CLEMENT, Thomas Petazzoni, Bjorn Helgaas, Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space 2021-07-23 16:45 ` Pali Rohár @ 2021-08-07 11:35 ` Pali Rohár 2021-08-15 20:30 ` Pali Rohár 0 siblings, 1 reply; 11+ messages in thread From: Pali Rohár @ 2021-08-07 11:35 UTC (permalink / raw) To: Lorenzo Pieralisi, Gregory CLEMENT Cc: robh, Thomas Petazzoni, Bjorn Helgaas, Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space 2021-08-07 11:35 ` Pali Rohár @ 2021-08-15 20:30 ` Pali Rohár 0 siblings, 0 replies; 11+ messages in thread From: Pali Rohár @ 2021-08-15 20:30 UTC (permalink / raw) To: Lorenzo Pieralisi, Gregory CLEMENT Cc: robh, Thomas Petazzoni, Bjorn Helgaas, Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space 2021-06-24 21:55 ` [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space Pali Rohár 2021-07-23 12:52 ` Gregory CLEMENT @ 2021-08-17 16:39 ` Gregory CLEMENT 1 sibling, 0 replies; 11+ messages in thread From: Gregory CLEMENT @ 2021-08-17 16:39 UTC (permalink / raw) To: Pali Rohár, Lorenzo Pieralisi, Thomas Petazzoni, Bjorn Helgaas, Rob Herring Cc: Marek Behún, Remi Pommarel, Xogium, Tomasz Maciej Nowak, Marc Zyngier, linux-pci, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] PCI: aardvark: Resource fixes 2021-06-24 21:55 [PATCH 0/2] PCI: aardvark: Resource fixes Pali Rohár 2021-06-24 21:55 ` [PATCH 1/2] PCI: aardvark: Configure PCIe resources from 'ranges' DT property Pali Rohár 2021-06-24 21:55 ` [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space Pali Rohár @ 2021-08-20 12:41 ` Lorenzo Pieralisi 2 siblings, 0 replies; 11+ messages in thread From: Lorenzo Pieralisi @ 2021-08-20 12:41 UTC (permalink / raw) To: Bjorn Helgaas, Gregory Clement, Rob Herring, Thomas Petazzoni, Pali Rohár Cc: Lorenzo Pieralisi, linux-pci, Marc Zyngier, Marek Behún, linux-arm-kernel, Tomasz Maciej Nowak, Remi Pommarel, linux-kernel, Xogium 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-08-20 12:41 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-24 21:55 [PATCH 0/2] PCI: aardvark: Resource fixes Pali Rohár 2021-06-24 21:55 ` [PATCH 1/2] PCI: aardvark: Configure PCIe resources from 'ranges' DT property Pali Rohár 2021-06-24 21:55 ` [PATCH 2/2] arm64: dts: marvell: armada-37xx: Extend PCIe MEM space Pali Rohár 2021-07-23 12:52 ` Gregory CLEMENT 2021-07-23 14:12 ` Pali Rohár 2021-07-23 15:52 ` Lorenzo Pieralisi 2021-07-23 16:45 ` Pali Rohár 2021-08-07 11:35 ` Pali Rohár 2021-08-15 20:30 ` Pali Rohár 2021-08-17 16:39 ` Gregory CLEMENT 2021-08-20 12:41 ` [PATCH 0/2] PCI: aardvark: Resource fixes Lorenzo Pieralisi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).