From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bjorn Helgaas Subject: Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function Date: Fri, 28 Sep 2012 13:44:06 -0600 Message-ID: References: <20120918151215.59ea763b.izumi.taku@jp.fujitsu.com> <20120918152553.8c8390d8.izumi.taku@jp.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Return-path: In-Reply-To: Sender: linux-pci-owner@vger.kernel.org To: Yinghai Lu Cc: Taku Izumi , linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org, kaneshige.kenji@jp.fujitsu.com, jiang.liu@huawei.com List-Id: linux-acpi@vger.kernel.org On Fri, Sep 28, 2012 at 10:19 AM, Yinghai Lu wrote: > On Fri, Sep 28, 2012 at 9:07 AM, Bjorn Helgaas wrote: >> On Thu, Sep 27, 2012 at 2:17 PM, Yinghai Lu 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.