On Fri, Sep 28, 2012 at 10:19 AM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote: > On Fri, Sep 28, 2012 at 9:07 AM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: >> On Thu, Sep 27, 2012 at 2:17 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote: >> Today we have this, which is more complicated than it should be. Note >> how we do some ACPI stuff, some PCI stuff, some more ACPI stuff, then >> more PCI stuff: >> >> acpi_pci_root_add >> pci_acpi_scan_root >> pci_scan_child_bus >> acpi_pci_irq_add_prt >> acpi_pci_osc_control_set >> acpi_pci_root_start >> pci_bus_add_devices >> >> I don't think the ACPI/PCI mixture is anything essential dictated by >> the way the hardware or firmware works. I think it's just an artifact >> of the current design, and it could be changed. It would be better to >> have this: >> >> acpi_pci_root_add >> acpi_pci_irq_add_prt >> acpi_pci_osc_control_set >> pci_acpi_scan_root >> pci_scan_root_bus >> pci_scan_child_bus >> pci_bus_add_devices >> >> We can't get to this latter strategy as long as the ACPI interfaces >> depend on the struct pci_bus. So the _PRT change is a small thing in >> itself, but I do think it helps enable significant improvements in the >> future. > > still to handle to those fallback path like create_bus and scan bus failure. > > in my for-pci-next branch, with Jiang's patches and mine, now we achieved at > > acpi_pci_root_add > pci_acpi_scan_root > pci_scan_root_bus > pci_scan_child_bus > acpi_pci_osc_control_set > pci_bus_add_devices > > acpi_pci_irq_add_prt is called later during acpi binding that is > triggered by adding to device tree. > thought os_control set via pci_host_bridge add interface.. > > with those BUS ADD notification, we can pass bus safely, and without > considering about cleanup PRT and OSC setting. I haven't looked at those patches yet. Is there a reason why acpi_pci_osc_control_set() needs to be done after pci_scan_child_bus()? The argument that it might make the error path somewhat simpler is not very convincing to me. Having the arch code call both pci_scan_child_bus() and pci_bus_add_devices() is a much more fundamental complexity -- it makes x86 and ia64 different from many architectures, and it exposes the intermediate state where "devices have been enumerated but not added" to a lot more code. It doesn't sound like an improvement to call acpi_pci_irq_add_prt() using a bus add notifier. At least for the host bridge case, it's clear, simple, and straightforward to call it in acpi_pci_root_add(). Notifiers are useful in some cases, but they definitely make the code harder to follow. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
