Stefan?
I merged the part which you did not object to. If you answer the questions below, we should be able to resolve this completely.
Regards, Carl-Daniel
On 18.08.2008 13:39, Carl-Daniel Hailfinger wrote:
On 18.08.2008 13:26, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
- known knowns. In this case the device is in the tree, i.e. not NULL,
- and we know it's there: it's soldered down or part of the on-chip
- hardware. In this case dev is not NULL.
The point above is the zeroth/third case.
- known unknowns. This is a device that might be there, but we don't
- know. So we have to probe it. It's in the dts, which is why
- it is a known unknown.
How s the function called for the known knowns? Those are neither PCI
Did you mean "called for the known UNknowns"?
No.
- The known unknowns ("listed, or expected pci devices") are called
from the dts, as you write.
- is "listed, but not necessarily present in hardware"
- The unknown unknowns ("scanned pci devices") are not in the dts but
scanned.
So what's the third case? A device is both listed in the dts and found while scanning? Are those "expected and found"?
The third (or zeroth) case are the known knowns ("listed and present in hardware").
@@ -1011,8 +1029,8 @@ if ((id == 0xffffffff) || (id == 0x00000000) || (id == 0x0000ffff) || (id == 0xffff0000)) { if (dev->enabled) {
printk(BIOS_INFO,"Disabling static device: %s\n",
printk(BIOS_INFO, "PCI: Static device not ""found, setting enabled=0: %s\n", dev_path(dev));I dislike this change of output. It's not a BIOS_DEBUG message.
Do you dislike the severity level (same as before) or the new wording (the old wording was wrong)?
Why did you consider it wrong?
The old message said "disabling static device", but the device is not touched at all.
Either the message should stay or the level should be raised to debug. Not sure. "Setting enabled=0" implies you have to understand the code in order to understand the message. This is bad for BIOS_INFO type messages (Imho, for everything more "severe" than DEBUG)
I have yet to find a better wording which says that we don't touch the physical device and set the matching struct device to disabled instead.
Improve/fix PCI device doxygen comments and printks. As a bonus, the boot messages are more understandable now.
Thanks to Ron for supplying better explanations. I've included most of them verbatim.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Ronald G. Minnich rminnich@gmail.com
Index: corebootv3-pci_device_better_prints_comments/device/pci_device.c =================================================================== --- corebootv3-pci_device_better_prints_comments/device/pci_device.c (Revision 801) +++ corebootv3-pci_device_better_prints_comments/device/pci_device.c (Arbeitskopie) @@ -963,6 +963,17 @@ * or the passed in device structure with enabled=0 if the device * does not exist in hardware and only in the tree * or NULL if no device is found and dev==NULL was passed in. + * + * There are three cases: + * - known knowns. In this case the device is in the tree, i.e. not NULL, + * and we know it's there: it's soldered down or part of the on-chip + * hardware. In this case dev is not NULL. + * - known unknowns. This is a device that might be there, but we don't + * know. So we have to probe it. It's in the dts, which is why + * it is a known unknown. + * - unknown unknowns. A PCI card in a PCI slot. We can't cover all + * possible cards. dev is NULL. We are checking to see if something is + * there; if so, we will allocate a dev and put it in the three. */ struct device *pci_probe_dev(struct device *dev, struct bus *bus, unsigned int devfn) @@ -1018,8 +1029,8 @@ if ((id == 0xffffffff) || (id == 0x00000000) || (id == 0x0000ffff) || (id == 0xffff0000)) { if (dev->enabled) { - printk(BIOS_INFO, - "Disabling static device: %s\n", + printk(BIOS_INFO, "PCI: Static device not " + "found, setting enabled=0: %s\n", dev_path(dev)); dev->enabled = 0; }