i450nx-scanning-fix.patch

i450nx-scanning-fix.patch

[Andrew Morton]

I have had this in -mm for some time.  It works on my 450nx-based 4-way. 
Was planning on merging it.  Speak now or forever hold your pieces.



From: Matthew Wilcox <willy@debian.org>

[moved from linux-scsi to linux-pci since this really isn't scsi-related]

On Sun, Nov 09, 2003 at 11:51:36AM -0500, Doug Ledford wrote:
> I can tell you what's going on here.  This is a 450NX based
> motherboard.  The 450NX chipset from Intel was the first chipset to have
> peer PCI busses.  For backwards compatibility, some machine makers
> hacked their PCI BIOS to have a fake bridge device on PCI bus 0 that
> points to the same bus number as the peer bus.  This way if the OS
> didn't know about the peer bus registers it would still find the devices
> by scanning behind the bridge.

Ah, thanks Doug.  That saved me a few hours of debugging and
head-scratching ;-)

> In this case we are scanning behind this
> fake bridge and then also scanning based upon the peer bus registers in
> the chipset, and as a result we are finding the device twice.  In order
> to fix this problem you need to change the peer bus quirk code for the
> 450NX chipset to scan the list of bus 0 devices looking for a bridge
> that has the same config as the peer bus registers and if so delete the
> bridge from the list.  That will avoid double scanning and will avoid
> having the PCI code try and configure sub busses via a fake bridge when
> it should do all configurations via the 450NX peer bus registers.

I agree.  I considered some other possibilities like having
pci_scan_bridge() check for duplicate busses and return if it detects
them, but that would leave some pcipci bridges in the list that really
don't exist.

Here's a patch that compiles ... comments?




 arch/i386/pci/fixup.c |   33 +++++++++++++++++++++++++++++----
 1 files changed, 29 insertions(+), 4 deletions(-)

diff -puN arch/i386/pci/fixup.c~i450nx-scanning-fix arch/i386/pci/fixup.c
--- 25/arch/i386/pci/fixup.c~i450nx-scanning-fix	2003-11-09 16:58:55.000000000 -0800
+++ 25-akpm/arch/i386/pci/fixup.c	2003-11-09 16:58:55.000000000 -0800
@@ -6,27 +6,52 @@
 #include <linux/init.h>
 #include "pci.h"
 
+static void __devinit i450nx_scan_bus(struct pci_bus *parent, u8 busnr)
+{
+	struct list_head *tmp;
+
+	pci_scan_bus(busnr, &pci_root_ops, NULL);
+
+	list_for_each(tmp, &parent->children) {
+		u8 childnr;
+		struct pci_dev *dev = pci_dev_b(tmp);
+
+		if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
+			continue;
+		pci_read_config_byte(dev, PCI_PRIMARY_BUS, &childnr);
+		if (childnr != busnr)
+			continue;
+
+		printk(KERN_WARNING "PCI: Removing fake PCI bridge %s\n",
+				pci_name(dev));
+		pci_remove_bus_device(dev);
+		break;
+	}
+}
 
 static void __devinit pci_fixup_i450nx(struct pci_dev *d)
 {
 	/*
 	 * i450NX -- Find and scan all secondary buses on all PXB's.
+	 * Some manufacturers added fake PCI-PCI bridges that also point
+	 * to the peer busses.  Look for them and delete them.
 	 */
 	int pxb, reg;
 	u8 busno, suba, subb;
 
-	printk(KERN_WARNING "PCI: Searching for i450NX host bridges on %s\n", pci_name(d));
+	printk(KERN_NOTICE "PCI: Searching for i450NX host bridges on %s\n", pci_name(d));
 	reg = 0xd0;
-	for(pxb=0; pxb<2; pxb++) {
+	for (pxb = 0; pxb < 2; pxb++) {
 		pci_read_config_byte(d, reg++, &busno);
 		pci_read_config_byte(d, reg++, &suba);
 		pci_read_config_byte(d, reg++, &subb);
 		DBG("i450NX PXB %d: %02x/%02x/%02x\n", pxb, busno, suba, subb);
 		if (busno)
-			pci_scan_bus(busno, &pci_root_ops, NULL);	/* Bus A */
+			i450nx_scan_bus(d->bus, busno);		/* Bus A */
 		if (suba < subb)
-			pci_scan_bus(suba+1, &pci_root_ops, NULL);	/* Bus B */
+			i450nx_scan_bus(d->bus, suba+1);	/* Bus B */
 	}
+
 	pcibios_last_bus = -1;
 }
 

_

Re: i450nx-scanning-fix.patch

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

On Mon, 2003-12-29 at 12:02, Andrew Morton wrote:
> I have had this in -mm for some time.  It works on my 450nx-based 4-way. 
> Was planning on merging it.  Speak now or forever hold your pieces.

the one thing I can say is that I had a "fix" for this in the RH kernel
for a while, but it broke with newer bioses. End result was that there
was a working kernel for either old or for new bioses; the existing
linux worked well for newer bioses so I just dropped the patch entirely.
(Several machines only come with newer bioses and most people with old
bioses had updated anyway, probably for y2k issues :)


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: i450nx-scanning-fix.patch

[Andrew Morton]

Arjan van de Ven <arjanv@redhat.com> wrote:
>
> On Mon, 2003-12-29 at 12:02, Andrew Morton wrote:
> > I have had this in -mm for some time.  It works on my 450nx-based 4-way. 
> > Was planning on merging it.  Speak now or forever hold your pieces.
> 
> the one thing I can say is that I had a "fix" for this in the RH kernel
> for a while, but it broke with newer bioses. End result was that there
> was a working kernel for either old or for new bioses; the existing
> linux worked well for newer bioses so I just dropped the patch entirely.
> (Several machines only come with newer bioses and most people with old
> bioses had updated anyway, probably for y2k issues :)
> 

Thus leaving me even-more-confused-than-usual.  Ho hum.

Was your fix equivalent to this patch?

Re: i450nx-scanning-fix.patch

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]


