On Fri, Sep 28, 2012 at 10:19 AM, Yinghai Lu [off-list ref] wrote:

On Fri, Sep 28, 2012 at 9:07 AM, Bjorn Helgaas [off-list ref] wrote:

quoted

On Thu, Sep 27, 2012 at 2:17 PM, Yinghai Lu [off-list ref] 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.