On Friday 12 November 2021 15:18:48 Stefan Roese wrote:
On 11/11/21 16:35, Marek Behún wrote:
From: Pali Rohár pali@kernel.org
Do not call pci_set_region() for resources which were not properly mapped. This prevents U-Boot to access unmapped memory space.
Update MBUS_PCI_MEM_SIZE and MBUS_PCI_IO_SIZE macros to cover all PCIe MEM and IO ranges. Previously these macros covered only address ranges for the first PCIe port. Between MBUS_PCI_IO_BASE and MBUS_PCI_MEM_BASE there is space for six 128 MB long address ranges. So set MBUS_PCI_MEM_SIZE to value of 6*128 MB. Similarly set MBUS_PCI_IO_SIZE to 6*64 KB.
Perhaps it makes sense to calculate this '6' in some macro?
Hm... maybe.
Or maybe better to add "#define maximal_number_of_pcie_ports 6" (but with better name!) as 6 is the maximal number of PCIe ports on 88f78xx0. Other SoCs can have maximally only 4 PCIe ports. So even if we change this default mapping in future and there would be bigger gap between MBUS_PCI_IO_BASE and MBUS_PCI_MEM_BASE, it does not make sense to change that '6' to any higher value.
What do you think?
This makes it easier to understand this value at a later time. Something like this:
#define MEM_SIZE_TOTAL (0xf0000000 - MBUS_PCI_MEM_BASE) // or some better macro for 0xf000000 here? #define MEM_REGION_COUNT (MEM_SIZE_TOTAL / (128 << 20))
?
Function resource_size() returns zero when start address is 0 and end address is -1. So set invalid resources to these values to indicate that resource has no mapping.
Split global PCIe MEM and IO resources (defined by MBUS_PCI_*_* macros) into PCIe ports in mvebu_pcie_bind() function which allocates per-port based struct mvebu_pcie, instead of using global state variables mvebu_pcie_membase and mvebu_pcie_iobase. This makes pci_mvebu.c driver independent of global static variables (which store the state of allocation) and allows to bind and unbind the driver more times.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún marek.behun@nic.cz
arch/arm/mach-mvebu/include/mach/cpu.h | 4 +- drivers/pci/pci_mvebu.c | 84 ++++++++++++++++++-------- 2 files changed, 61 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h index a7a62c7e7d..4c52a330d9 100644 --- a/arch/arm/mach-mvebu/include/mach/cpu.h +++ b/arch/arm/mach-mvebu/include/mach/cpu.h @@ -75,9 +75,9 @@ enum {
- Default Device Address MAP BAR values
*/ #define MBUS_PCI_MEM_BASE MVEBU_SDRAM_SIZE_MAX -#define MBUS_PCI_MEM_SIZE (128 << 20) +#define MBUS_PCI_MEM_SIZE ((6*128) << 20)
Nitpicking: Space before and after the '*' please.
#define MBUS_PCI_IO_BASE 0xF1100000 -#define MBUS_PCI_IO_SIZE (64 << 10) +#define MBUS_PCI_IO_SIZE ((6*64) << 10)
Same here.
#define MBUS_SPI_BASE 0xF4000000 #define MBUS_SPI_SIZE (8 << 20) #define MBUS_DFX_BASE 0xF6000000 diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index 701a17dfb7..fea32414bf 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -26,6 +26,7 @@ #include <linux/errno.h> #include <linux/ioport.h> #include <linux/mbus.h> +#include <linux/sizes.h> DECLARE_GLOBAL_DATA_PTR; @@ -96,14 +97,6 @@ struct mvebu_pcie { u32 cfgcache[(0x3c - 0x10) / 4]; }; -/*
- MVEBU PCIe controller needs MEMORY and I/O BARs to be mapped
- into SoCs address space. Each controller will map 128M of MEM
- and 64K of I/O space when registered.
- */
-static void __iomem *mvebu_pcie_membase = (void __iomem *)MBUS_PCI_MEM_BASE; -static void __iomem *mvebu_pcie_iobase = (void __iomem *)MBUS_PCI_IO_BASE;
- static inline bool mvebu_pcie_link_up(struct mvebu_pcie *pcie) { u32 val;
@@ -478,26 +471,24 @@ static int mvebu_pcie_probe(struct udevice *dev) mvebu_pcie_set_local_bus_nr(pcie, 0); mvebu_pcie_set_local_dev_nr(pcie, 1);
- pcie->mem.start = (u32)mvebu_pcie_membase;
- pcie->mem.end = pcie->mem.start + MBUS_PCI_MEM_SIZE - 1;
- mvebu_pcie_membase += MBUS_PCI_MEM_SIZE;
- if (mvebu_mbus_add_window_by_id(pcie->mem_target, pcie->mem_attr,
- if (resource_size(&pcie->mem) &&
mvebu_mbus_add_window_by_id(pcie->mem_target, pcie->mem_attr, (phys_addr_t)pcie->mem.start, resource_size(&pcie->mem))) {pcie->mem.start = 0;pcie->mem.end = -1;
- pcie->io.start = (u32)mvebu_pcie_iobase;
- pcie->io.end = pcie->io.start + MBUS_PCI_IO_SIZE - 1;
- mvebu_pcie_iobase += MBUS_PCI_IO_SIZE;
- if (mvebu_mbus_add_window_by_id(pcie->io_target, pcie->io_attr,
- if (resource_size(&pcie->io) &&
mvebu_mbus_add_window_by_id(pcie->io_target, pcie->io_attr, (phys_addr_t)pcie->io.start, resource_size(&pcie->io))) {pcie->io.start = 0;pcie->io.end = -1;@@ -506,13 +497,23 @@ static int mvebu_pcie_probe(struct udevice *dev) /* PCI memory space */ pci_set_region(hose->regions + 0, pcie->mem.start, pcie->mem.start, resource_size(&pcie->mem), PCI_REGION_MEM);
- pci_set_region(hose->regions + 1,
0, 0,gd->ram_size,PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);- pci_set_region(hose->regions + 2, pcie->io.start,
pcie->io.start, resource_size(&pcie->io), PCI_REGION_IO);- hose->region_count = 3;
- hose->region_count = 1;
- if (resource_size(&pcie->mem)) {
pci_set_region(hose->regions + hose->region_count,pcie->mem.start, pcie->mem.start,resource_size(&pcie->mem),PCI_REGION_MEM);hose->region_count++;- }
- if (resource_size(&pcie->io)) {
pci_set_region(hose->regions + hose->region_count,pcie->io.start, pcie->io.start,resource_size(&pcie->io),PCI_REGION_IO);hose->region_count++;- } /* PCI Bridge support 32-bit I/O and 64-bit prefetch mem addressing */ pcie->cfgcache[(PCI_IO_BASE - 0x10) / 4] =
@@ -680,6 +681,8 @@ static int mvebu_pcie_bind(struct udevice *parent) struct mvebu_pcie *pcie; struct uclass_driver *drv; struct udevice *dev;
- struct resource mem;
- struct resource io; ofnode subnode; /* Lookup pci driver */
@@ -689,6 +692,11 @@ static int mvebu_pcie_bind(struct udevice *parent) return -ENOENT; }
- mem.start = MBUS_PCI_MEM_BASE;
- mem.end = MBUS_PCI_MEM_BASE + MBUS_PCI_MEM_SIZE - 1;
- io.start = MBUS_PCI_IO_BASE;
- io.end = MBUS_PCI_IO_BASE + MBUS_PCI_IO_SIZE - 1;
- ofnode_for_each_subnode(subnode, dev_ofnode(parent)) { if (!ofnode_is_available(subnode)) continue;
@@ -697,6 +705,32 @@ static int mvebu_pcie_bind(struct udevice *parent) if (!pcie) return -ENOMEM;
/** MVEBU PCIe controller needs MEMORY and I/O BARs to be mapped* into SoCs address space. Each controller will map 128M of MEM* and 64K of I/O space when registered.*/if (resource_size(&mem) >= SZ_128M) {pcie->mem.start = mem.start;pcie->mem.end = mem.start + SZ_128M - 1;mem.start += SZ_128M;} else {printf("PCIe unable to assign mbus window for mem\n");pcie->mem.start = 0;pcie->mem.end = -1;}if (resource_size(&io) >= SZ_64K) {pcie->io.start = io.start;pcie->io.end = io.start + SZ_64K - 1;io.start += SZ_64K;} else {printf("PCIe unable to assign mbus window for io\n");pcie->io.start = 0;pcie->io.end = -1;}- /* Create child device UCLASS_PCI and bind it */ device_bind(parent, &pcie_mvebu_drv, pcie->name, pcie, subnode, &dev);
Viele Grüße, Stefan Roese
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de