Re: [PATCH] 2.6 SGI Altix I/O code reorganization

From: Grant Grundler
Date: Wed Oct 06 2004 - 13:59:58 EST


On Wed, Oct 06, 2004 at 10:32:23AM -0500, Patrick Gefre wrote:
> o added our own pci_ops (Grant/Matthew's request)

Sorry - my bad.
I confused the issue by claiming one should replace pci_root_ops.
It was one possibility but it's not an easy path to take.

Mathew explained replacing the raw_pci_ops pointer is the Right Thing
and I suspect it's easier to properly implement.

Some comments on the implementation:
o sn_pci_fixup_bus() is a confusing name. "pcibios_fixup_bus" is normally
called by generic PCI code after each bus is walked.
This code obviously doesn't support that.
Maybe, sn_init_pci_controller() or something like that would be clearer.

o This bit of code belongs in the pcibios_fixup_bus() call path:
+ /*
+ * Generic Linux PCI Layer has created the pci_bus and pci_dev
+ * structures - time for us to add our SN PLatform specific
+ * information.
+ */
+
+ while ((pci_dev =
+ pci_find_device(PCI_ANY_ID, PCI_ANY_ID, pci_dev)) != NULL) {
+ sn_pci_fixup_slot(pci_dev);
+ }

I realize that's not easy to add/maintain in the arch/ia64 port though
since pcibios_fixup_bus() is common code for multiple platforms.

o sn_pci_fixup_bus() should be called for each PCI root bus controller
the firmware advertises. The loop in sn_pci_init() is hard coded
to loop from 0 to 256 busses.
Is ACPI the only way PCI host controllers are advertised?
SN2 doesn't use a different method today?

It means we are telling PCI subsystem to walk root busses that don't
exist in all configurations. I hope there are no nasty side effects
from that.

o the BUG() in:

+ controller = sn_alloc_pci_sysdata();
+ if (!controller) {
+ BUG();
+ }
is redundant with the BUG in sn_alloc_pci_sysdata().

sorry for the initial bad advice and I hope this helps,
grant
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/