Re: [PATCH u-boot-marvell v2 2/6] arm: a37xx: pci: Add support for accessing PCI Bridge on root bus

On 05.10.21 23:55, Pali Rohár wrote:

On Sunday 26 September 2021 00:54:42 Marek Behún wrote:

...

@@ -355,20 +369,55 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, enum pci_size_t size) { struct pcie_advk *pcie = dev_get_priv(bus);

• int busno = PCI_BUS(bdf) - dev_seq(bus); int retry_count; bool allow_crs;

• ulong data; uint reg; int ret;

  dev_dbg(pcie->dev, "PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ", PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));

• if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) {

• if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) { dev_dbg(pcie->dev, "- out of range\n"); *valuep = pci_get_ff(size); return 0; }

• /*

• * The configuration space of the PCI Bridge on primary (local) bus 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. Moreover PCI Bridge registers in the
  

• * range 0x10 - 0x34 are not available and register 0x38 (Expansion ROM
  

• * Base Address) is at offset 0x30.
  

• * We therefore read configuration space content of the primary PCI
  

• * Bridge from our virtual cache.
  

• */
  

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

• if (offset >= 0x10 && offset < 0x34)
  

• 	data = pcie->cfgcache[(offset - 0x10) / 4];
  

• else if ((offset & ~3) == PCI_ROM_ADDRESS1)
  

• 	data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG);
  

• else
  

• 	data = advk_readl(pcie, offset & ~3);
  

• if ((offset & ~3) == (PCI_HEADER_TYPE & ~3)) {
  

• 	/*
  

• 	 * Change Header Type of PCI Bridge device to Type 1
  

• 	 * (0x01, used by PCI Bridges) because hardwired value
  

• 	 * is Type 0 (0x00, used by Endpoint devices).
  

• 	 */
  

• 	data &= ~0x00ff0000;
  

                                  ^^

There is a small mistake. Header Type is 7-bit value. Upper 8th bit indicates if device is multifunction. So correct mask should be:

	data &= ~0x007f0000;

I'll try to get to these (and other) patches today and will fix this while applying.

Thanks, Stefan

...

• 	data |= PCI_HEADER_TYPE_BRIDGE << 16;
  

• }
  

• *valuep = pci_conv_32_to_size(data, offset, size);
  

• return 0;
  

• }
• /*
  • Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to
  • OS is allowed only for 4-byte PCI_VENDOR_ID config read request and

Viele Grüße, Stefan