[coreboot] r3705 - in trunk/coreboot-v2/src: include/device northbridge/intel/i945 southbridge/intel/i82801gx

[Carl-Daniel Hailfinger]

On 29.10.2008 18:26, Uwe Hermann wrote:
> On Wed, Oct 29, 2008 at 05:02:17PM +0100, Peter Stuge wrote:
>   
>> svn at coreboot.org wrote:
>>     
>>>  static const struct pci_driver mc_driver __pci_driver = {
>>>  	.ops    = &mc_ops,
>>>  	.vendor = PCI_VENDOR_ID_INTEL,
>>> -	.device = 0x27a0,
>>> +	.device = PCI_DEVICE_ID_INTEL_945_HOST_BRIDGE,
>>>       
>> Sorry, but I don't really agree with these changes.
>>     
>
> Yes, we really seems to have a disagreement here. I think the device
> names from pci_ids.h make perfect sense, that's why we (and Linux for
> that matters) has such a file in the first place, and why we use the
> #defines instead of hard-coded PCI device numbers in the code.
>   

I have to side with Uwe here. The name is even human readable, so
anybody reading the code saves a lookup for the meaning of a magic
number. The only issue is getting the number right in the first place.


>>>  static const struct pci_driver i82801gx_ide __pci_driver = {
>>>  	.ops	= &ide_ops,
>>>  	.vendor	= PCI_VENDOR_ID_INTEL,
>>> -	.device	= 0x27df,
>>> +	.device	= PCI_DEVICE_ID_INTEL_82801GB_IDE,
>>>  };
>>>       
>> My reasoning is that #defines should add information to the code and
>> not be an end in itself.
>>     
>
> That's exactly my reasoning as well. The number 0x27df is utterly
> useless and conveys not information at all. The #define
> PCI_DEVICE_ID_INTEL_82801GB_IDE however, where-ever in the code I
> stumble upon it, I immediately know that it's an Intel device,
> it belongs to the 82801GB chipset/southbridge, and it refers to
> the IDE device (not audio, not USB, not anything else).
>
> Yes, if you really want to know the hex number you'll have to look it
> up in the pci_ids.h file but that's the case for all other PCI device
> numbers we use all over the place (and for all #defines in general).
> That shouldn't be a reason to _not_ use them, IMO.
>   

Well said.


> Or, to put it in another way, if we all agree to not use #defines for
> PCI IDs (or no #defines at all), we should just drop pci_ids.h (or all
> *.h files with #defines in them) entirely. I cannot image we'd want that.
>   

I prefer meaning over numbers.

Basically, it boils down to a choice between:
- Code which is easy to read.
- Code which is easy to check against the numbers in a data sheet.

Converting #defines to numbers is easy to do automatically (the C
preprocessor does that). The reverse action is hard and can't be
automated due to ambiguities.
That's why I want to keep the #defines.


Regards,
Carl-Daniel

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