Messages in this thread Patch in this message | | | Date | Tue, 14 Feb 2006 16:23:57 +0000 | From | Ralf Baechle <> | Subject | Re: PCI probe leaves master abort disabled in PCI_BRIDGE_CONTROL |
| |
On Mon, Feb 13, 2006 at 09:17:00PM -0800, Greg KH wrote: > Date: Mon, 13 Feb 2006 21:17:00 -0800 > From: Greg KH <gregkh@suse.de> > To: "David S. Miller" <davem@davemloft.net> > Cc: linux-kernel@vger.kernel.org, ralf@linux-mips.org, > linux-pci@atrey.karlin.mff.cuni.cz > Subject: Re: PCI probe leaves master abort disabled in PCI_BRIDGE_CONTROL > Content-Type: text/plain; charset=us-ascii > > On Mon, Feb 13, 2006 at 05:13:21PM -0800, David S. Miller wrote: > > > > In drivers/pci/probe.c:pci_scan_bridge(), if this is not the first > > pass (pass != 0) we don't restore the PCI_BRIDGE_CONTROL_REGISTER and > > thus leave PCI_BRIDGE_CTL_MASTER_ABORT off: > > > > int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev * dev, int max, int pass) > > { > > ... > > /* Disable MasterAbortMode during probing to avoid reporting > > of bus errors (in some architectures) */ > > pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &bctl); > > pci_write_config_word(dev, PCI_BRIDGE_CONTROL, > > bctl & ~PCI_BRIDGE_CTL_MASTER_ABORT); > > ... > > if ((buses & 0xffff00) && !pcibios_assign_all_busses() && !is_cardbus) { > > unsigned int cmax, busnr; > > /* > > * Bus already configured by firmware, process it in the first > > * pass and just note the configuration. > > */ > > if (pass) > > return max; > > ... > > } > > > > pci_write_config_word(dev, PCI_BRIDGE_CONTROL, bctl); > > ... > > > > This doesn't seem intentional. > > No it doesn't. Ralf added that code, way back in 2.6.2-rc2, with a > patch description of "[PATCH] PCI: fix probing for some mips systems" > > David, does this break some stuff on your boxes? > > Ralf, any thoughts? It was way back in January of 2004 that this change > went in.
Agreed, looks like an accident. The patch [1] originally came from Kip Walker (Broadcom back then) between 2.6.0-test3 and 2.6.0-test4. As I recall it was supposed to fix an issue with with PCI aborts being signalled by the PCI bridge of the Broadcom BCM1250 family of SOCs when probing behind pci_scan_bridge. It is undeseriable to disable PCI_BRIDGE_CTL_MASTER_ABORT in pci_{read,write)_config_* and the behaviour wasn't considered a bug in need of a workaround, so this was put in probe.c.
I don't have an affected system at hand, so can't really test but I propose something like the below patch.
Ralf
[1] http://www.linux-mips.org/git?p=linux.git;a=commit;h=599457e0cb702a31a3247ea6a5d9c6c99c4cf195
[PCI] Avoid leaving MASTER_ABORT disabled permanently when returning from pci_scan_bridge.
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index adfad4f..6de7c59 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -456,7 +456,7 @@ int __devinit pci_scan_bridge(struct pci * pass and just note the configuration. */ if (pass) - return max; + goto out; busnr = (buses >> 8) & 0xFF; /* @@ -466,12 +466,12 @@ int __devinit pci_scan_bridge(struct pci if (pci_find_bus(pci_domain_nr(bus), busnr)) { printk(KERN_INFO "PCI: Bus %04x:%02x already known\n", pci_domain_nr(bus), busnr); - return max; + goto out; } child = pci_add_new_bus(bus, dev, busnr); if (!child) - return max; + goto out; child->primary = buses & 0xFF; child->subordinate = (buses >> 16) & 0xFF; child->bridge_ctl = bctl; @@ -496,7 +496,7 @@ int __devinit pci_scan_bridge(struct pci bus ranges. */ pci_write_config_dword(dev, PCI_PRIMARY_BUS, buses & ~0xffffff); - return max; + goto out; } /* Clear errors */ @@ -505,7 +505,7 @@ int __devinit pci_scan_bridge(struct pci /* Prevent assigning a bus number that already exists. * This can happen when a bridge is hot-plugged */ if (pci_find_bus(pci_domain_nr(bus), max+1)) - return max; + goto out; child = pci_add_new_bus(bus, dev, ++max); buses = (buses & 0xff000000) | ((unsigned int)(child->primary) << 0) @@ -576,8 +576,6 @@ int __devinit pci_scan_bridge(struct pci pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max); } - pci_write_config_word(dev, PCI_BRIDGE_CONTROL, bctl); - sprintf(child->name, (is_cardbus ? "PCI CardBus #%02x" : "PCI Bus #%02x"), child->number); while (bus->parent) { @@ -596,6 +594,9 @@ int __devinit pci_scan_bridge(struct pci bus = bus->parent; } +out: + pci_write_config_word(dev, PCI_BRIDGE_CONTROL, bctl); + return max; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |