Re: [PATCH 01/15] PCI, acpiphp: Separate out hot-add support of pci host bridge

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Friday, December 07, 2012 12:32:46 PM Bjorn Helgaas wrote:
> On Thu, Dec 6, 2012 at 11:25 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
> > From: Yinghai Lu <yinghai@kernel.org>
> >
> > It causes confusion.
> 
> I completely agree that acpiphp causes confusion  :)
> 
> > We may only need acpi hp for pci host bridge.
> >
> > Split host bridge hot-add support to pci_root_hp, and keep acpiphp simple.
> >
> > -v2: put back pci_root_hp change in one patch
> > -v3: add pcibios_resource_survey_bus() calling
> > -v4: remove not needed code with remove_bridge
> > -v5: put back support for acpiphp support for slots just on root bus.
> > -v6: change some functions to *_p2p_* to make it more clean.
> > -v7: split hot_added change out.
> >
> > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> > Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
> > ---
> >
> >  drivers/acpi/Makefile              |    1
> >  drivers/acpi/pci_root_hp.c         |  228 ++++++++++++++++++++++++++++++++++++
> >  drivers/pci/hotplug/acpiphp_glue.c |   59 ++-------
> >  3 files changed, 244 insertions(+), 44 deletions(-)
> >  create mode 100644 drivers/acpi/pci_root_hp.c
> >
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index 82422fe..4edfe41 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -36,6 +36,7 @@ acpi-y                                += processor_core.o
> >  acpi-y                         += ec.o
> >  acpi-$(CONFIG_ACPI_DOCK)       += dock.o
> >  acpi-y                         += pci_root.o pci_link.o pci_irq.o pci_bind.o
> > +acpi-$(CONFIG_HOTPLUG)         += pci_root_hp.o
> 
> I absolutely, 110% agree with splitting out the host bridge hotplug
> code from the PCI device hotplug code.
> 
> But I don't want to put this in a separate file; I think it should go
> directly in pci_root.c.  Putting it in a separate file gives the
> illusion that hotplug is something we only do in unusual
> circumstances.  But that's wrong -- even at boot-time we should be
> using the same hot-plug flow as we do later.
> 
> Plus, I want people to be forced to look at the ugliness of this code
> every time they look at pci_root.c.  Maybe that will help get it
> cleaned up eventually.
> 
> For example, as soon as you put this code in pci_root.c instead of
> pci_root_hp.c, it becomes obvious that we're keeping a list of host
> bridges in both places, and we probably don't need two lists.  And it
> seems dubious that acpi_pci_root_hp_init() is an initcall that walks
> the namespace looking for host bridges, when acpi_pci_root_add()
> already exists and is called for every host bridge.

Agreed.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.