| Message ID | 20120928184627.0b039d14.izumi.taku@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
I think I'm missing patches 7/8 and 8/8 from this series. Can you repost them to make sure I have the latest versions? Note the comments below: On Fri, Sep 28, 2012 at 06:46:27PM +0900, Taku Izumi wrote: > > Currently there's no PCI-related clean-up code > in acpi_pci_root_remove() function. > This patch introduces function for hostbridge removal, > and brings back pci_stop_bus_devices() function. > > diff: rebased against current next > updated according to Bjorn's comment > > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> > --- > drivers/acpi/pci_bind.c | 7 +++++++ > drivers/acpi/pci_root.c | 6 ++++++ > drivers/pci/remove.c | 28 ++++++++++++++++++++++++++++ > include/acpi/acpi_drivers.h | 1 + > include/linux/pci.h | 2 ++ > 5 files changed, 44 insertions(+) > > Index: Bjorn-next-0925-configchange/drivers/pci/remove.c > =================================================================== > --- Bjorn-next-0925-configchange.orig/drivers/pci/remove.c > +++ Bjorn-next-0925-configchange/drivers/pci/remove.c > @@ -111,3 +111,31 @@ void pci_stop_and_remove_bus_device(stru > pci_remove_bus_device(dev); > } > EXPORT_SYMBOL(pci_stop_and_remove_bus_device); > + > +void pci_stop_bus_devices(struct pci_bus *bus) > +{ > + struct pci_dev *dev, *tmp; > + > + list_for_each_entry_safe_reverse(dev, tmp, > + &bus->devices, bus_list) { > + pci_stop_bus_device(dev); > + } > + > +} > +EXPORT_SYMBOL(pci_stop_bus_devices); I'm hesitant to introduce pci_stop_bus_devices() again, particularly when it is exported. The stop/remove split introduces the state where devices are "stopped" but haven't been "removed" yet. In this state, the driver .remove() method has been called, sysfs has been cleaned up, and the struct device has been unregistered, but the struct pci_dev itself still exists. Obviously, this state *must* exist internally in the PCI core as we remove the PCI device. The problem is that we have non-core code that *depends* on being run while in this transitory state. I think this is a design mistake. The code that depends on this state is basically just the stuff in the acpi_pci_drivers list, namely, acpi_pci_hp_driver and acpi_pci_slot_driver. I suspect that the main reason we have the acpi_pci_drivers list and the whole acpi_pci_register_driver() infrastructure is so that these PCI host bridge "sub-drivers" can be built as modules. I don't think there's really any value in having these sub-drivers as modules, and it leads to a lot of complication in the code. I'm pretty sure that forcing them to be selected at build-time will let us make things much simpler. If we have to have pci_stop_bus_devices() as an interim measure, I can live with it, but it doesn't need to be exported, does it? > +void pci_remove_host_bridge(struct pci_host_bridge *bridge) > +{ > + struct pci_bus *root = bridge->bus; > + struct pci_dev *dev, *tmp; > + > + list_for_each_entry_safe_reverse(dev, tmp, > + &root->devices, bus_list) { > + pci_remove_bus_device(dev); > + } > + > + pci_remove_bus(root); > + > + device_unregister(&bridge->dev); > +} > +EXPORT_SYMBOL(pci_remove_host_bridge); > Index: Bjorn-next-0925-configchange/drivers/acpi/pci_root.c > =================================================================== > --- Bjorn-next-0925-configchange.orig/drivers/acpi/pci_root.c > +++ Bjorn-next-0925-configchange/drivers/acpi/pci_root.c > @@ -659,8 +659,10 @@ static int acpi_pci_root_remove(struct a > { > struct acpi_pci_root *root = acpi_driver_data(device); > struct acpi_pci_driver *driver; > + struct pci_host_bridge *bridge = to_pci_host_bridge(root->bus->bridge); > > mutex_lock(&acpi_pci_root_lock); > + pci_stop_bus_devices(root->bus); > list_for_each_entry(driver, &acpi_pci_drivers, node) > if (driver->remove) > driver->remove(root); > @@ -668,6 +670,10 @@ static int acpi_pci_root_remove(struct a > device_set_run_wake(root->bus->bridge, false); > pci_acpi_remove_bus_pm_notifier(device); > > + acpi_pci_unbind_root(device); > + > + pci_remove_host_bridge(bridge); > + > list_del(&root->node); > mutex_unlock(&acpi_pci_root_lock); > kfree(root); > Index: Bjorn-next-0925-configchange/include/linux/pci.h > =================================================================== > --- Bjorn-next-0925-configchange.orig/include/linux/pci.h > +++ Bjorn-next-0925-configchange/include/linux/pci.h > @@ -734,6 +734,8 @@ extern struct pci_dev *pci_dev_get(struc > extern void pci_dev_put(struct pci_dev *dev); > extern void pci_remove_bus(struct pci_bus *b); > extern void pci_stop_and_remove_bus_device(struct pci_dev *dev); > +extern void pci_stop_bus_devices(struct pci_bus *bus); > +extern void pci_remove_host_bridge(struct pci_host_bridge *bridge); > void pci_setup_cardbus(struct pci_bus *bus); > extern void pci_sort_breadthfirst(void); > #define dev_is_pci(d) ((d)->bus == &pci_bus_type) > Index: Bjorn-next-0925-configchange/drivers/acpi/pci_bind.c > =================================================================== > --- Bjorn-next-0925-configchange.orig/drivers/acpi/pci_bind.c > +++ Bjorn-next-0925-configchange/drivers/acpi/pci_bind.c > @@ -118,3 +118,10 @@ int acpi_pci_bind_root(struct acpi_devic > > return 0; > } > + > +void acpi_pci_unbind_root(struct acpi_device *device) > +{ > + device->ops.bind = NULL; > + device->ops.unbind = NULL; > +} > + > Index: Bjorn-next-0925-configchange/include/acpi/acpi_drivers.h > =================================================================== > --- Bjorn-next-0925-configchange.orig/include/acpi/acpi_drivers.h > +++ Bjorn-next-0925-configchange/include/acpi/acpi_drivers.h > @@ -101,6 +101,7 @@ struct pci_bus; > > struct pci_dev *acpi_get_pci_dev(acpi_handle); > int acpi_pci_bind_root(struct acpi_device *device); > +void acpi_pci_unbind_root(struct acpi_device *device); > > /* Arch-defined function to add a bus to the system */ > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Subject: [PATCH resend 0/8] PCI, ACPI, x86: pci root bus hotplug support resources assign and remove path
1. add support for assign resource for hot add path.
2. stop and remove root bus during acpi root remove.
could get from
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-root-bus-hotplug
Yinghai Lu (8):
PCI: Separate out pci_assign_unassigned_bus_resources()
PCI: Move pci_rescan_bus() back to probe.c
PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
PCI, ACPI: assign unassigned resource for hot add root bus
PCI: Add pci_stop/remove_root_bus()
PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus
PCI, ACPI: delete root bus prt during hot remove path
PCI, ACPI: remove acpi_root_driver in reserse order
drivers/acpi/pci_root.c | 21 ++++++++++++++++++++-
drivers/pci/probe.c | 22 ++++++++++++++++++++++
drivers/pci/remove.c | 36 ++++++++++++++++++++++++++++++++++++
drivers/pci/setup-bus.c | 22 +---------------------
include/linux/pci.h | 3 +++
5 files changed, 82 insertions(+), 22 deletions(-)
Hi, On Tuesday, October 30, 2012 10:42:37 AM Yinghai Lu wrote: > Subject: [PATCH resend 0/8] PCI, ACPI, x86: pci root bus hotplug support resources assign and remove path > > 1. add support for assign resource for hot add path. > 2. stop and remove root bus during acpi root remove. > > > could get from > git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-root-bus-hotplug > > Yinghai Lu (8): > PCI: Separate out pci_assign_unassigned_bus_resources() > PCI: Move pci_rescan_bus() back to probe.c > PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res > PCI, ACPI: assign unassigned resource for hot add root bus > PCI: Add pci_stop/remove_root_bus() > PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus > PCI, ACPI: delete root bus prt during hot remove path > PCI, ACPI: remove acpi_root_driver in reserse order > > drivers/acpi/pci_root.c | 21 ++++++++++++++++++++- > drivers/pci/probe.c | 22 ++++++++++++++++++++++ > drivers/pci/remove.c | 36 ++++++++++++++++++++++++++++++++++++ > drivers/pci/setup-bus.c | 22 +--------------------- > include/linux/pci.h | 3 +++ > 5 files changed, 82 insertions(+), 22 deletions(-) Please feel free to add Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> to the ACPI-related patches in this series. Thanks, Rafael
For root bus hot add, fw could assign some resource for the devices for
that root bus before notifying os via acpi, we should check and use those
resources at first just like we do for booting path.
At first, we need to refactor x86 pci pcibios_allocate related functions
for booting path to take bus as parameter.
After that, we could use the survey function for hot add root bus.
based on pci/yinghai-for-pci-root-bus-hotplug
could get from
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-survey-resources
Yinghai Lu (8):
PCI, x86: Separate out pcibios_allocate_bridge_resources()
PCI, x86: Separate out pcibios_allocate_dev_resources()
PCI, x86: Let pcibios_allocate_bus_resources() take bus instead
PCI, x86: Separate out rom resource claim
PCI, x86: Add pcibios_fw_addr_done
PCI, x86: Remove __init for hw/fw allocated functions
PCI, x86: Claim FW allocated resources in hot add path.
PCI, ACPI: reserve fw allocated resource for hot added root bus
arch/x86/pci/i386.c | 185 +++++++++++++++++++++++++++++++----------------
drivers/acpi/pci_root.c | 4 +-
drivers/pci/bus.c | 2 +
include/linux/pci.h | 1 +
4 files changed, 127 insertions(+), 65 deletions(-)
On Thu, Nov 1, 2012 at 6:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > Hi, > > On Tuesday, October 30, 2012 10:42:37 AM Yinghai Lu wrote: >> Subject: [PATCH resend 0/8] PCI, ACPI, x86: pci root bus hotplug support resources assign and remove path >> >> 1. add support for assign resource for hot add path. >> 2. stop and remove root bus during acpi root remove. >> >> >> could get from >> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-root-bus-hotplug >> >> Yinghai Lu (8): >> PCI: Separate out pci_assign_unassigned_bus_resources() >> PCI: Move pci_rescan_bus() back to probe.c >> PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res >> PCI, ACPI: assign unassigned resource for hot add root bus >> PCI: Add pci_stop/remove_root_bus() >> PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus >> PCI, ACPI: delete root bus prt during hot remove path >> PCI, ACPI: remove acpi_root_driver in reserse order >> >> drivers/acpi/pci_root.c | 21 ++++++++++++++++++++- >> drivers/pci/probe.c | 22 ++++++++++++++++++++++ >> drivers/pci/remove.c | 36 ++++++++++++++++++++++++++++++++++++ >> drivers/pci/setup-bus.c | 22 +--------------------- >> include/linux/pci.h | 3 +++ >> 5 files changed, 82 insertions(+), 22 deletions(-) > > Please feel free to add > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > to the ACPI-related patches in this series. I applied these to my pci/yinghai-for-pci-root-bus-hotplug branch as v3.8 material. They should appear in "next" tomorrow. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 5, 2012 at 2:27 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > I applied these to my pci/yinghai-for-pci-root-bus-hotplug branch as > v3.8 material. They should appear in "next" tomorrow. Thanks! Thanks... please check batch 2 at https://patchwork.kernel.org/patch/1693211/ Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Reviewed and tested by Taku Izumi <izumi.taku@jp.fujitsu.com> > -----Original Message----- > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] On Behalf Of Yinghai Lu > Sent: Wednesday, October 31, 2012 2:43 AM > To: Bjorn Helgaas; Len Brown; Taku Izumi; Jiang Liu > Cc: linux-pci@vger.kernel.org; linux-acpi@vger.kernel.org; Yinghai Lu > Subject: > > Subject: [PATCH resend 0/8] PCI, ACPI, x86: pci root bus hotplug support resources assign and remove path > > 1. add support for assign resource for hot add path. > 2. stop and remove root bus during acpi root remove. > > > could get from > git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-root-bus-hotplug > > Yinghai Lu (8): > PCI: Separate out pci_assign_unassigned_bus_resources() > PCI: Move pci_rescan_bus() back to probe.c > PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res > PCI, ACPI: assign unassigned resource for hot add root bus > PCI: Add pci_stop/remove_root_bus() > PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus > PCI, ACPI: delete root bus prt during hot remove path > PCI, ACPI: remove acpi_root_driver in reserse order > > drivers/acpi/pci_root.c | 21 ++++++++++++++++++++- > drivers/pci/probe.c | 22 ++++++++++++++++++++++ > drivers/pci/remove.c | 36 ++++++++++++++++++++++++++++++++++++ > drivers/pci/setup-bus.c | 22 +--------------------- > include/linux/pci.h | 3 +++ > 5 files changed, 82 insertions(+), 22 deletions(-) > > -- > 1.7.7 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Nov 3, 2012 at 9:39 PM, Yinghai Lu <yinghai@kernel.org> wrote: > For root bus hot add, fw could assign some resource for the devices for > that root bus before notifying os via acpi, we should check and use those > resources at first just like we do for booting path. > > At first, we need to refactor x86 pci pcibios_allocate related functions > for booting path to take bus as parameter. > > After that, we could use the survey function for hot add root bus. > > based on pci/yinghai-for-pci-root-bus-hotplug > > could get from > git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-survey-resources > > Yinghai Lu (8): > PCI, x86: Separate out pcibios_allocate_bridge_resources() > PCI, x86: Separate out pcibios_allocate_dev_resources() > PCI, x86: Let pcibios_allocate_bus_resources() take bus instead > PCI, x86: Separate out rom resource claim > PCI, x86: Add pcibios_fw_addr_done > PCI, x86: Remove __init for hw/fw allocated functions > PCI, x86: Claim FW allocated resources in hot add path. > PCI, ACPI: reserve fw allocated resource for hot added root bus > > arch/x86/pci/i386.c | 185 +++++++++++++++++++++++++++++++---------------- > drivers/acpi/pci_root.c | 4 +- > drivers/pci/bus.c | 2 + > include/linux/pci.h | 1 + > 4 files changed, 127 insertions(+), 65 deletions(-) > Bjorn, Can you queue those 8 patches for v3.9 in pci tree? So I could resend out other pci root hotplug patches. Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[+cc David, Michal, Koichi, Ben, Paul] On Fri, Dec 7, 2012 at 12:15 AM, Yinghai Lu <yinghai@kernel.org> wrote: > On Sat, Nov 3, 2012 at 9:39 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> For root bus hot add, fw could assign some resource for the devices for >> that root bus before notifying os via acpi, we should check and use those >> resources at first just like we do for booting path. >> >> At first, we need to refactor x86 pci pcibios_allocate related functions >> for booting path to take bus as parameter. >> >> After that, we could use the survey function for hot add root bus. >> >> based on pci/yinghai-for-pci-root-bus-hotplug >> >> could get from >> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-survey-resources >> >> Yinghai Lu (8): >> PCI, x86: Separate out pcibios_allocate_bridge_resources() >> PCI, x86: Separate out pcibios_allocate_dev_resources() >> PCI, x86: Let pcibios_allocate_bus_resources() take bus instead >> PCI, x86: Separate out rom resource claim >> PCI, x86: Add pcibios_fw_addr_done >> PCI, x86: Remove __init for hw/fw allocated functions >> PCI, x86: Claim FW allocated resources in hot add path. >> PCI, ACPI: reserve fw allocated resource for hot added root bus >> >> arch/x86/pci/i386.c | 185 +++++++++++++++++++++++++++++++---------------- >> drivers/acpi/pci_root.c | 4 +- >> drivers/pci/bus.c | 2 + >> include/linux/pci.h | 1 + >> 4 files changed, 127 insertions(+), 65 deletions(-) >> > > Bjorn, > > Can you queue those 8 patches for v3.9 in pci tree? > > So I could resend out other pci root hotplug patches. I'm really sorry that it's taken me so long to get to these. I applied these to my pci/yinghai-survey-resources branch. I re-ordered the last two and reworked some of the changelogs. In general these look good. My main concern is that they only touch x86, without touching the similar code in frv, microblaze, mn10300, and powerpc. This code (pcibios_resource_survey(), pcibios_assign_resources(), pcibios_allocate_resources(), pcibios_allocate_bus_resources()) was obviously copied from x86 originally, and I'd like to preserve the similarity between them. It would be even better to refactor it so it's actually *shared*, but I don't think that's a requirement right now. If we allow it to diverge now, it will make it harder to refactor and harder to notice when bug fixes should be applied to all of them. For example, looking at pcibios_allocate_resources(), commit 575939cf5 added some SR-IOV support to x86. Should similar code be added for frv, microblaze, mn10300, and powerpc? Anybody else have thoughts on this? Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 7, 2013 at 4:49 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > [+cc David, Michal, Koichi, Ben, Paul] > > On Fri, Dec 7, 2012 at 12:15 AM, Yinghai Lu <yinghai@kernel.org> wrote: >> On Sat, Nov 3, 2012 at 9:39 PM, Yinghai Lu <yinghai@kernel.org> wrote: >>> For root bus hot add, fw could assign some resource for the devices for >>> that root bus before notifying os via acpi, we should check and use those >>> resources at first just like we do for booting path. >>> >>> At first, we need to refactor x86 pci pcibios_allocate related functions >>> for booting path to take bus as parameter. >>> >>> After that, we could use the survey function for hot add root bus. >>> >>> based on pci/yinghai-for-pci-root-bus-hotplug >>> >>> could get from >>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-survey-resources >>> >>> Yinghai Lu (8): >>> PCI, x86: Separate out pcibios_allocate_bridge_resources() >>> PCI, x86: Separate out pcibios_allocate_dev_resources() >>> PCI, x86: Let pcibios_allocate_bus_resources() take bus instead >>> PCI, x86: Separate out rom resource claim >>> PCI, x86: Add pcibios_fw_addr_done >>> PCI, x86: Remove __init for hw/fw allocated functions >>> PCI, x86: Claim FW allocated resources in hot add path. >>> PCI, ACPI: reserve fw allocated resource for hot added root bus >>> >>> arch/x86/pci/i386.c | 185 +++++++++++++++++++++++++++++++---------------- >>> drivers/acpi/pci_root.c | 4 +- >>> drivers/pci/bus.c | 2 + >>> include/linux/pci.h | 1 + >>> 4 files changed, 127 insertions(+), 65 deletions(-) >>> >> >> Bjorn, >> >> Can you queue those 8 patches for v3.9 in pci tree? >> >> So I could resend out other pci root hotplug patches. > > I'm really sorry that it's taken me so long to get to these. > > I applied these to my pci/yinghai-survey-resources branch. I > re-ordered the last two and reworked some of the changelogs. To be clear about this, the pci/yinghai-survey-resources branch I mentioned is a staging branch that just gets build test coverage. I don't plan to actually merge this or put it into -next until the questions below are resolved. My inclination, until I'm persuaded otherwise, is to wait for patches that preserve the similarities among these architectures. > In general these look good. My main concern is that they only touch > x86, without touching the similar code in frv, microblaze, mn10300, > and powerpc. > > This code (pcibios_resource_survey(), pcibios_assign_resources(), > pcibios_allocate_resources(), pcibios_allocate_bus_resources()) was > obviously copied from x86 originally, and I'd like to preserve the > similarity between them. It would be even better to refactor it so > it's actually *shared*, but I don't think that's a requirement right > now. > > If we allow it to diverge now, it will make it harder to refactor and > harder to notice when bug fixes should be applied to all of them. For > example, looking at pcibios_allocate_resources(), commit 575939cf5 > added some SR-IOV support to x86. Should similar code be added for > frv, microblaze, mn10300, and powerpc? > > Anybody else have thoughts on this? > > Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 8, 2013 at 9:57 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> I'm really sorry that it's taken me so long to get to these. >> >> I applied these to my pci/yinghai-survey-resources branch. I >> re-ordered the last two and reworked some of the changelogs. > > To be clear about this, the pci/yinghai-survey-resources branch I > mentioned is a staging branch that just gets build test coverage. I > don't plan to actually merge this or put it into -next until the > questions below are resolved. > > My inclination, until I'm persuaded otherwise, is to wait for patches > that preserve the similarities among these architectures. I don't know, that could be separated patcheset after we conclude pci root bus hotplug support. > >> In general these look good. My main concern is that they only touch >> x86, without touching the similar code in frv, microblaze, mn10300, >> and powerpc. >> >> This code (pcibios_resource_survey(), pcibios_assign_resources(), >> pcibios_allocate_resources(), pcibios_allocate_bus_resources()) was >> obviously copied from x86 originally, and I'd like to preserve the >> similarity between them. It would be even better to refactor it so >> it's actually *shared*, but I don't think that's a requirement right >> now. yes, should be moved to drivers/pci >> >> If we allow it to diverge now, it will make it harder to refactor and >> harder to notice when bug fixes should be applied to all of them. For >> example, looking at pcibios_allocate_resources(), commit 575939cf5 >> added some SR-IOV support to x86. Should similar code be added for >> frv, microblaze, mn10300, and powerpc? should be treated the same. >> >> Anybody else have thoughts on this? >> >> Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 8, 2013 at 11:27 AM, Yinghai Lu <yinghai@kernel.org> wrote: > On Tue, Jan 8, 2013 at 9:57 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>> I'm really sorry that it's taken me so long to get to these. >>> >>> I applied these to my pci/yinghai-survey-resources branch. I >>> re-ordered the last two and reworked some of the changelogs. >> >> To be clear about this, the pci/yinghai-survey-resources branch I >> mentioned is a staging branch that just gets build test coverage. I >> don't plan to actually merge this or put it into -next until the >> questions below are resolved. >> >> My inclination, until I'm persuaded otherwise, is to wait for patches >> that preserve the similarities among these architectures. > > I don't know, that could be separated patcheset after we conclude > pci root bus hotplug support. The main reason I review patches before merging them is to identify issues that we can fix before they affect everybody. It makes my life a lot easier if I don't have to keep track of pending unaddressed review comments. Is there an advantage to waiting and doing this work later? >>> In general these look good. My main concern is that they only touch >>> x86, without touching the similar code in frv, microblaze, mn10300, >>> and powerpc. >>> >>> This code (pcibios_resource_survey(), pcibios_assign_resources(), >>> pcibios_allocate_resources(), pcibios_allocate_bus_resources()) was >>> obviously copied from x86 originally, and I'd like to preserve the >>> similarity between them. It would be even better to refactor it so >>> it's actually *shared*, but I don't think that's a requirement right >>> now. > > yes, should be moved to drivers/pci > >>> >>> If we allow it to diverge now, it will make it harder to refactor and >>> harder to notice when bug fixes should be applied to all of them. For >>> example, looking at pcibios_allocate_resources(), commit 575939cf5 >>> added some SR-IOV support to x86. Should similar code be added for >>> frv, microblaze, mn10300, and powerpc? > > should be treated the same. > >>> >>> Anybody else have thoughts on this? >>> >>> Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 9, 2013 at 9:35 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> I don't know, that could be separated patcheset after we conclude >> pci root bus hotplug support. > > The main reason I review patches before merging them is to identify > issues that we can fix before they affect everybody. It makes my life > a lot easier if I don't have to keep track of pending unaddressed > review comments. Is there an advantage to waiting and doing this work > later? > >>>> In general these look good. My main concern is that they only touch >>>> x86, without touching the similar code in frv, microblaze, mn10300, >>>> and powerpc. the reason why we need to change those codes for x86, we want to make it support pci root bus hotplug. So it would be reasonable for us to align other platform to x86 changes after pci root bus hotplug change is completely done. Also I have for_dev_each_res patches rely on this patchset. After those done, we can move the code from x86 to drivers/pci and delete other arches code one by one. Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 9, 2013 at 10:53 AM, Yinghai Lu <yinghai@kernel.org> wrote: > On Wed, Jan 9, 2013 at 9:35 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>> I don't know, that could be separated patcheset after we conclude >>> pci root bus hotplug support. >> >> The main reason I review patches before merging them is to identify >> issues that we can fix before they affect everybody. It makes my life >> a lot easier if I don't have to keep track of pending unaddressed >> review comments. Is there an advantage to waiting and doing this work >> later? >> >>>>> In general these look good. My main concern is that they only touch >>>>> x86, without touching the similar code in frv, microblaze, mn10300, >>>>> and powerpc. > > the reason why we need to change those codes for x86, we want to make it support > pci root bus hotplug. So it would be reasonable for us to align other > platform to x86 > changes after pci root bus hotplug change is completely done. OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a way to keep track of this consistency issue and merged pci/yinghai-survey-resources to my -next branch. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 9, 2013 at 10:39 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> the reason why we need to change those codes for x86, we want to make it support >> pci root bus hotplug. So it would be reasonable for us to align other >> platform to x86 >> changes after pci root bus hotplug change is completely done. > > OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a > way to keep track of this consistency issue and merged > pci/yinghai-survey-resources to my -next branch. Thanks a lot. will send other pci root bus hotplug out. question: now Rafael's tree has acpi-scan branch and it touches pci-root.c. so is it ok for me to base patches on your pci/next and his pm/acpi-scan? how? can you two have some arrangement like you pulling Rafael's branch? Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, January 09, 2013 11:01:39 AM Yinghai Lu wrote: > On Wed, Jan 9, 2013 at 10:39 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > >> the reason why we need to change those codes for x86, we want to make it support > >> pci root bus hotplug. So it would be reasonable for us to align other > >> platform to x86 > >> changes after pci root bus hotplug change is completely done. > > > > OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a > > way to keep track of this consistency issue and merged > > pci/yinghai-survey-resources to my -next branch. > > Thanks a lot. will send other pci root bus hotplug out. > > question: now Rafael's tree has acpi-scan branch and it touches pci-root.c. > > so is it ok for me to base patches on your pci/next and his pm/acpi-scan? > how? > can you two have some arrangement like you pulling Rafael's branch? My acpi-scan branch is not going to be rebased going forward, so it can be pulled from safely if that helps. Thanks, Rafael
On Wed, 2013-01-09 at 11:39 -0700, Bjorn Helgaas wrote: > > > the reason why we need to change those codes for x86, we want to > make it support > > pci root bus hotplug. So it would be reasonable for us to align > other > > platform to x86 > > changes after pci root bus hotplug change is completely done. > > OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a > way to keep track of this consistency issue and merged > pci/yinghai-survey-resources to my -next branch. Hrm... that's going to be tricky. For example the SR-IOV stuff will not work for us, we are working on a different implementation, due to some of "interesting" bridge constraints etc... It would be nice to get more of the resource survey in common, our original code was actually completely different and I reconciled it a lot with x86 in the past few years, but I'm sure we can do better. However there are still going to be corner cases.... Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 9, 2013 at 1:10 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Wednesday, January 09, 2013 11:01:39 AM Yinghai Lu wrote: >> On Wed, Jan 9, 2013 at 10:39 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> >> the reason why we need to change those codes for x86, we want to make it support >> >> pci root bus hotplug. So it would be reasonable for us to align other >> >> platform to x86 >> >> changes after pci root bus hotplug change is completely done. >> > >> > OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a >> > way to keep track of this consistency issue and merged >> > pci/yinghai-survey-resources to my -next branch. >> >> Thanks a lot. will send other pci root bus hotplug out. >> >> question: now Rafael's tree has acpi-scan branch and it touches pci-root.c. >> >> so is it ok for me to base patches on your pci/next and his pm/acpi-scan? >> how? >> can you two have some arrangement like you pulling Rafael's branch? > > My acpi-scan branch is not going to be rebased going forward, so it can be > pulled from safely if that helps. I'm happy to do that, but it is outside the scope of my limited git experience. My guess is that I should do this (doing the pull into a branch which I later merge into my -next branch): $ git checkout -b pci/yinghai-survey-resources+acpi-scan pci/yinghai-survey-resources $ git pull --no-ff --log git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi-scan $ vi drivers/acpi/pci_root.c # resolve conflicts $ git add drivers/acpi/pci_root.c $ git commit $ git checkout next $ git merge --no-ff --log pci/yinghai-survey-resources+acpi-scan Is that reasonable? This won't cause issues when both Rafael and I ask Linus to pull from our trees later? Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, January 09, 2013 05:34:32 PM Bjorn Helgaas wrote: > On Wed, Jan 9, 2013 at 1:10 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Wednesday, January 09, 2013 11:01:39 AM Yinghai Lu wrote: > >> On Wed, Jan 9, 2013 at 10:39 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > >> >> the reason why we need to change those codes for x86, we want to make it support > >> >> pci root bus hotplug. So it would be reasonable for us to align other > >> >> platform to x86 > >> >> changes after pci root bus hotplug change is completely done. > >> > > >> > OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a > >> > way to keep track of this consistency issue and merged > >> > pci/yinghai-survey-resources to my -next branch. > >> > >> Thanks a lot. will send other pci root bus hotplug out. > >> > >> question: now Rafael's tree has acpi-scan branch and it touches pci-root.c. > >> > >> so is it ok for me to base patches on your pci/next and his pm/acpi-scan? > >> how? > >> can you two have some arrangement like you pulling Rafael's branch? > > > > My acpi-scan branch is not going to be rebased going forward, so it can be > > pulled from safely if that helps. > > I'm happy to do that, but it is outside the scope of my limited git > experience. My guess is that I should do this (doing the pull into a > branch which I later merge into my -next branch): > > $ git checkout -b pci/yinghai-survey-resources+acpi-scan > pci/yinghai-survey-resources > $ git pull --no-ff --log > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git > acpi-scan > $ vi drivers/acpi/pci_root.c # resolve conflicts > $ git add drivers/acpi/pci_root.c > $ git commit > > $ git checkout next > $ git merge --no-ff --log pci/yinghai-survey-resources+acpi-scan > > Is that reasonable? Yes, it looks reasonable. > This won't cause issues when both Rafael and I ask Linus to pull from our > trees later? No, it won't, as long as I don't rebase the original acpi-scan branch (which I'm not going to do) and you don't rebase your pci/yinghai-survey-resources+acpi-scan branch going forward. The pull makes your tree contain the same commits (i.e. commit IDs along with the data) that are in my acpi-scan branch, so when Linus merges them together, git will notice that the commits are the same. Thanks, Rafael
On Thu, Jan 10, 2013 at 6:07 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Wednesday, January 09, 2013 05:34:32 PM Bjorn Helgaas wrote: >> On Wed, Jan 9, 2013 at 1:10 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> > On Wednesday, January 09, 2013 11:01:39 AM Yinghai Lu wrote: >> >> On Wed, Jan 9, 2013 at 10:39 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> >> >> the reason why we need to change those codes for x86, we want to make it support >> >> >> pci root bus hotplug. So it would be reasonable for us to align other >> >> >> platform to x86 >> >> >> changes after pci root bus hotplug change is completely done. >> >> > >> >> > OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a >> >> > way to keep track of this consistency issue and merged >> >> > pci/yinghai-survey-resources to my -next branch. >> >> >> >> Thanks a lot. will send other pci root bus hotplug out. >> >> >> >> question: now Rafael's tree has acpi-scan branch and it touches pci-root.c. >> >> >> >> so is it ok for me to base patches on your pci/next and his pm/acpi-scan? >> >> how? >> >> can you two have some arrangement like you pulling Rafael's branch? >> > >> > My acpi-scan branch is not going to be rebased going forward, so it can be >> > pulled from safely if that helps. >> >> I'm happy to do that, but it is outside the scope of my limited git >> experience. My guess is that I should do this (doing the pull into a >> branch which I later merge into my -next branch): >> >> $ git checkout -b pci/yinghai-survey-resources+acpi-scan >> pci/yinghai-survey-resources >> $ git pull --no-ff --log >> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git >> acpi-scan >> $ vi drivers/acpi/pci_root.c # resolve conflicts >> $ git add drivers/acpi/pci_root.c >> $ git commit >> >> $ git checkout next >> $ git merge --no-ff --log pci/yinghai-survey-resources+acpi-scan >> >> Is that reasonable? > > Yes, it looks reasonable. > >> This won't cause issues when both Rafael and I ask Linus to pull from our >> trees later? > > No, it won't, as long as I don't rebase the original acpi-scan branch (which > I'm not going to do) and you don't rebase your > pci/yinghai-survey-resources+acpi-scan branch going forward. > > The pull makes your tree contain the same commits (i.e. commit IDs along with > the data) that are in my acpi-scan branch, so when Linus merges them together, > git will notice that the commits are the same. OK, I did the above, merged it into my -next branch, and pushed it. Let me know if you see any issues. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 9, 2013 at 11:39 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Wed, Jan 9, 2013 at 10:53 AM, Yinghai Lu <yinghai@kernel.org> wrote: >> the reason why we need to change those codes for x86, we want to make it support >> pci root bus hotplug. So it would be reasonable for us to align other >> platform to x86 >> changes after pci root bus hotplug change is completely done. > > OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a > way to keep track of this consistency issue ... What's your plan for this? We desperately need more cross-arch consistency in how we manage resources, and I'd like to make some progress in this cycle. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 2, 2013 at 2:31 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Wed, Jan 9, 2013 at 11:39 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> On Wed, Jan 9, 2013 at 10:53 AM, Yinghai Lu <yinghai@kernel.org> wrote: > >>> the reason why we need to change those codes for x86, we want to make it support >>> pci root bus hotplug. So it would be reasonable for us to align other >>> platform to x86 >>> changes after pci root bus hotplug change is completely done. >> >> OK, I opened https://bugzilla.kernel.org/show_bug.cgi?id=52531 as a >> way to keep track of this consistency issue ... > > What's your plan for this? We desperately need more cross-arch > consistency in how we manage resources, and I'd like to make some > progress in this cycle. Ok, will work on that. but will on top of [PATCH v5 0/7] PCI: Change assign unassigned resources per root bus bassis and attached. will resend them after v3.11-rc1 is out. Thanks Yinghai
Index: Bjorn-next-0925-configchange/drivers/pci/remove.c =================================================================== --- Bjorn-next-0925-configchange.orig/drivers/pci/remove.c +++ Bjorn-next-0925-configchange/drivers/pci/remove.c @@ -111,3 +111,31 @@ void pci_stop_and_remove_bus_device(stru pci_remove_bus_device(dev); } EXPORT_SYMBOL(pci_stop_and_remove_bus_device); + +void pci_stop_bus_devices(struct pci_bus *bus) +{ + struct pci_dev *dev, *tmp; + + list_for_each_entry_safe_reverse(dev, tmp, + &bus->devices, bus_list) { + pci_stop_bus_device(dev); + } + +} +EXPORT_SYMBOL(pci_stop_bus_devices); + +void pci_remove_host_bridge(struct pci_host_bridge *bridge) +{ + struct pci_bus *root = bridge->bus; + struct pci_dev *dev, *tmp; + + list_for_each_entry_safe_reverse(dev, tmp, + &root->devices, bus_list) { + pci_remove_bus_device(dev); + } + + pci_remove_bus(root); + + device_unregister(&bridge->dev); +} +EXPORT_SYMBOL(pci_remove_host_bridge); Index: Bjorn-next-0925-configchange/drivers/acpi/pci_root.c =================================================================== --- Bjorn-next-0925-configchange.orig/drivers/acpi/pci_root.c +++ Bjorn-next-0925-configchange/drivers/acpi/pci_root.c @@ -659,8 +659,10 @@ static int acpi_pci_root_remove(struct a { struct acpi_pci_root *root = acpi_driver_data(device); struct acpi_pci_driver *driver; + struct pci_host_bridge *bridge = to_pci_host_bridge(root->bus->bridge); mutex_lock(&acpi_pci_root_lock); + pci_stop_bus_devices(root->bus); list_for_each_entry(driver, &acpi_pci_drivers, node) if (driver->remove) driver->remove(root); @@ -668,6 +670,10 @@ static int acpi_pci_root_remove(struct a device_set_run_wake(root->bus->bridge, false); pci_acpi_remove_bus_pm_notifier(device); + acpi_pci_unbind_root(device); + + pci_remove_host_bridge(bridge); + list_del(&root->node); mutex_unlock(&acpi_pci_root_lock); kfree(root); Index: Bjorn-next-0925-configchange/include/linux/pci.h =================================================================== --- Bjorn-next-0925-configchange.orig/include/linux/pci.h +++ Bjorn-next-0925-configchange/include/linux/pci.h @@ -734,6 +734,8 @@ extern struct pci_dev *pci_dev_get(struc extern void pci_dev_put(struct pci_dev *dev); extern void pci_remove_bus(struct pci_bus *b); extern void pci_stop_and_remove_bus_device(struct pci_dev *dev); +extern void pci_stop_bus_devices(struct pci_bus *bus); +extern void pci_remove_host_bridge(struct pci_host_bridge *bridge); void pci_setup_cardbus(struct pci_bus *bus); extern void pci_sort_breadthfirst(void); #define dev_is_pci(d) ((d)->bus == &pci_bus_type) Index: Bjorn-next-0925-configchange/drivers/acpi/pci_bind.c =================================================================== --- Bjorn-next-0925-configchange.orig/drivers/acpi/pci_bind.c +++ Bjorn-next-0925-configchange/drivers/acpi/pci_bind.c @@ -118,3 +118,10 @@ int acpi_pci_bind_root(struct acpi_devic return 0; } + +void acpi_pci_unbind_root(struct acpi_device *device) +{ + device->ops.bind = NULL; + device->ops.unbind = NULL; +} + Index: Bjorn-next-0925-configchange/include/acpi/acpi_drivers.h =================================================================== --- Bjorn-next-0925-configchange.orig/include/acpi/acpi_drivers.h +++ Bjorn-next-0925-configchange/include/acpi/acpi_drivers.h @@ -101,6 +101,7 @@ struct pci_bus; struct pci_dev *acpi_get_pci_dev(acpi_handle); int acpi_pci_bind_root(struct acpi_device *device); +void acpi_pci_unbind_root(struct acpi_device *device); /* Arch-defined function to add a bus to the system */
Currently there's no PCI-related clean-up code in acpi_pci_root_remove() function. This patch introduces function for hostbridge removal, and brings back pci_stop_bus_devices() function. diff: rebased against current next updated according to Bjorn's comment Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- drivers/acpi/pci_bind.c | 7 +++++++ drivers/acpi/pci_root.c | 6 ++++++ drivers/pci/remove.c | 28 ++++++++++++++++++++++++++++ include/acpi/acpi_drivers.h | 1 + include/linux/pci.h | 2 ++ 5 files changed, 44 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html