> 
> Thus leaving me even-more-confused-than-usual.  Ho hum.
> 
> Was your fix equivalent to this patch?

not fully (looking waay back into kernel rpms ....)... the one I had was
a bit simpler in concept so this one might actually work better.

Also the problem wasn't "it doesn't work at all" it was "this ONE pci
slot isn't visible" which is why it going wrong escaped QA.... 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: i450nx-scanning-fix.patch

[Matthew Wilcox]

On Mon, Dec 29, 2003 at 03:02:35AM -0800, Andrew Morton wrote:
> 
> I have had this in -mm for some time.  It works on my 450nx-based 4-way. 
> Was planning on merging it.  Speak now or forever hold your pieces.

Well, I made the same mistake in this patch as I made in a later patch, so
I know it never worked for anyone.  Namely, pci_bus->children is the list
of child busses, not the list of devices on the bus.  That's ->devices.
I swear I'm going to write a for_each_pci_device_on_this_bus() macro so
I don't make the same mistake a third time.

>  arch/i386/pci/fixup.c |   33 +++++++++++++++++++++++++++++----
>  1 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff -puN arch/i386/pci/fixup.c~i450nx-scanning-fix arch/i386/pci/fixup.c
> --- 25/arch/i386/pci/fixup.c~i450nx-scanning-fix	2003-11-09 16:58:55.000000000 -0800
> +++ 25-akpm/arch/i386/pci/fixup.c	2003-11-09 16:58:55.000000000 -0800
> @@ -6,27 +6,52 @@
>  #include <linux/init.h>
>  #include "pci.h"
>  
> +static void __devinit i450nx_scan_bus(struct pci_bus *parent, u8 busnr)
> +{
> +	struct list_head *tmp;
> +
> +	pci_scan_bus(busnr, &pci_root_ops, NULL);
> +
> +	list_for_each(tmp, &parent->children) {
> +		u8 childnr;
> +		struct pci_dev *dev = pci_dev_b(tmp);
> +
> +		if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> +			continue;
> +		pci_read_config_byte(dev, PCI_PRIMARY_BUS, &childnr);
> +		if (childnr != busnr)
> +			continue;
> +
> +		printk(KERN_WARNING "PCI: Removing fake PCI bridge %s\n",
> +				pci_name(dev));
> +		pci_remove_bus_device(dev);
> +		break;
> +	}
> +}
>  
>  static void __devinit pci_fixup_i450nx(struct pci_dev *d)
>  {
>  	/*
>  	 * i450NX -- Find and scan all secondary buses on all PXB's.
> +	 * Some manufacturers added fake PCI-PCI bridges that also point
> +	 * to the peer busses.  Look for them and delete them.
>  	 */
>  	int pxb, reg;
>  	u8 busno, suba, subb;
>  
> -	printk(KERN_WARNING "PCI: Searching for i450NX host bridges on %s\n", pci_name(d));
> +	printk(KERN_NOTICE "PCI: Searching for i450NX host bridges on %s\n", pci_name(d));
>  	reg = 0xd0;
> -	for(pxb=0; pxb<2; pxb++) {
> +	for (pxb = 0; pxb < 2; pxb++) {
>  		pci_read_config_byte(d, reg++, &busno);
>  		pci_read_config_byte(d, reg++, &suba);
>  		pci_read_config_byte(d, reg++, &subb);
>  		DBG("i450NX PXB %d: %02x/%02x/%02x\n", pxb, busno, suba, subb);
>  		if (busno)
> -			pci_scan_bus(busno, &pci_root_ops, NULL);	/* Bus A */
> +			i450nx_scan_bus(d->bus, busno);		/* Bus A */
>  		if (suba < subb)
> -			pci_scan_bus(suba+1, &pci_root_ops, NULL);	/* Bus B */
> +			i450nx_scan_bus(d->bus, suba+1);	/* Bus B */
>  	}
> +
>  	pcibios_last_bus = -1;
>  }
>  
> 
> _
> 

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: i450nx-scanning-fix.patch

[Grant Grundler]

On Mon, Dec 29, 2003 at 12:28:09PM +0000, Matthew Wilcox wrote:
> Well, I made the same mistake in this patch as I made in a later patch, so
> I know it never worked for anyone.  Namely, pci_bus->children is the list
> of child busses, not the list of devices on the bus.  That's ->devices.

Yeah - it's not obvious unless one looks at the types carefully.

> I swear I'm going to write a for_each_pci_device_on_this_bus() macro so
> I don't make the same mistake a third time.

Can we rename "children" to "secondary_busses" or something like that?

grant