[U-Boot] [PATCH 1/1] pci: pci_mvebu: fix bus enumeration if some buses have empty slots

[U-Boot] [PATCH 1/1] pci: pci_mvebu: fix bus enumeration if some buses have empty slots

[Marek Behún]

The ofdata_to_platdata method for this driver returns -ENODEV if link is
down for a given bus, for example if there is no device in the slot.
This causes the uclass_{first,next}_device to return NULL for this bus
in pci-uclass.c:pci_init, which of course stops probing of buses which
come after.

So if the slot on the first bus is empty on Turris Omnia, and the slot
on second bus has a device connected, the device is not probed in
U-Boot. On Turris Omnia the PCIe devices have to be probed in U-Boot to
work correctly in Linux. Therefore we need this fix.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Stefan Roese <sr@denx.de>
Cc: Anton Schubert <anton.schubert@gmx.de>
Cc: Dirk Eibach <dirk.eibach@gdsys.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Phil Sutter <phil@nwl.cc>
Cc: VlaoMao <vlaomao@gmail.com>
---
 drivers/pci/pci_mvebu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 653f445a0f..7ec6a2be27 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -436,7 +436,6 @@ static int mvebu_pcie_ofdata_to_platdata(struct udevice *dev)
 	/* Check link and skip ports that have no link */
 	if (!mvebu_pcie_link_up(pcie)) {
 		debug("%s: %s - down\n", __func__, pcie->name);
-		ret = -ENODEV;
 		goto err;
 	}
 
-- 
2.21.0

[U-Boot] [PATCH 1/1] pci: pci_mvebu: fix bus enumeration if some buses have empty slots

[Marek Behún]

On Tue, 14 May 2019 16:58:59 +0200
Marek Behún <marek.behun@nic.cz> wrote:

> The ofdata_to_platdata method for this driver returns -ENODEV if link
> is down for a given bus, for example if there is no device in the
> slot. This causes the uclass_{first,next}_device to return NULL for
> this bus in pci-uclass.c:pci_init, which of course stops probing of
> buses which come after.
> 
> So if the slot on the first bus is empty on Turris Omnia, and the slot
> on second bus has a device connected, the device is not probed in
> U-Boot. On Turris Omnia the PCIe devices have to be probed in U-Boot
> to work correctly in Linux. Therefore we need this fix.
> 
...
>  	if (!mvebu_pcie_link_up(pcie)) {
>  		debug("%s: %s - down\n", __func__, pcie->name);
> -		ret = -ENODEV;
>  		goto err;
>  	}
>  

The problem is how uclass_{first,next}_device functions work.

They use helpers uclass_find_{first,next}_device, which iterate all
devices. But uclass_{first,next}_device functions also use
uclass_get_device_tail on the device returned by helper. This function
can return NULL if device failed to probe, which causes the iteration
to stop.

Wouldn't it be better if uclass_next_device tried iterating via
uclass_find_next_device until a device was found which probed
successfully?

I don't know if this would break other things in U-Boot though.

Marek

[U-Boot] [PATCH 1/1] pci: pci_mvebu: fix bus enumeration if some buses have empty slots

[Marek Behún]

The documentation for the uclass_next_device says this:

  @devp: On entry, pointer to device to lookup. On exit, returns pointer
  to the next device in the uclass if no error occurred, or NULL if
  there is no next device, or an error occurred with that next device.

But this is useless, because if an error occured with that next device,
the iteration stops and devices which should work won't be probed.

[U-Boot] [PATCH 1/1] pci: pci_mvebu: fix bus enumeration if some buses have empty slots

[Mario Six]

Hi Marek,

On Tue, May 14, 2019 at 5:12 PM Marek Behún <marek.behun@nic.cz> wrote:
>
> The documentation for the uclass_next_device says this:
>
>   @devp: On entry, pointer to device to lookup. On exit, returns pointer
>   to the next device in the uclass if no error occurred, or NULL if
>   there is no next device, or an error occurred with that next device.
>
> But this is useless, because if an error occured with that next device,
> the iteration stops and devices which should work won't be probed.

The class_{first,next}_device_check functions do exactly what you need: They
skip the devices that won't probe and only return the ones that do probe.

Best regards,

Mario

[U-Boot] [PATCH 1/1] pci: pci_mvebu: fix bus enumeration if some buses have empty slots

[Marek Behún]

Hi Mario, you are right. I shall send a new patch chaning pci_init to
use the _check functions after I test it.
Marek

On Wed, 15 May 2019 07:05:43 +0200
Mario Six <mario.six@gdsys.cc> wrote:

> Hi Marek,
> 
> On Tue, May 14, 2019 at 5:12 PM Marek Behún <marek.behun@nic.cz>
> wrote:
> >
> > The documentation for the uclass_next_device says this:
> >
> >   @devp: On entry, pointer to device to lookup. On exit, returns
> > pointer to the next device in the uclass if no error occurred, or
> > NULL if there is no next device, or an error occurred with that
> > next device.
> >
> > But this is useless, because if an error occured with that next
> > device, the iteration stops and devices which should work won't be
> > probed.  
> 
> The class_{first,next}_device_check functions do exactly what you
> need: They skip the devices that won't probe and only return the ones
> that do probe.
> 
> Best regards,
> 
> Mario