Re: [LinuxBIOS] r2951 - trunk/LinuxBIOSv2/src/southbridge/intel/i82801xx

[Corey Osgood]

[e-mail] wrote:
> Little confused by this. Isn't this supposed to be posted to the list  
> first for review, before being commited? Is it ok for the same person  
> to "Signed-off-by:" and "Acked-by:"?? See comments below...

This qualifies as a "trivial" patch, in other words the only changes are
to improve comments and fix possible whitespace errors (I think using
defined PCI IDs also qualifies as trivial). There are no real changes to
the actual code, so nothing should be broken in the process. Hence the
normal process is forgone.
> > Modified: trunk/LinuxBIOSv2/src/southbridge/intel/i82801xx/i82801xx.c
> >
> > trunk/LinuxBIOSv2/src/southbridge/intel/i82801xx/i82801xx.c	2007-11-07   
> > 19:07:17 UTC (rev 2950)
> > +++   
> > trunk/LinuxBIOSv2/src/southbridge/intel/i82801xx/i82801xx.c	2007-11-07   
> > 22:09:02 UTC (rev 2951)
> > @@ -30,7 +30,7 @@
> >    unsigned int index = 0;
> >    uint16_t cur_disable_mask, new_disable_mask;
> > /* All 82801 devices should be on bus 0. */
> > /* All 82801xx devices should be on bus 0. */
> > unsigned int devfn = PCI_DEVFN(0x1f, 0);	// LPC
> > device_t lpc_dev = dev_find_slot(0, devfn);	// 0
> > if (!lpc_dev)
> >
> > @@ -50,10 +50,11 @@
> >    if (index == 0) {
> >    	index = 14;
> >    }
>
> All devices are not on Bus 0. The NIC and PCI Cards are on Bus 1 down  
> from the PCI Bus.

Interesting, I'll have a look at that tomorrow. This isn't a change in
the code in any way though, just fixing a comment.
> > -static const struct pci_driver i82801dbm_nic __pci_driver = {
> > +/* Note: There's no NIC on 82801AA/AB (ICH/ICH0). */
> >
> > +/* 82801BA/BAM/CA/CAM (ICH2/ICH2-M/ICH3-S/ICH3-M) */
> > +static const struct pci_driver i82801ba_nic __pci_driver = {
> >    .ops	= &nic_ops,
> >    .vendor	= PCI_VENDOR_ID_INTEL,
> > .device	= 0x103a,
> > .device	= PCI_DEVICE_ID_INTEL_82801BA_LAN,
> >
> > };
> >
> > -static const struct pci_driver i82801ex_nic __pci_driver = {
> > +/* 82801DB/DBL/DBM (ICH4/ICH4-L/ICH4-M) */
> > +static const struct pci_driver i82801db_nic __pci_driver = {
> >    .ops	= &nic_ops,
> >    .vendor	= PCI_VENDOR_ID_INTEL,
> > .device	= 0x1051,
> > .device	= PCI_DEVICE_ID_INTEL_82801DB_LAN,
> >
> > };
>
> My NIC on the 82801DB is .device = 0x103a, this is also what is in the  
> datasheet.

It's because of the way the patch got set up. For some reason, it's
replacing the lines where the i82801ex driver was with the i82801db
(which is a few lines up). Since PCI_DEVICE_ID_INTEL_82801DB_LAN =
0x103a, it should be fine.
> > Modified: trunk/LinuxBIOSv2/src/southbridge/intel/i82801xx/i82801xx_sata.c
> >
> > trunk/LinuxBIOSv2/src/southbridge/intel/i82801xx/i82801xx_sata.c	2007-11-07   
> > 19:07:17 UTC (rev 2950)
> > +++   
> > trunk/LinuxBIOSv2/src/southbridge/intel/i82801xx/i82801xx_sata.c	2007-11-07   
> > 22:09:02 UTC (rev 2951)
> > @@ -23,7 +23,6 @@
> >  #include <device/device.h>
> >  #include <device/pci.h>
> >  #include <device/pci_ids.h>
> > -#include <device/pci_ops.h>
> >  #include "i82801xx.h"
> >
> > /* TODO: Set dynamically, if the user only wants one SATA channel or none
>
> This should be setup to only work with ICH's that support sata.

And it does. The driver is included per default, but will only load/run
when LB comes across a device ID for an SATA device. No ID, the driver
doesn't do anything. It should probably be somehow fixed to only build
for SATA-supporting chipsets, but alas the world isn't perfect ;)

-Corey