On 11/18/21 18:46, Pali Rohár wrote:
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 <[email protected]>

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?

Sounds good to me.

Thanks,
Stefan

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 <[email protected]>
Signed-off-by: Marek Behún <[email protected]>
---
   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))) {
                printf("PCIe unable to add mbus window for mem at %08x+%08x\n",
                       (u32)pcie->mem.start, 
(unsigned)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))) {
                printf("PCIe unable to add mbus window for IO at %08x+%08x\n",
                       (u32)pcie->io.start, (unsigned)resource_size(&pcie->io));
+               pcie->io.start = 0;
+               pcie->io.end = -1;
        }
        /* Setup windows and configure host bridge */
@@ -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: [email protected]

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

Reply via email to