[U-Boot] [PATCH 1/1] pci: pci_mvebu: ignore the local device

[Stefan Roese]

On 10.05.19 04:56, Marek Behún wrote:
> The local device (host "bridge"?) on pci_mvebu controller reports
> PCI_CLASS_MEMORY_OTHER in the class register.
> 
> This does not work in U-Boot, because U-Boot then tries to autoconfigure
> this device in pci_auto.c. This causes the real connected PCIe device
> not to work (in U-Boot nor in Linux).

What exactly does not work or is the issue here with the current
implementation in Linux? I'm asking, since we have not noticed any
issues here?

Thanks,
Stefan
  
> Linux solves this by emulating PCI bridge device (see
> drivers/pci/pci-bridge-emul.c in Linux). The Marvell vendor/device IDs
> are used for this pci-bridge-emul device in Linux, but the actual
> register accesses are emulated and the value of class register is
> PCI_CLASS_BRIDGE_PCI.
> 
> The simplest solution here in my opinion is to just ignore the local
> device in this driver's read/write methods. This fixes PCIe issues for
> me.
> 
> Signed-off-by: Marek Behún <marek.behun at nic.cz>
> Cc: Stefan Roese <sr at denx.de>
> Cc: Anton Schubert <anton.schubert at gmx.de>
> Cc: Dirk Eibach <dirk.eibach at gdsys.cc>
> Cc: Mario Six <mario.six at gdsys.cc>
> Cc: Chris Packham <chris.packham at alliedtelesis.co.nz>
> Cc: Phil Sutter <phil at nwl.cc>
> Cc: VlaoMao <vlaomao at gmail.com>
> ---
>   drivers/pci/pci_mvebu.c | 59 +++++++++++++++++------------------------
>   1 file changed, 24 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index e21dc10c2f..653f445a0f 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -143,31 +143,31 @@ static int mvebu_pcie_read_config(struct udevice *bus, pci_dev_t bdf,
>   	struct mvebu_pcie *pcie = dev_get_platdata(bus);
>   	int local_bus = PCI_BUS(pcie->dev);
>   	int local_dev = PCI_DEV(pcie->dev);
> +	int other_dev;
>   	u32 reg;
>   	u32 data;
>   
>   	debug("PCIE CFG read:  (b,d,f)=(%2d,%2d,%2d) ",
>   	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>   
> -	/* Only allow one other device besides the local one on the local bus */
> -	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) {
> -		if (local_dev == 0 && PCI_DEV(bdf) != 1) {
> -			debug("- out of range\n");
> -			/*
> -			 * If local dev is 0, the first other dev can
> -			 * only be 1
> -			 */
> -			*valuep = pci_get_ff(size);
> -			return 0;
> -		} else if (local_dev != 0 && PCI_DEV(bdf) != 0) {
> -			debug("- out of range\n");
> -			/*
> -			 * If local dev is not 0, the first other dev can
> -			 * only be 0
> -			 */
> -			*valuep = pci_get_ff(size);
> -			return 0;
> -		}
> +	/*
> +	 * The local device has PCI_CLASS_MEMORY_OTHER in the class register.
> +	 * This does not work for U-Boot because pci_auto.c tries to configure
> +	 * this as a device. This results in U-Boot/Linux being unable to access
> +	 * PCIe devices.
> +	 *
> +	 * Linux solves this by emulating a bridge (see
> +	 * drivers/pci/pci-bridge-emul.c in Linux).
> +	 *
> +	 * Let's ignore the local device in U-Boot.
> +	 *
> +	 * Also only allow one other device besides the (ignored) local one.
> +	 */
> +
> +	other_dev = local_dev ? 0 : 1;
> +	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != other_dev) {
> +		*valuep = pci_get_ff(size);
> +		return 0;
>   	}
>   
>   	/* write address */
> @@ -187,28 +187,17 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
>   	struct mvebu_pcie *pcie = dev_get_platdata(bus);
>   	int local_bus = PCI_BUS(pcie->dev);
>   	int local_dev = PCI_DEV(pcie->dev);
> +	int other_dev;
>   	u32 data;
>   
>   	debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
>   	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>   	debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value);
>   
> -	/* Only allow one other device besides the local one on the local bus */
> -	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) {
> -		if (local_dev == 0 && PCI_DEV(bdf) != 1) {
> -			/*
> -			 * If local dev is 0, the first other dev can
> -			 * only be 1
> -			 */
> -			return 0;
> -		} else if (local_dev != 0 && PCI_DEV(bdf) != 0) {
> -			/*
> -			 * If local dev is not 0, the first other dev can
> -			 * only be 0
> -			 */
> -			return 0;
> -		}
> -	}
> +	/* See the comment in mvebu_pcie_read_config */
> +	other_dev = local_dev ? 0 : 1;
> +	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != other_dev)
> +		return 0;
>   
>   	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
>   	data = pci_conv_size_to_32(0, value, offset, size);
> 

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