Re: [PATCH u-boot-marvell 4/4] arm: a37xx: pci: Ensure that root port is always on root zero bus

On 2/10/22 14:53, Pali Rohár wrote:

Writing to the PCI_PRIMARY_BUS register of the root port should not change bus number on which is root port present.

This PCI_PRIMARY_BUS register is used only for correct configuration of legacy PCI stuff, like forwarding of PCI special cycles between buses.

Aardvark HW does not support PCI special cycles, so it does not have HW register for PCI_PRIMARY_BUS and therefore it does not matter what is stored in this register.

So fix this issue and do not use PCI_PRIMARY_BUS register in pci-aardvark.c driver for moving root bus of the root port.

After this change there is no reason for storing bus number (zero) into first_busno variable, so remove this variable.

Signed-off-by: Pali Rohár pali@kernel.org Fixes: cb056005dc67 ("arm: a37xx: pci: Add support for accessing PCI Bridge on root bus")

Applied to u-boot-marvell/master

Thanks, Stefan

---

drivers/pci/pci-aardvark.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index 6e65c0e1558c..1eb257ea8b4a 100644 --- a/drivers/pci/pci-aardvark.c +++ b/drivers/pci/pci-aardvark.c @@ -151,8 +151,6 @@

• struct pcie_advk - Advk PCIe controller state
• @base: The base address of the register space.

• • @first_busno: Bus number of the PCIe root-port.

• •           This may vary depending on the PCIe setup.
    

  • @sec_busno: Bus number for the device behind the PCIe root-port.
  • @dev: The pointer to PCI uclass device.
  • @reset_gpio: GPIO descriptor for PERST.

@@ -162,7 +160,6 @@ */ struct pcie_advk { void *base;

• int first_busno; int sec_busno; struct udevice *dev; struct gpio_desc reset_gpio;

@@ -194,8 +191,8 @@ static inline uint advk_readl(struct pcie_advk *pcie, uint reg) static bool pcie_advk_addr_valid(struct pcie_advk *pcie, int busno, u8 dev, u8 func) {

• /* On the primary (local) bus there is only one PCI Bridge */
• if (busno == pcie->first_busno && (dev != 0 || func != 0))

• /* On the root bus there is only one PCI Bridge */

• if (busno == 0 && (dev != 0 || func != 0)) return false;

  /*

@@ -353,17 +350,17 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, }

/*

• * The configuration space of the PCI Bridge on primary (first) bus is
  

• * The configuration space of the PCI Bridge on the root bus (zero) is
  

  • not accessible via PIO transfers like all other PCIe devices. PCI
  • Bridge config registers are available directly in Aardvark memory
  • space starting at offset zero. The PCI Bridge config space is of
  • Type 0, but the BAR registers (including ROM BAR) don't have the same
  • meaning as in the PCIe specification. Therefore do not access BAR
  • registers and non-common registers (those which have different

• * meaning for Type 0 and Type 1 config space) of the primary PCI Bridge
  

• * meaning for Type 0 and Type 1 config space) of the PCI Bridge
  

  • and instead read their content from driver virtual cfgcache[].
  */

• if (busno == pcie->first_busno) {

• if (busno == 0) { if ((offset >= 0x10 && offset < 0x34) || (offset >= 0x38 && offset < 0x3c)) data = pcie->cfgcache[(offset - 0x10) / 4]; else

@@ -543,7 +540,7 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, * zero. Type 1 specific registers are not available, so we write their * content only into driver virtual cfgcache[]. */

• if (busno == pcie->first_busno) {

• if (busno == 0) { if ((offset >= 0x10 && offset < 0x34) || (offset >= 0x38 && offset < 0x3c)) { data = pcie->cfgcache[(offset - 0x10) / 4];

@@ -560,9 +557,6 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, advk_writel(pcie, data, ADVK_ROOT_PORT_PCI_CFG_OFF + (offset & ~3)); }

• if (offset == PCI_PRIMARY_BUS)
  

• 	pcie->first_busno = data & 0xff;
  

• if (offset == PCI_SECONDARY_BUS || (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8)) pcie->sec_busno = (data >> 8) & 0xff;

Viele Grüße, Stefan Roese