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

[Carl-Daniel Hailfinger]

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.
>
> 1. The known unknowns ("listed, or expected pci devices") are called
> from the dts, as you write.
>   

1. is "listed, but not necessarily present in hardware"

> 2. 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.


Regards,
Carl-Daniel

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