On Wed, Apr 27, 2016 at 06:31:29PM +0100, Lorenzo Pieralisi wrote: > On Wed, Apr 27, 2016 at 11:44:53AM -0500, Bjorn Helgaas wrote: > > On Wed, Apr 27, 2016 at 12:17:58PM +0100, Lorenzo Pieralisi wrote: > > > On Tue, Apr 26, 2016 at 09:26:49PM -0500, Bjorn Helgaas wrote: > > > > On Fri, Apr 15, 2016 at 07:06:37PM +0200, Tomasz Nowicki wrote: > > Today we call pci_bus_assign_domain_nr() from the PCI core (from > > pci_create_root_bus()). This is only implemented for > > PCI_DOMAINS_GENERIC, but even so, it fiddles around to figure out > > whether to get the domain from DT or to assign a new one. > > > > That seems backwards to me. The host bridge drivers already know > > where the domain should come from (ACPI _SEG, DT, etc.) and in the > > long term, I think they should be responsible for looking up or > > assigning a domain number *before* they call pci_create_root_bus(). > > Yes, the question still is how pci_create_root_bus() can get that > value (I am pretty certain this was heavily debated in the past, which > does not mean we can't give it another try). Right, we don't have a good mechanism for passing more info into pci_create_root_bus(). Maybe the caller could fill in a struct so we have a chance to extend it without having to change all the existing callers. I wonder if there's a design pattern we can copy, e.g., would something like the scsi_host_alloc(), scsi_add_host(), scsi_scan_host() model work here? > > > > > +void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) > > > > > +{ > > > > > + bus->domain_nr = acpi_disabled ? of_pci_bus_domain_nr(parent) : > > > > > + acpi_pci_bus_domain_nr(parent); > > > > We have the pci_bus * here, so to_pci_host_bridge(bus->bridge) gives > > us the struct pci_host_bridge. I can't remember why we put domain_nr > > in the struct pci_bus instead of in the struct pci_host_bridge. It > > seems like pci_host_bridge is the more logical place for it, because > > every bus below the host bridge must have the same domain by > > definition. > > > > Would it be feasible to either (a) move domain_nr to the > > pci_host_bridge, or (b) change acpi_pci_bus_domain_nr() so it uses the > > struct pci_bus * or the struct device * to find the struct > > acpi_pci_root where segment has already been stored by > > acpi_pci_root_add()? > > (b) is what JC implemented even though it works differently for > different hosts since it all depends on what's in bus->sysdata. > > It can certainly be done in a generic way (that works on X86 and IA64 > too), let's give it more thought. > > > Another wrinkle is the quirk added by 1f09b09b4de0 ("x86/PCI: Ignore > > _SEG on HP xw9300"). x86 doesn't use PCI_DOMAINS_GENERIC yet, so this > > patch wouldn't break it, but I hope x86 can use PCI_DOMAINS_GENERIC in > > the future, and then it will be a problem if we evaluate _SEG again. > > Yes, I share your concern here and I thought about that, if that's the > end goal let's find a solution that works across arches (or we temporarily > use JC's code and we then generalize it). I would ultimately like all arches to use PCI_DOMAINS_GENERIC, because I don't think there's anything intrisically arch-specific about where we store the domain number. The means of discovering or assigning a domain number might be arch-specific, but I think it would be cleanest if the host bridge driver handled that. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html