[PATCH u-boot-marvell 07/10] pci: pci_mvebu: Fix PCIe MEM and IO resources assignment and mbus mapping
Stefan Roese
sr at denx.de
Fri Nov 19 07:27:09 CET 2021
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 <pali at 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?
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 <pali at kernel.org>
>>> Signed-off-by: Marek Behún <marek.behun at 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))) {
>>> 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: sr at denx.de
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 at denx.de
More information about the U-Boot
mailing list