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

Peter Stuge peter at stuge.se
Wed Oct 29 18:44:57 CET 2008

Uwe Hermann wrote:
> > >  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.

It _is_ the information. 0x27df is the number encoded in harware.

> The #define PCI_DEVICE_ID_INTEL_82801GB_IDE however, where-ever in
> the code I stumble upon it,

I'll try to clarify. My point is that context is very important for
whether a define makes sense or not.

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

Not really. The only way to know that is to look at hardware
documentation (and hope it's correct) and compare that to the number
you get from hardware.

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

Again; the motivation is that in this context the number is more
useful than the name.

> just drop pci_ids.h entirely. I cannot image we'd want that.

Let's do it! :) No, of course defines have their uses, but I think
readability is a function of more than individual lines of code.
Context is important.


More information about the coreboot mailing list