Re: [coreboot] [PATCH] v3: improve PCI device doxygen comments and printks

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> Index: corebootv3-pci_device_better_prints_comments/device/pci_device.c
>
> --- corebootv3-pci_device_better_prints_comments/device/pci_device.c	(Revision 780)
> +++ corebootv3-pci_device_better_prints_comments/device/pci_device.c	(Arbeitskopie)
> @@ -950,12 +950,30 @@
>  /**
> Scan a PCI bus.
> Determine the existence of a given PCI device.
> Determine the existence of a given PCI device. Allocate a new struct device
> if dev==NULL was passed in and the device exists in hardware.
> @param dev Pointer to the device structure if it already is in the device
>     tree, i.e. was specified in the dts. It may not exist on hardware,
>     however. Looking for hardware not yet in the device tree has this
>     parameter as NULL.
> @param bus Pointer to the bus structure.
> @param devfn A device/function number.
> @return The device structure for the device (if found)
>     or the NULL if no device is found.
> @return The device structure for the device if it exists in hardware
>     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:

.. of what?
> 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.

How s the function called for the known knowns? Those are neither PCI
cards nor stuff in the DTS according to your description? So where does
it come from?
> 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.
> @@ -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.
> 		dev->enabled = 0;
> 	}