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

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; }

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);
goto err; }ret = -ENODEV;
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

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.

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

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
participants (2)
-
Marek Behún
-
Mario Six