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

[Marc Jones]

Corey Osgood wrote:
> On Wed, Oct 29, 2008 at 1:26 PM, Uwe Hermann <uwe at hermann-uwe.de 
> <mailto:uwe at hermann-uwe.de>> wrote:
> 
>     On Wed, Oct 29, 2008 at 05:02:17PM +0100, Peter Stuge wrote:
>      > svn at coreboot.org <mailto: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.
>     that.
> 
> 
> I'm in agreement with Peter on this one. The pci_driver is named 
> i82801gx_ide, that should clarify exactly what device this is working 
> on. I can agree that having the #define anywhere else in code (where it 
> may not be as clear what device you're aiming for) makes sense, but here 
> it could be much more useful to know the device id here, ex. if you 
> wanted to compare a register set by the driver to a stock bios' lspci 
> -nxxx.

And I will disagree with you and Peter. The #define makes the code self 
documenting. Without it you must have the documentation to know what 
device is being accessed.

Marc


-- 
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors