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

["Pali Rohár" <pali@kernel.org>]

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;

> +			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