On 04.11.2008 21:04, Myles Watson wrote:
This patch clarifies/adds comments and changes names in device/pci_device.c
It also changes %p debug statements in various places. I think they get in the way of diffs when you have log files to compare. I don't want to see the allocation differences most of the time. I turned most of them into NULL checks. If they were supposed to be "Where are we in device allocation?" checks, we could make them into that too.
It's a work-in-progress. Comments welcome.
I think most of the changes are self explanatory, but this one might not be:
If you are reading all the BARs from a device, and you come to a 64-bit BAR. No matter why you skip it, you should skip it as a 64-bit BAR, and not try to read the upper half as the next 32-bit BAR.
Because of that, set the 64-bit flag IORESOURCE_PCI64 early, and don't clear it on return.
Signed-off-by: Myles Watson mylesgw@gmail.com
@@ -899,18 +924,18 @@
- Given a linked list of PCI device structures and a devfn number, find the
- device structure correspond to the devfn, if present. This function also
- removes the device structure from the linked list.
*/
- @param list The device structure list.
- @param devfn A device/function number.
- @return Pointer to the device structure found or NULL of we have not
allocated a device for this devfn yet.-static struct device *pci_scan_get_dev(struct device **list, unsigned int devfn) +static struct device *pci_get_dev(struct device **list, unsigned int devfn) { struct device *dev; dev = 0;
- printk(BIOS_SPEW, "%s: list is %p, *list is %p\n", __func__, list,
*list);
- printk(BIOS_SPEW, "%s: list is %sNULL, *list is %sNULL\n", __func__,
list?"NOT ":"", *list?"NOT ":"");
Was that change really intentional? Sure, it makes diffing easier, but some diagnostics (especially considering the still buggy device enumeration) are now more difficult.
@@ -1043,15 +1068,22 @@ dev->irq_pin = pci_read_config8(dev, PCI_INTERRUPT_PIN); dev->min_gnt = pci_read_config8(dev, PCI_MIN_GNT); dev->max_lat = pci_read_config8(dev, PCI_MAX_LAT); -#warning Per-device subsystem ID should only be read from the device if none has been specified for the device in the dts.
dev->subsystem_vendor = pci_read_config16(dev, PCI_SUBSYSTEM_VENDOR_ID);
dev->subsystem_device = pci_read_config16(dev, PCI_SUBSYSTEM_ID);
/* Store the interesting information in the device structure. */
- /*Per-device subsystem ID should only be read from the device if none
* has been specified for the device in the dts.*/- if (!dev->subsystem_vendor && !dev->subsystem_device) {
dev->subsystem_vendor =pci_read_config16(dev, PCI_SUBSYSTEM_VENDOR_ID);dev->subsystem_device =pci_read_config16(dev, PCI_SUBSYSTEM_ID);- }
- dev->id.type = DEVICE_ID_PCI; dev->id.pci.vendor = id & 0xffff; dev->id.pci.device = (id >> 16) & 0xffff; dev->hdr_type = hdr_type;
- /* Class code, the upper 3 bytes of PCI_CLASS_REVISION. */ dev->class = class >> 8;
Hm. Although you fixed the code as indicated in the #warning, I fear there's still something missing and that's the reason the warning was there. Where do we set the subsystem ID of the real device (not struct device)?
@@ -1105,6 +1135,8 @@
printk(BIOS_DEBUG, "%s start bus %p, bus->dev %p\n", __func__, bus, bus->dev);
+#warning This check needs checking. 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 you manage to fix the device code in a way which doesn't trigger this anymore, your static devices will suddenly be found. Since this check is only triggered if your code and/or device tree is really buggy, we could upgrade it to a die().
Regards, Carl-Daniel