[coreboot] [PATCH] v3: fix PCI bus scan / device tree interaction

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Fri Aug 29 04:22:02 CEST 2008


Remember when I said many months ago that something with PCI was fishy?
That PCI devices were in the device tree twice and init functions were
called multiple times? Turns out I was right and the bug is totally
nasty. I finally tracked it down after tracing function calls for 6
hours in qemu.

pci_scan_bus(bus, ...) is not called with the expected bus type.
Instead, it is called with a domain type from pci_domain_scan_bus().
pci_scan_bus(bus, ...) looks at bus->children and expects it to be a PCI
device. Of course the children of the domain are the buses and
bus->children is a PCI bus. That causes all lookups in the device tree
to fail horribly.

This patch fixes the dts lookup, BUT it introduces new undesired behaviour:

- Only the first bus of a PCI domain is scanned. That's fixable in an
obvious way.

- For dynamic PCI devices on qemu, have_resources is now always 0. The
reason can be seen in this error message:
"read_resources: bus_0(PCI_BUS: 0000) missing phase4_read_resources"
Basically, we have phase4_read_resources defined for the PCI domain, but
not for PCI bus 0. We can either inherit phase4_read_resources from the
domain or we define phase4_read_resources for each PCI bus. I prefer the
latter.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: corebootv3-pci_scan_bus/device/pci_device.c
===================================================================
--- corebootv3-pci_scan_bus/device/pci_device.c	(Revision 842)
+++ corebootv3-pci_scan_bus/device/pci_device.c	(Arbeitskopie)
@@ -636,6 +636,8 @@
 			vendor = mainboard_pci_subsystem_vendor;
 		if (!device)
 			device = mainboard_pci_subsystem_device;
+	} else {
+		printk(BIOS_DEBUG, "%s: Device not on_mainboard\n", dev_path(dev));
 	}
 #endif
 	/* Set the subsystem vendor and device ID for mainboard devices. */
@@ -1098,6 +1100,10 @@
 
 	printk(BIOS_DEBUG, "%s start bus %p, bus->dev %p\n", __func__, bus,
 	       bus->dev);
+	if (bus->dev->path.type != DEVICE_PATH_PCI_BUS)
+		printk(BIOS_ERR, "ERROR: pci_scan_bus called with incorrect "
+		       "bus->dev->path.type, path is %s\n", dev_path(bus->dev));
+
 #if PCI_BUS_SEGN_BITS
 	printk(BIOS_DEBUG, "PCI: pci_scan_bus for bus %04x:%02x\n",
 	       bus->secondary >> 8, bus->secondary & 0xff);
@@ -1139,6 +1145,8 @@
 		if ((PCI_FUNC(devfn) == 0x00) &&
 		    (!dev
 		     || (dev->enabled && ((dev->hdr_type & 0x80) != 0x80)))) {
+			printk(BIOS_SPEW, "Not a multi function device, or the "
+			       "device is not present. Skip to next device.\n");
 			devfn += 0x07;
 		}
 	}
@@ -1186,8 +1194,9 @@
  */
 unsigned int pci_domain_scan_bus(struct device *dev, unsigned int max)
 {
+	printk(BIOS_SPEW, "pci_domain_scan_bus: calling pci_scan_bus\n");
 	/* There is only one link on this device, and it is always link 0. */
-	return pci_scan_bus(&dev->link[0], PCI_DEVFN(0, 0), 0xff, max);
+	return pci_scan_bus(&dev->link[0].children->link[0], PCI_DEVFN(0, 0), 0xff, max);
 }
 
 /**
@@ -1277,6 +1286,7 @@
  */
 unsigned int pci_scan_bridge(struct device *dev, unsigned int max)
 {
+	printk(BIOS_SPEW, "pci_scan_bridge: calling pci_scan_bus\n");
 	return do_pci_scan_bridge(dev, max, pci_scan_bus);
 }
 


-- 
http://www.hailfinger.org/





More information about the coreboot mailing list