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

[Uwe Hermann]

On Thu, Nov 08, 2007 at 01:09:16AM -0500, Corey Osgood wrote:
> >> -	/* 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.

Yep. But Joe is correct, the code probably needs fixing.


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

Yes, the IDs are correct, I think.

When there are multiple ICH* southbridges with the same PCI device IDs,
I always used the "first" one, but you could really use any of them.

Example:

#define PCI_DEVICE_ID_INTEL_82801FB_IDE         0x266f
#define PCI_DEVICE_ID_INTEL_82801FR_IDE         0x266f
#define PCI_DEVICE_ID_INTEL_82801FW_IDE         0x266f
#define PCI_DEVICE_ID_INTEL_82801FRW_IDE        0x266f
#define PCI_DEVICE_ID_INTEL_82801FBM_IDE        0x266f

It doesn't matter which of the above you use, they're all the same.

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

That, _and_ the code probably needs fixing to work with all the
different SATA devices on different ICH* southbridges. I haven't yet
checked which differences (if any) there are, though.

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

Yep.


> It should probably be somehow fixed to only build
> for SATA-supporting chipsets, but alas the world isn't perfect ;)

Yeah, we could do that, but it's not really high on the (my) priority
list, as the overhead is minimal. If I'm not mistaken such a struct
contains two 16bit values and a pointer. We have _way_ more critical
stuff to care about than this, IHMO.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20071108/b72f5c37/attachment.sig>