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

Stefan Reinauer stepan at coresystems.de
Wed Oct 29 21:39:39 CET 2008


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.
>
>  
>   
>>>  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).
>   
What other PCI IDs would you expect in
southbridge/intel/i82801gx/i82801gx_ide.c
> 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.
>   
When looking at that part of the code, you always want to know the
number, so you basically add another indirection to the code. Especially
since the code contains comments on what devices are associated with
those IDs

> 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 think they're fine in the code, but they hurt in the driver ID tables.

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20081029/6a028685/attachment.sig>


More information about the coreboot mailing list