[PATCH 3/8] pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port)
Stefan Roese
sr at denx.de
Tue Nov 2 11:47:17 CET 2021
On 22.10.21 16:22, Pali Rohár wrote:
> The mysterious "Memory controller" PCI device which is present in PCI
> config space is improperly configured and crippled PCI Bridge which acts
> as PCIe Root Port for endpoint PCIe card.
>
> This PCI Bridge reports in PCI config space incorrect Class Code (Memory
> Controller) and incorrect Header Type (Type 0). It looks like HW bug in
> mvebu PCIe controller but apparently it can be changed via mvebu registers
> to correct values.
>
> The worst thing is that this PCI Bridge is crippled and its PCI config
> registers in range 0x10-0x34 alias access to internal mvebu registers which
> have different functionality as PCI Bridge registers. Moreover,
> configuration of PCI primary and secondary bus numbers (registers 0x18
> and 0x19) is done via totally different mvebu registers via totally strange
> method and cannot be done via PCI Bridge config space.
>
> Due to above fact about PCI config range 0x10-0x34, allocate a private
> cfgcache[] buffer in the driver, to which PCI config access requests to
> the 0x10-0x34 space will be redirected in mvebu_pcie_read_config() and
> mvebu_pcie_write_config() functions. Function mvebu_pcie_write_config()
> will also catch writes to PCI_PRIMARY_BUS (0x18) and PCI_SECONDARY_BUS
> (0x19) registers and set PCI Bridge primary and secondary bus numbers via
> mvebu's own method.
>
> Also, Expansion ROM Base Address register (0x38) is available, but at
> different offset 0x30. So recalculate register offset before accessing PCI
> config space.
>
> After these steps U-Boot sees working PCI Bridge and CONFIG_PCI_PNP code
> can finally start enumerating all PCIe devices correctly, even with more
> complicated PCI topology. So update also mvebu_pcie_valid_addr() function
> to reflect state of the real device topology.
>
> Each PCIe port is de-facto isolated and every PCI Bridge which is part of
> PCIe Root Complex is also isolated, so put them on separate PCI buses as
> (local) device 0.
>
> U-Boot already supports enumerating separate PCI buses, real (HW) bus
> number can be retrieved by "PCI_BUS(bdf) - dev_seq(bus)" code, so update
> config read/write functions to properly handle more complicated tree
> topologies (e.g. when a PCIe switch with multiple PCI buses is connected
> to the PCIe port).
>
> Local bus number and local device number on mvebu are used for determining
> which config request type is used (Type 0 vs Type 1). On normal non-broken
> PCIe hardware it is done by primary and secondary bus numbers. So correctly
> translate settings between these numbers to ensure that correct config
> requests are sent over the PCIe bus.
>
> As bus numbers are correctly re-configured, it does not make sense to print
> some initial bogus configuration during probe, so remove this debug code.
>
> Signed-off-by: Pali Rohár <pali at kernel.org>
> Reviewed-by: Marek Behún <marek.behun at nic.cz>
Many thanks for this huge work here. Especially with the extensive
documentation.
Reviewed-by: Stefan Roese <sr at denx.de>
Thanks,
Stefan
> ---
> drivers/pci/pci_mvebu.c | 199 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 164 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 3991086e0d29..244dcc8fb050 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -36,6 +36,7 @@ DECLARE_GLOBAL_DATA_PTR;
> #define PCIE_DEV_REV_OFF 0x0008
> #define PCIE_BAR_LO_OFF(n) (0x0010 + ((n) << 3))
> #define PCIE_BAR_HI_OFF(n) (0x0014 + ((n) << 3))
> +#define PCIE_EXP_ROM_BAR_OFF 0x0030
> #define PCIE_CAPAB_OFF 0x0060
> #define PCIE_CTRL_STAT_OFF 0x0068
> #define PCIE_HEADER_LOG_4_OFF 0x0128
> @@ -52,9 +53,9 @@ DECLARE_GLOBAL_DATA_PTR;
> #define PCIE_CONF_BUS(b) (((b) & 0xff) << 16)
> #define PCIE_CONF_DEV(d) (((d) & 0x1f) << 11)
> #define PCIE_CONF_FUNC(f) (((f) & 0x7) << 8)
> -#define PCIE_CONF_ADDR(dev, reg) \
> - (PCIE_CONF_BUS(PCI_BUS(dev)) | PCIE_CONF_DEV(PCI_DEV(dev)) | \
> - PCIE_CONF_FUNC(PCI_FUNC(dev)) | PCIE_CONF_REG(reg) | \
> +#define PCIE_CONF_ADDR(b, d, f, reg) \
> + (PCIE_CONF_BUS(b) | PCIE_CONF_DEV(d) | \
> + PCIE_CONF_FUNC(f) | PCIE_CONF_REG(reg) | \
> PCIE_CONF_ADDR_EN)
> #define PCIE_CONF_DATA_OFF 0x18fc
> #define PCIE_MASK_OFF 0x1910
> @@ -80,12 +81,13 @@ struct mvebu_pcie {
> int devfn;
> u32 lane_mask;
> int first_busno;
> - int local_dev;
> + int sec_busno;
> char name[16];
> unsigned int mem_target;
> unsigned int mem_attr;
> unsigned int io_target;
> unsigned int io_attr;
> + u32 cfgcache[0x34 - 0x10];
> };
>
> /*
> @@ -145,23 +147,18 @@ static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
> return container_of(hose, struct mvebu_pcie, hose);
> }
>
> -static int mvebu_pcie_valid_addr(struct mvebu_pcie *pcie, pci_dev_t bdf)
> +static bool mvebu_pcie_valid_addr(struct mvebu_pcie *pcie,
> + int busno, int dev, int func)
> {
> - /*
> - * There are two devices visible on local bus:
> - * * on slot configured by function mvebu_pcie_set_local_dev_nr()
> - * (by default this register is set to 0) there is a
> - * "Marvell Memory controller", which isn't useful in root complex
> - * mode,
> - * * on all other slots the real PCIe card connected to the PCIe slot.
> - *
> - * We therefore allow access only to the real PCIe card.
> - */
> - if (PCI_BUS(bdf) == pcie->first_busno &&
> - PCI_DEV(bdf) != !pcie->local_dev)
> - return 0;
> + /* On primary bus is only one PCI Bridge */
> + if (busno == pcie->first_busno && (dev != 0 || func != 0))
> + return false;
> +
> + /* On secondary bus can be only one PCIe device */
> + if (busno == pcie->sec_busno && dev != 0)
> + return false;
>
> - return 1;
> + return true;
> }
>
> static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
> @@ -169,19 +166,46 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
> enum pci_size_t size)
> {
> struct mvebu_pcie *pcie = dev_get_plat(bus);
> - u32 data;
> + int busno = PCI_BUS(bdf) - dev_seq(bus);
> + u32 addr, data;
>
> debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ",
> PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>
> - if (!mvebu_pcie_valid_addr(pcie, bdf)) {
> + if (!mvebu_pcie_valid_addr(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
> debug("- out of range\n");
> *valuep = pci_get_ff(size);
> return 0;
> }
>
> + /*
> + * mvebu has different internal registers mapped into PCI config space
> + * in range 0x10-0x34 for PCI bridge, so do not access PCI config space
> + * for this range and instead read content from driver virtual cfgcache
> + */
> + if (busno == pcie->first_busno && offset >= 0x10 && offset < 0x34) {
> + data = pcie->cfgcache[(offset - 0x10) / 4];
> + debug("(addr,size,val)=(0x%04x, %d, 0x%08x) from cfgcache\n",
> + offset, size, data);
> + *valuep = pci_conv_32_to_size(data, offset, size);
> + return 0;
> + } else if (busno == pcie->first_busno &&
> + (offset & ~3) == PCI_ROM_ADDRESS1) {
> + /* mvebu has Expansion ROM Base Address (0x38) at offset 0x30 */
> + offset -= PCI_ROM_ADDRESS1 - PCIE_EXP_ROM_BAR_OFF;
> + }
> +
> + /*
> + * PCI bridge is device 0 at primary bus but mvebu has it mapped on
> + * secondary bus with device number 1.
> + */
> + if (busno == pcie->first_busno)
> + addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset);
> + else
> + addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
> +
> /* write address */
> - writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
> + writel(addr, pcie->base + PCIE_CONF_ADDR_OFF);
>
> /* read data */
> switch (size) {
> @@ -198,6 +222,19 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
> return -EINVAL;
> }
>
> + if (busno == pcie->first_busno &&
> + (offset & ~3) == (PCI_HEADER_TYPE & ~3)) {
> + /*
> + * Change Header Type of PCI Bridge device to Type 1
> + * (0x01, used by PCI Bridges) because mvebu reports
> + * Type 0 (0x00, used by Upstream and Endpoint devices).
> + */
> + data = pci_conv_size_to_32(data, 0, offset, size);
> + data &= ~0x007f0000;
> + data |= PCI_HEADER_TYPE_BRIDGE << 16;
> + data = pci_conv_32_to_size(data, offset, size);
> + }
> +
> debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data);
> *valuep = data;
>
> @@ -209,19 +246,64 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
> enum pci_size_t size)
> {
> struct mvebu_pcie *pcie = dev_get_plat(bus);
> - u32 data;
> + int busno = PCI_BUS(bdf) - dev_seq(bus);
> + u32 addr, data;
>
> debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
> PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
> debug("(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", offset, size, value);
>
> - if (!mvebu_pcie_valid_addr(pcie, bdf)) {
> + if (!mvebu_pcie_valid_addr(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
> debug("- out of range\n");
> return 0;
> }
>
> + /*
> + * mvebu has different internal registers mapped into PCI config space
> + * in range 0x10-0x34 for PCI bridge, so do not access PCI config space
> + * for this range and instead write content to driver virtual cfgcache
> + */
> + if (busno == pcie->first_busno && offset >= 0x10 && offset < 0x34) {
> + debug("Writing to cfgcache only\n");
> + data = pcie->cfgcache[(offset - 0x10) / 4];
> + data = pci_conv_size_to_32(data, value, offset, size);
> + /* mvebu PCI bridge does not have configurable bars */
> + if ((offset & ~3) == PCI_BASE_ADDRESS_0 ||
> + (offset & ~3) == PCI_BASE_ADDRESS_1)
> + data = 0x0;
> + pcie->cfgcache[(offset - 0x10) / 4] = data;
> + /* mvebu has its own way how to set PCI primary bus number */
> + if (offset == PCI_PRIMARY_BUS) {
> + pcie->first_busno = data & 0xff;
> + debug("Primary bus number was changed to %d\n",
> + pcie->first_busno);
> + }
> + /* mvebu has its own way how to set PCI secondary bus number */
> + if (offset == PCI_SECONDARY_BUS ||
> + (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8)) {
> + pcie->sec_busno = (data >> 8) & 0xff;
> + mvebu_pcie_set_local_bus_nr(pcie, pcie->sec_busno);
> + debug("Secondary bus number was changed to %d\n",
> + pcie->sec_busno);
> + }
> + return 0;
> + } else if (busno == pcie->first_busno &&
> + (offset & ~3) == PCI_ROM_ADDRESS1) {
> + /* mvebu has Expansion ROM Base Address (0x38) at offset 0x30 */
> + offset -= PCI_ROM_ADDRESS1 - PCIE_EXP_ROM_BAR_OFF;
> + }
> +
> + /*
> + * PCI bridge is device 0 at primary bus but mvebu has it mapped on
> + * secondary bus with device number 1.
> + */
> + if (busno == pcie->first_busno)
> + addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset);
> + else
> + addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
> +
> /* write address */
> - writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
> + writel(addr, pcie->base + PCIE_CONF_ADDR_OFF);
>
> /* write data */
> switch (size) {
> @@ -301,22 +383,63 @@ static int mvebu_pcie_probe(struct udevice *dev)
> struct mvebu_pcie *pcie = dev_get_plat(dev);
> struct udevice *ctlr = pci_get_controller(dev);
> struct pci_controller *hose = dev_get_uclass_priv(ctlr);
> - int bus = dev_seq(dev);
> u32 reg;
>
> debug("%s: PCIe %d.%d - up, base %08x\n", __func__,
> pcie->port, pcie->lane, (u32)pcie->base);
>
> - /* Read Id info and local bus/dev */
> - debug("direct conf read %08x, local bus %d, local dev %d\n",
> - readl(pcie->base), mvebu_pcie_get_local_bus_nr(pcie),
> - mvebu_pcie_get_local_dev_nr(pcie));
> -
> - pcie->first_busno = bus;
> - pcie->local_dev = 1;
> + /*
> + * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
> + * because default value is Memory controller (0x508000) which
> + * U-Boot cannot recognize as P2P Bridge.
> + *
> + * Note that this mvebu PCI Bridge does not have compliant Type 1
> + * Configuration Space. Header Type is reported as Type 0 and in
> + * range 0x10-0x34 it has aliased internal mvebu registers 0x10-0x34
> + * (e.g. PCIE_BAR_LO_OFF) and register 0x38 is reserved.
> + *
> + * Driver for this range redirects access to virtual cfgcache[] buffer
> + * which avoids changing internal mvebu registers. And changes Header
> + * Type response value to Type 1.
> + */
> + reg = readl(pcie->base + PCIE_DEV_REV_OFF);
> + reg &= ~0xffffff00;
> + reg |= (PCI_CLASS_BRIDGE_PCI << 8) << 8;
> + writel(reg, pcie->base + PCIE_DEV_REV_OFF);
>
> - mvebu_pcie_set_local_bus_nr(pcie, bus);
> - mvebu_pcie_set_local_dev_nr(pcie, pcie->local_dev);
> + /*
> + * mvebu uses local bus number and local device number to determinate
> + * type of config request. Type 0 is used if target bus number equals
> + * local bus number and target device number differs from local device
> + * number. Type 1 is used if target bus number differs from local bus
> + * number. And when target bus number equals local bus number and
> + * target device equals local device number then request is routed to
> + * PCI Bridge which represent local PCIe Root Port.
> + *
> + * It means that PCI primary and secondary buses shares one bus number
> + * which is configured via local bus number. Determination if config
> + * request should go to primary or secondary bus is done based on local
> + * device number.
> + *
> + * PCIe is point-to-point bus, so at secondary bus is always exactly one
> + * device with number 0. So set local device number to 1, it would not
> + * conflict with any device on secondary bus number and will ensure that
> + * accessing secondary bus and all buses behind secondary would work
> + * automatically and correctly. Therefore this configuration of local
> + * device number implies that setting of local bus number configures
> + * secondary bus number. Set it to 0 as U-Boot CONFIG_PCI_PNP code will
> + * later configure it via config write requests to the correct value.
> + * mvebu_pcie_write_config() catches config write requests which tries
> + * to change primary/secondary bus number and correctly updates local
> + * bus number based on new secondary bus number.
> + *
> + * With this configuration is PCI Bridge available at secondary bus as
> + * device number 1. But it must be available at primary bus as device
> + * number 0. So in mvebu_pcie_read_config() and mvebu_pcie_write_config()
> + * functions rewrite address to the real one when accessing primary bus.
> + */
> + 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 + PCIE_MEM_SIZE - 1;
> @@ -366,6 +489,12 @@ static int mvebu_pcie_probe(struct udevice *dev)
> writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
> writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
>
> + /* PCI Bridge support 32-bit I/O and 64-bit prefetch mem addressing */
> + pcie->cfgcache[(PCI_IO_BASE - 0x10) / 4] =
> + PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8);
> + pcie->cfgcache[(PCI_PREF_MEMORY_BASE - 0x10) / 4] =
> + PCI_PREF_RANGE_TYPE_64 | (PCI_PREF_RANGE_TYPE_64 << 16);
> +
> return 0;
> }
>
>
Viele Grüße,
Stefan
--
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