PCI, x86: pci root bus hotplug support

During testing pci root bus removal, found some root bus bridge is not freed.

If booting with pnpacpi=off, those hostbridge could be freed without problem.

It turns out that some devices reference are not released during acpi_pnp_match.

That match should not hold one device ref during every calling.

Add put_device calling before returning.

Signed-off-by Yinghai Lu <[email protected]>

Cc [email protected]

Cc Len Brown <[email protected]>

Cc Adam Belay <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc [email protected]

---
 drivers/pnp/pnpacpi/core.c |    7 ++++++-

 1 files changed, 6 insertions(+), 1 deletions(-)

drivers/pnp/pnpacpi/core.c +6 -2

index b00c176..d21e8f5 100644
@@ -321,9 +321,14 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
  {
        struct acpi_device *acpi = to_acpi_device(dev);
        struct pnp_dev *pnp = _pnp;
+       struct device *physical_device;
+
+       physical_device = acpi_get_physical_device(acpi->handle);
+       if (physical_device)
+               put_device(physical_device);
 
        /* true means it matched */
-       return !acpi_get_physical_device(acpi->handle)
+       return !physical_device
            && compare_pnp_id(pnp->id, acpi_device_hid(acpi));
  }
 
--

1.7.7

During checking acpiphp code, found following sequence:
       pci_bus_size_bridges(bus); pci_bus_assign_resources(bus); pci_enable_bridges(bus);

The problem is that when bus is not root bus, pci_bus_size_bridges would also size own pci bridge bus->self if that bridge resource is not inserted before. but later pci_bus_assign_resources will not allocate those size bridge res.

So try make it less confusing, let it does not include self sizing. and add with_self parameter info __pci_bus_size_bridge()

Fixes caller in pci hotplug componets.

Signed-off-by Yinghai Lu <[email protected]>

---
 drivers/pci/hotplug/acpiphp_glue.c     |    2 +- drivers/pci/hotplug/cpci_hotplug_pci.c |    2 +- drivers/pci/hotplug/shpchp_pci.c       |    2 +- drivers/pci/setup-bus.c                |   24 +++++++++++++++--------- include/linux/pci.h                    |    1 +

 5 files changed, 19 insertions(+), 12 deletions(-)

drivers/pci/hotplug/acpiphp_glue.c +1 -1

index 806c44f..10b2122 100644
@@ -816,7 +816,7 @@ static int __ref enable_device(struct acpiphp_slot *slot)
                            dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
                                max = pci_scan_bridge(bus, dev, max, pass);
                                if (pass && dev->subordinate)
-                                       pci_bus_size_bridges(dev->subordinate);
+                                       pci_bus_size_bridges_with_self(dev->subordinate);
                        }
                }
        }

drivers/pci/hotplug/cpci_hotplug_pci.c +1 -1

index ae853cc..4ef80ad 100644
@@ -313,7 +313,7 @@ int __ref cpci_configure_slot(struct slot *slot)
                                continue;
                        }
                        child->subordinate = pci_do_scan_bus(child);
-                       pci_bus_size_bridges(child);
+                       pci_bus_size_bridges_with_self(child);
                }
                pci_dev_put(dev);
        }

drivers/pci/hotplug/shpchp_pci.c +1 -1

index df7e4bf..d92807b 100644
@@ -85,7 +85,7 @@ int __ref shpchp_configure_device(struct slot *p_slot)
                                continue;
                        }
                        child->subordinate = pci_do_scan_bus(child);
-                       pci_bus_size_bridges(child);
+                       pci_bus_size_bridges_with_self(child);
                }
                pci_configure_slot(dev);
                pci_dev_put(dev);

drivers/pci/setup-bus.c +2 -2

index f35a679..ec2026c 100644
@@ -982,8 +982,8 @@ handle_done:
        ;
  }
 
-void __ref __pci_bus_size_bridges(struct pci_bus *bus,
-                       struct list_head *realloc_head)
+static void __ref __pci_bus_size_bridges(struct pci_bus *bus,
+                       struct list_head *realloc_head, bool with_self)
  {
        struct pci_dev *dev;
        unsigned long mask, prefmask;

@@ -1001,13 +1001,13 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus,
 

               case PCI_CLASS_BRIDGE_PCI:
               default:
-                       __pci_bus_size_bridges(b, realloc_head);
+                       __pci_bus_size_bridges(b, realloc_head, true); break;
               }
       }

-       /* The root bus? */
-       if (!bus->self)
+       /* Is root bus or not wanted? */
+       if (!bus->self || !with_self)
               return;

       switch (bus->self->class >> 8) {

drivers/pci/setup-bus.c +7 -1

@@ -1047,9 +1047,15 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus,
        }
  }
 
+void __ref pci_bus_size_bridges_with_self(struct pci_bus *bus)
+{
+       __pci_bus_size_bridges(bus, NULL, true);
+}
+EXPORT_SYMBOL(pci_bus_size_bridges_with_self);
+
  void __ref pci_bus_size_bridges(struct pci_bus *bus)
  {
-       __pci_bus_size_bridges(bus, NULL);
+       __pci_bus_size_bridges(bus, NULL, false);
  }
  EXPORT_SYMBOL(pci_bus_size_bridges);
 

@@ -1362,7 +1368,7 @@ again:
        /* Depth first, calculate sizes and alignments of all

          subordinate buses. */ list_for_each_entry(bus, &pci_root_buses, node)
-               __pci_bus_size_bridges(bus, add_list);
+               __pci_bus_size_bridges(bus, add_list, false);

       /* Depth last, allocate resources and update the hardware. */ list_for_each_entry(bus, &pci_root_buses, node)

drivers/pci/setup-bus.c +0 -0

@@ -1439,7 +1445,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
                                  IORESOURCE_PREFETCH;
 

 again:
-       __pci_bus_size_bridges(parent, &add_list);
+       __pci_bus_size_bridges(parent, &add_list, true); __pci_bridge_assign_resources(bridge, &add_list, &fail_head); BUG_ON(!list_empty(&add_list));
       tried_times++;

drivers/pci/setup-bus.c +1 -1

@@ -1500,7 +1506,7 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
                    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
                        if (dev->subordinate)
                                __pci_bus_size_bridges(dev->subordinate,
-                                                        &add_list);
+                                                &add_list, true);
        up_read(&pci_bus_sem);
        __pci_bus_assign_resources(bus, &add_list, NULL);
        BUG_ON(!list_empty(&add_list));

include/linux/pci.h +1 -1

index 634bbf5..87dceca 100644
@@ -906,6 +906,7 @@ int pci_vpd_truncate(struct pci_dev *dev, size_t size);
  resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
  void pci_bus_assign_resources(const struct pci_bus *bus);
  void pci_bus_size_bridges(struct pci_bus *bus);
+void pci_bus_size_bridges_with_self(struct pci_bus *bus);
  int pci_claim_resource(struct pci_dev *, int);
  void pci_assign_unassigned_resources(void);
  void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
--

1.7.7

Current code will create rescan for every pci device under parent bus. that is not right. the device is already there, there is no reason to rescan it.

We could have rescan for pci bridges. less confusing.

Need to move rescan attr to pci dev bridge attribute group. And We should rescan bridge's secondary bus instead of primary bus.

(quoted diff) +0 -3

-v3: Use device_type for pci dev.
-v4: Seperate pci device type change out
-v5: add rescan_bridge for bridge type, and still keep the old rescan.

       may remove the old rescan later.

Signed-off-by Yinghai Lu <[email protected]>

Cc Randy Dunlap <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-pci |   10 ++++++++++ drivers/pci/pci-sysfs.c                 |   24 ++++++++++++++++++++++++

 2 files changed, 34 insertions(+), 0 deletions(-)

Documentation/ABI/testing/sysfs-bus-pci +0 -0

index 34f5110..95f0f37 100644
@@ -111,6 +111,16 @@ Description:

               from this part of the device tree.
               Depends on CONFIG_HOTPLUG.

Documentation/ABI/testing/sysfs-bus-pci +10 -0

+What:          /sys/bus/pci/devices/.../rescan_bridge
+Date:          February 2012
+Contact:       Linux PCI developers <[email protected]>
+Description:
+               Writing a non-zero value to this attribute will
+               force a rescan of the bridge and all child buses, and
+               re-discover devices removed earlier from this part of
+               the device tree.
+               Depends on CONFIG_HOTPLUG.
+

 What:          /sys/bus/pci/devices/.../reset
 Date:          July 2009
 Contact:       Michael S. Tsirkin <[email protected]>

drivers/pci/pci-sysfs.c +24 -1

index 39d15e6..d5c8ffb 100644
@@ -325,6 +325,27 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
        return count;
  }
 
+static ssize_t
+dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
+                const char *buf, size_t count)
+{
+       unsigned long val;
+       struct pci_dev *pdev = to_pci_dev(dev);
+
+       if (kstrtoul(buf, 0, &val) < 0)
+               return -EINVAL;
+
+       if (val) {
+               mutex_lock(&pci_remove_rescan_mutex);
+               pci_rescan_bus(pdev->subordinate);
+               mutex_unlock(&pci_remove_rescan_mutex);
+       }
+       return count;
+}
+
+static struct device_attribute pci_dev_bridge_rescan_attr =
+       __ATTR(rescan_bridge, (S_IWUSR|S_IWGRP), NULL, dev_bridge_rescan_store);
+
  static void remove_callback(struct device *dev)
  {
        struct pci_dev *pdev = to_pci_dev(dev);

@@ -1337,6 +1358,9 @@ static int __init pci_sysfs_init(void)
  late_initcall(pci_sysfs_init);
 
  static struct attribute *pci_dev_bridge_attrs[] = {
+#ifdef CONFIG_HOTPLUG
+       &pci_dev_bridge_rescan_attr.attr,
+#endif
        NULL,
  };
 
--

1.7.7

Quoted 2 lines · Yinghai Lu

Current code will create rescan for every pci device under parent bus. that is not right. the device is already there, there is no reason to rescan it.

"The device" (not "the device").

Quoted 5 lines

We could have rescan for pci bridges. less confusing.

Need to move rescan attr to pci dev bridge attribute group. And We should rescan bridge's secondary bus instead of primary bus.

"And we" (not "And We").

I think the idea of this patch makes sense.

You might want to consider having this actually read the bridge config again.  That way, if the user manually changed the secondary bus number or the bridge windows using setpci or something, this rescan could use that new config if possible.

I still hate the fact that we will have three different rescan files (four if you count /sys/bus/pci/rescan), but I don't have a solution to offer:
   device/rescan
   device/rescan_bridge
   bus/rescan

It supports both pci root bus and pci bus under pci bridge.

-v2: clear pci_bridge's subordinate.

Signed-off-by Yinghai Lu <[email protected]>

---
 drivers/pci/remove.c |   32 ++++++++++++++++++++++++++++++++
 include/linux/pci.h  |    1 +
 2 files changed, 33 insertions(+), 0 deletions(-)

drivers/pci/remove.c +15 -0

index d4fdf7b..9ffc071 100644
@@ -170,6 +170,38 @@ void pci_stop_bus_device(struct pci_dev *dev)
 	pci_stop_dev(dev);
 }
 
+static void pci_stop_host_bridge(struct pci_host_bridge *bridge)
+{
+	device_unregister(&bridge->dev);
+}
+/*
+ * it will support pci root bus too, in that case we need
+ *  stop and remove host bridge
+ */
+void pci_stop_and_remove_bus(struct pci_bus *bus)
+{
+	struct pci_host_bridge *host_bridge = NULL;
+	struct pci_dev *pci_bridge = NULL;
+
+	pci_stop_bus_devices(bus);
+

drivers/pci/remove.c +7 -0

+	if (host_bridge)
+		host_bridge->bus = NULL;
+
+	if (pci_bridge)
+		pci_bridge->subordinate = NULL;
+}
+
 EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
 EXPORT_SYMBOL(pci_stop_and_remove_behind_bridge);
 EXPORT_SYMBOL_GPL(pci_stop_bus_device);

include/linux/pci.h +1 -0

index 860091f..8874b90 100644
@@ -702,6 +702,7 @@ extern void pci_dev_put(struct pci_dev *dev);
 extern void pci_remove_bus(struct pci_bus *b);
 extern void __pci_remove_bus_device(struct pci_dev *dev);
 extern void pci_stop_and_remove_bus_device(struct pci_dev *dev);
+void pci_stop_and_remove_bus(struct pci_bus *bus);
 extern void pci_stop_bus_device(struct pci_dev *dev);
 void pci_setup_cardbus(struct pci_bus *bus);
 extern void pci_sort_breadthfirst(void);

Quoted 11 lines

drivers/pci/remove.c +11 -0

+void pci_stop_and_remove_bus(struct pci_bus *bus)
+{
+     struct pci_host_bridge *host_bridge = NULL;
+     struct pci_dev *pci_bridge = NULL;
+
+     pci_stop_bus_devices(bus);
+
+     if (pci_is_root_bus(bus)) {
+             host_bridge = to_pci_host_bridge(bus->bridge);
+             pci_stop_host_bridge(host_bridge);
+     } else

drivers/pci/remove.c void pci_stop_bus_device(struct pci_dev *dev) · line 197

Quoted 6 lines

+             pci_bridge = bus->self;

drivers/pci/remove.c +5 -0

+
+     __pci_remove_bus_devices(bus);
+
+     pci_remove_bus(bus);
+

It seems that the bus and host_bridge structures may have already been freed here, so should not access host_bridge structure any more.

Quoted 3 lines

+     if (host_bridge)
+             host_bridge->bus = NULL;
+

it supports both pci root bus and pci bus under pci bridge.

-v2: Change to three returns way in dev_bus_remove_store.

Signed-off-by Yinghai Lu <[email protected]>

Cc Randy Dunlap <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc [email protected]

---
 Documentation/ABI/testing/sysfs-bus-pci |    8 ++++++++ drivers/pci/pci-sysfs.c                 |   30 ++++++++++++++++++++++++++++++

 2 files changed, 38 insertions(+), 0 deletions(-)

Documentation/ABI/testing/sysfs-bus-pci +0 -0

index 95f0f37..22392de 100644
@@ -92,6 +92,14 @@ Description:

               hot-remove the PCI device and any of its children. Depends on CONFIG_HOTPLUG.

 What:          /sys/bus/pci/devices/.../pci_bus/.../rescan
 Date:          May 2011
 Contact:       Linux PCI developers <[email protected]>

drivers/pci/pci-sysfs.c +30 -1

index 4697afe..4d122cb 100644
@@ -388,6 +388,35 @@ remove_store(struct device *dev, struct device_attribute *dummy,
        return count;
  }
 
+static void bus_remove_callback(struct device *dev)
+{
+       struct pci_bus *bus = to_pci_bus(dev);
+
+       mutex_lock(&pci_remove_rescan_mutex);
+       pci_stop_and_remove_bus(bus);
+       mutex_unlock(&pci_remove_rescan_mutex);
+}
+static ssize_t
+dev_bus_remove_store(struct device *dev, struct device_attribute *attr,
+                const char *buf, size_t count)
+{
+       int err;
+       unsigned long val;
+
+       if (kstrtoul(buf, 0, &val) < 0)
+               return -EINVAL;
+
+       if (!val)
+               return count;
+
+       err = device_schedule_callback(dev, bus_remove_callback);
+
+       if (err)
+               return err;
+
+       return count;
+}
+
  static void bus_rescan_callback(struct device *dev)
  {
        struct pci_bus *bus = to_pci_bus(dev);

@@ -447,6 +476,7 @@ struct device_attribute pci_dev_attrs[] = {
  struct device_attribute pcibus_dev_attrs[] = {
  #ifdef CONFIG_HOTPLUG
        __ATTR(rescan, (S_IWUSR|S_IWGRP), NULL, dev_bus_rescan_store),
+       __ATTR(remove, (S_IWUSR|S_IWGRP), NULL, dev_bus_remove_store),
  #endif
        __ATTR(cpuaffinity, S_IRUGO, pci_bus_show_cpumaskaffinity, NULL),
        __ATTR(cpulistaffinity, S_IRUGO, pci_bus_show_cpulistaffinity, NULL),
--

1.7.7

Quoted 21 lines · Yinghai Lu

it supports both pci root bus and pci bus under pci bridge.

-v2: Change to three returns way in dev_bus_remove_store.

Signed-off-by Yinghai Lu <[email protected]>

Cc Randy Dunlap <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc [email protected]

---
 Documentation/ABI/testing/sysfs-bus-pci |    8 ++++++++ drivers/pci/pci-sysfs.c                 |   30 ++++++++++++++++++++++++++++++

 2 files changed, 38 insertions(+), 0 deletions(-)

Documentation/ABI/testing/sysfs-bus-pci +0 -0

index 95f0f37..22392de 100644
@@ -92,6 +92,14 @@ Description:

               hot-remove the PCI device and any of its children. Depends on CONFIG_HOTPLUG.

Documentation/ABI/testing/sysfs-bus-pci Description: · line 101

Quoted 7 lines

Documentation/ABI/testing/sysfs-bus-pci +7 -0

+What:          /sys/bus/pci/devices/.../pci_bus/.../remove
+Date:          March 2012
+Contact:       Linux PCI developers <[email protected]>
+Description:
+               Writing a non-zero value to this attribute will
+               hot-remove the PCI bus and any of its children.
+               Depends on CONFIG_HOTPLUG.

I don't think this interface makes sense.  This stops every device on the bus, removes the bus, unregisters the host bridge device if we're removing a root bus.

I think we should just have a "remove *bridge*" interface.  It makes no sense to me to remove a bus but leave the bridge, and it makes the code quite complicated.

The point of hotplug is to add and remove devices.  We'll get interrupts saying "please remove this device" or "please rescan the bus behind this bridge because something might have changed."

If we're removing a host bridge, the platform, e.g., ACPI, will tell us about the bridge that is going away.  The notification is attached to the *bridge*, not the bus it leads to.

The expected mechanism on x86 is an SCI from the platform, but if we need a sysfs way to start host bridge removal, I think it should be something like this:

   echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove

Quoted 22 lines · Bjorn Helgaas

Quoted 21 lines · Yinghai Lu

it supports both pci root bus and pci bus under pci bridge.

-v2: Change to three returns way in dev_bus_remove_store.

Signed-off-by Yinghai Lu <[email protected]>

Cc Randy Dunlap <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc [email protected]

---
 Documentation/ABI/testing/sysfs-bus-pci |    8 ++++++++ drivers/pci/pci-sysfs.c                 |   30 ++++++++++++++++++++++++++++++

 2 files changed, 38 insertions(+), 0 deletions(-)

Documentation/ABI/testing/sysfs-bus-pci +0 -0

index 95f0f37..22392de 100644
@@ -92,6 +92,14 @@ Description:

               hot-remove the PCI device and any of its children. Depends on CONFIG_HOTPLUG.

Quoted 11 lines

Documentation/ABI/testing/sysfs-bus-pci Description: · line 101

Quoted 7 lines

Documentation/ABI/testing/sysfs-bus-pci +7 -0

+What:          /sys/bus/pci/devices/.../pci_bus/.../remove
+Date:          March 2012
+Contact:       Linux PCI developers <[email protected]>
+Description:
+               Writing a non-zero value to this attribute will
+               hot-remove the PCI bus and any of its children.
+               Depends on CONFIG_HOTPLUG.

I don't think this interface makes sense.  This stops every device on the bus, removes the bus, unregisters the host bridge device if we're removing a root bus.

it is some kind of handy. otherwise if there is several devices on the bus, will need to
remove the devices one by one.

Quoted 3 lines

I think we should just have a "remove *bridge*" interface.  It makes no sense to me to remove a bus but leave the bridge, and it makes the code quite complicated.

I want to remove all devices under the bridge and then rescan the bridge. then if the bridge get removed.
then we have to rescan bridge's parent bus.

Quoted 9 lines

The point of hotplug is to add and remove devices.  We'll get interrupts saying "please remove this device" or "please rescan the bus behind this bridge because something might have changed."

If we're removing a host bridge, the platform, e.g., ACPI, will tell us about the bridge that is going away.  The notification is attached to the *bridge*, not the bus it leads to.

Quoted 5 lines

The expected mechanism on x86 is an SCI from the platform, but if we need a sysfs way to start host bridge removal, I think it should be something like this:

   echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove

for that,  let just use
echo "\_SB.PCI0 3" > /proc/acpi/sci/notify

It will call new added pci_stop_and_remove_bus() to stop/remove pci root bus.

Also checking if that pci_root_bus get removed already in bus remove in /sys

Signed-off-by Yinghai Lu <[email protected]>

Cc Len Brown <[email protected]>

Cc [email protected]

---
 drivers/acpi/pci_root.c |   14 ++++++++++++++

 1 files changed, 14 insertions(+), 0 deletions(-)

drivers/acpi/pci_root.c +0 -0

index 7aff631..b38e347 100644

drivers/acpi/pci_root.c +9 -1

+       }
+
        device_set_run_wake(root->bus->bridge, false);
        pci_acpi_remove_bus_pm_notifier(device);
 
+       dev_printk(KERN_DEBUG, &device->dev,
+               "freeing acpi_pci_root, will remove pci root bus at first");
+       pci_stop_and_remove_bus(root->bus);
+
+out:
+       list_del(&root->node);
        kfree(root);
+
        return 0;
  }
 
--

1.7.7

It causes confusing.

We may only need acpi hp for pci host bridge.

Split host bridge hot-add support to another file, and keep acpiphp simple.

Also remove not used res_lock in the struct.

Signed-off-by Yinghai Lu <[email protected]>

---
 drivers/pci/hotplug/acpiphp.h      |    9 +--- drivers/pci/hotplug/acpiphp_glue.c |  104 +++++++-----------------------------

 2 files changed, 21 insertions(+), 92 deletions(-)

drivers/pci/hotplug/acpiphp.h +1 -8

index 7722108..1a62e7b 100644
@@ -79,18 +79,15 @@ struct acpiphp_bridge {
        /* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */
        struct acpiphp_func *func;
 
-       int type;
        int nr_slots;
 
        u32 flags;
 
-       /* This bus (host bridge) or Secondary bus (PCI-to-PCI bridge) */
+       /* Secondary bus (PCI-to-PCI bridge) */
        struct pci_bus *pci_bus;
 
        /* PCI-to-PCI bridge device */
        struct pci_dev *pci_dev;
-
-       spinlock_t res_lock;
  };
 
 

@@ -148,10 +145,6 @@ struct acpiphp_attention_info
  /* PCI bus bridge HID */
  #define ACPI_PCI_HOST_HID              "PNP0A03"
 
-/* PCI BRIDGE type */
-#define BRIDGE_TYPE_HOST               0
-#define BRIDGE_TYPE_P2P                        1
-
  /* ACPI _STA method value (ignore bit 4; battery present) */
  #define ACPI_STA_PRESENT               (0x00000001)
  #define ACPI_STA_ENABLED               (0x00000002)

drivers/pci/hotplug/acpiphp_glue.c +18 -58

index 10b2122..1ca82ae 100644
@@ -283,7 +283,7 @@ static int detect_ejectable_slots(acpi_handle handle)
        return found;
  }
 
-/* initialize miscellaneous stuff for both root and PCI-to-PCI bridge */
+/* initialize miscellaneous stuff for PCI-to-PCI bridge */
  static void init_bridge_misc(struct acpiphp_bridge *bridge)
  {
        acpi_status status;

@@ -300,25 +300,21 @@ static void init_bridge_misc(struct acpiphp_bridge *bridge)
        }
 
        /* install notify handler */
-       if (bridge->type != BRIDGE_TYPE_HOST) {
-               if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
-                       status = acpi_remove_notify_handler(bridge->func->handle,
-                                               ACPI_SYSTEM_NOTIFY,
-                                               handle_hotplug_event_func);
-                       if (ACPI_FAILURE(status))
-                               err("failed to remove notify handler\n");
-               }
-               status = acpi_install_notify_handler(bridge->handle,
-                                            ACPI_SYSTEM_NOTIFY,
-                                            handle_hotplug_event_bridge,
-                                            bridge);
-
-               if (ACPI_FAILURE(status)) {
-                       err("failed to register interrupt notify handler\n");
-               }
+       if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
+               status = acpi_remove_notify_handler(bridge->func->handle,
+                                       ACPI_SYSTEM_NOTIFY,
+                                       handle_hotplug_event_func);
+               if (ACPI_FAILURE(status))
+                       err("failed to remove notify handler\n");
        }
-}
+       status = acpi_install_notify_handler(bridge->handle,
+                                    ACPI_SYSTEM_NOTIFY,
+                                    handle_hotplug_event_bridge,
+                                    bridge);
 
+       if (ACPI_FAILURE(status))
+               err("failed to register interrupt notify handler\n");
+}
 
  /* find acpiphp_func from acpiphp_bridge */
  static struct acpiphp_func *acpiphp_bridge_handle_to_function(acpi_handle handle)

@@ -375,28 +371,6 @@ static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge)
        }
  }
 
-
-/* allocate and initialize host bridge data structure */
-static void add_host_bridge(acpi_handle *handle)
-{
-       struct acpiphp_bridge *bridge;
-       struct acpi_pci_root *root = acpi_pci_find_root(handle);
-
-       bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
-       if (bridge == NULL)
-               return;
-
-       bridge->type = BRIDGE_TYPE_HOST;
-       bridge->handle = handle;
-
-       bridge->pci_bus = root->bus;
-
-       spin_lock_init(&bridge->res_lock);
-
-       init_bridge_misc(bridge);
-}
-
-
  /* allocate and initialize PCI-to-PCI bridge data structure */
  static void add_p2p_bridge(acpi_handle *handle)
  {

@@ -408,7 +382,6 @@ static void add_p2p_bridge(acpi_handle *handle)
                return;
        }
 
-       bridge->type = BRIDGE_TYPE_P2P;
        bridge->handle = handle;
        config_p2p_bridge_flags(bridge);
 

@@ -425,7 +398,6 @@ static void add_p2p_bridge(acpi_handle *handle)
         * (which we access during module unload).
         */
        get_device(&bridge->pci_bus->dev);
-       spin_lock_init(&bridge->res_lock);
 
        init_bridge_misc(bridge);
        return;

@@ -485,12 +457,6 @@ static int add_bridge(acpi_handle handle)
                        return 0;
        }
 
-       /* check if this bridge has ejectable slots */
-       if (detect_ejectable_slots(handle) > 0) {
-               dbg("found PCI host-bus bridge with hot-pluggable slots\n");
-               add_host_bridge(handle);
-       }
-
        /* search P2P bridges under this host bridge */
        status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
                                     find_p2p_bridge, NULL, NULL, NULL);

@@ -524,8 +490,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
        if (ACPI_FAILURE(status))
                err("failed to remove notify handler\n");
 
-       if ((bridge->type != BRIDGE_TYPE_HOST) &&
-           ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
+       if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
                status = acpi_install_notify_handler(bridge->func->handle,
                                                ACPI_SYSTEM_NOTIFY,
                                                handle_hotplug_event_func,

@@ -1078,15 +1043,10 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
  static int acpiphp_configure_bridge (acpi_handle handle)
  {
        struct pci_bus *bus;
+       struct pci_dev *pdev = acpi_get_pci_dev(handle);
 
-       if (acpi_is_root_bridge(handle)) {
-               struct acpi_pci_root *root = acpi_pci_find_root(handle);
-               bus = root->bus;
-       } else {
-               struct pci_dev *pdev = acpi_get_pci_dev(handle);
-               bus = pdev->subordinate;
-               pci_dev_put(pdev);
-       }
+       bus = pdev->subordinate;
+       pci_dev_put(pdev);
 
        pci_bus_size_bridges(bus);
        pci_bus_assign_resources(bus);

@@ -1249,8 +1209,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)

       case ACPI_NOTIFY_EJECT_REQUEST:
               /* request device eject */
               dbg("%s: Device eject notify on %s\n", __func__, objname);
-               if ((bridge->type != BRIDGE_TYPE_HOST) &&
-                   (bridge->flags & BRIDGE_HAS_EJ0)) {
+               if (bridge->flags & BRIDGE_HAS_EJ0) {
                       struct acpiphp_slot *slot;
                       slot = bridge->func->slot;
                       if (!acpiphp_disable_slot(slot))

drivers/pci/hotplug/acpiphp_glue.c +1 -25

@@ -1381,21 +1340,6 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
                              _handle_hotplug_event_func);
  }
 
-static acpi_status
-find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-       int *count = (int *)context;
-
-       if (!acpi_is_root_bridge(handle))
-               return AE_OK;
-
-       (*count)++;
-       acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-                                   handle_hotplug_event_bridge, NULL);
-
-       return AE_OK ;
-}
-
  static struct acpi_pci_driver acpi_pci_hp_driver = {
        .add =          add_bridge,
        .remove =       remove_bridge,

@@ -1406,15 +1350,7 @@ static struct acpi_pci_driver acpi_pci_hp_driver = {
  */
  int __init acpiphp_glue_init(void)
  {
-       int num = 0;
-
-       acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-                       ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
-
-       if (num <= 0)
-               return -1;
-       else
-               acpi_pci_register_driver(&acpi_pci_hp_driver);
+       acpi_pci_register_driver(&acpi_pci_hp_driver);
 
        return 0;
  }
--

1.7.7

Quoted 1 line · Yinghai Lu

It causes confusing.

"causes confusion."

Quoted 2 lines

We may only need acpi hp for pci host bridge.

Quoted 1 line

Split host bridge hot-add support to another file, and keep acpiphp simple.

I think it's a good thing to remove host bridge hotplug from acpiphp; thanks for doing that.

However, I think you removed it here, but didn't add it elsewhere until a different patch.  That means we can't bisect across this.  It would be better to do the entire move in a single patch.

Quoted 9 lines · Bjorn Helgaas

Quoted 1 line · Yinghai Lu

It causes confusing.

"causes confusion."

Quoted 3 lines

We may only need acpi hp for pci host bridge.

Split host bridge hot-add support to another file, and keep acpiphp simple.

Quoted 6 lines

I think it's a good thing to remove host bridge hotplug from acpiphp; thanks for doing that.

However, I think you removed it here, but didn't add it elsewhere until a different patch.  That means we can't bisect across this.  It would be better to do the entire move in a single patch.

do you mean merge patch 22 and 23 together?

It is from acpiphp_glue.c

How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys

 echo "\_SB.PCIB 0" > /proc/acpi/sci/notify
to add it back

Signed-off-by Yinghai Lu <[email protected]>

Cc Len Brown <[email protected]>

Cc [email protected]

---
 drivers/acpi/Makefile      |    1 +
 drivers/acpi/pci_root_hp.c |  237 ++++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 238 insertions(+), 0 deletions(-)

 create mode 100644 drivers/acpi/pci_root_hp.c

drivers/acpi/Makefile +0 -0

index 1567028..bc6e53f 100644
@@ -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
 acpi-y                         += power.o
 acpi-y                         += event.o
 acpi-y                         += sysfs.o

drivers/acpi/pci_root_hp.c new file +237 -1

new file mode 100644
index 0000000..dc11e81
@@ -0,0 +1,237 @@
+/*
+ * Separated from drivers/pci/hotplug/acpiphp_glue.c
+ *     only support root bridge
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+
+static LIST_HEAD(acpi_root_bridge_list);
+struct acpi_root_bridge {
+       struct list_head list;
+       acpi_handle handle;
+       u32 flags;
+};
+
+/* bridge flags */
+#define ROOT_BRIDGE_HAS_EJ0    (0x00000002)
+#define ROOT_BRIDGE_HAS_PS3    (0x00000080)
+
+#define ACPI_STA_FUNCTIONING   (0x00000008)
+
+static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
+{
+       struct acpi_root_bridge *bridge;
+
+       list_for_each_entry(bridge, &acpi_root_bridge_list, list)
+               if (bridge->handle == handle)
+                       return bridge;
+
+       return NULL;
+}
+
+/* allocate and initialize host bridge data structure */
+static void add_acpi_root_bridge(acpi_handle handle)
+{
+       struct acpi_root_bridge *bridge;
+       acpi_handle dummy_handle;
+       acpi_status status;
+
+       /* if the bridge doesn't have _STA, we assume it is always there */
+       status = acpi_get_handle(handle, "_STA", &dummy_handle);
+       if (ACPI_SUCCESS(status)) {
+               unsigned long long tmp;
+
+               status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
+               if (ACPI_FAILURE(status)) {
+                       printk(KERN_DEBUG "%s: _STA evaluation failure\n",
+                                                __func__);
+                       return;
+               }
+               if ((tmp & ACPI_STA_FUNCTIONING) == 0)
+                       /* don't register this object */
+                       return;
+       }
+
+       bridge = kzalloc(sizeof(struct acpi_root_bridge), GFP_KERNEL);
+       if (!bridge)
+               return;
+
+       bridge->handle = handle;
+
+       if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
+               bridge->flags |= ROOT_BRIDGE_HAS_EJ0;
+       if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
+               bridge->flags |= ROOT_BRIDGE_HAS_PS3;
+
+       list_add(&bridge->list, &acpi_root_bridge_list);
+}
+
+struct acpi_root_hp_work {
+       struct work_struct work;
+       acpi_handle handle;
+       u32 type;
+       void *context;
+};
+
+static void alloc_acpi_root_hp_work(acpi_handle handle, u32 type,
+                                       void *context,
+                                       void (*func)(struct work_struct *work))
+{
+       struct acpi_root_hp_work *hp_work;
+       int ret;
+
+       hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
+       if (!hp_work)
+               return;
+
+       hp_work->handle = handle;
+       hp_work->type = type;
+       hp_work->context = context;
+
+       INIT_WORK(&hp_work->work, func);
+       ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
+       if (!ret)
+               kfree(hp_work);
+}
+
+/* Program resources in newly inserted bridge */
+static void acpi_root_configure_bridge(acpi_handle handle)
+{
+       struct acpi_pci_root *root = acpi_pci_find_root(handle);
+
+       pci_assign_unassigned_bus_resources(root->bus);
+}
+
+static void handle_root_bridge_insertion(acpi_handle handle)
+{
+       struct acpi_device *device, *pdevice;
+       acpi_handle phandle;
+       int ret_val;
+
+       acpi_get_parent(handle, &phandle);
+       if (acpi_bus_get_device(phandle, &pdevice)) {
+               printk(KERN_DEBUG "no parent device, assuming NULL\n");
+               pdevice = NULL;
+       }
+       if (!acpi_bus_get_device(handle, &device)) {
+               /* check if  pci root_bus is removed */
+               struct acpi_pci_root *root = acpi_driver_data(device);
+               if (pci_find_bus(root->segment, root->secondary.start))
+                       return;
+
+               printk(KERN_DEBUG "bus exists... trim\n");
+               /* this shouldn't be in here, so remove
+                * the bus then re-add it...
+                */
+               ret_val = acpi_bus_trim(device, 1);
+               printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
+       }
+       if (acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE)) {
+               printk(KERN_ERR "cannot add bridge to acpi list\n");
+               return;
+       }
+       acpi_root_configure_bridge(handle);
+       if (acpi_bus_start(device))
+               printk(KERN_ERR "cannot start bridge\n");
+}
+
+static void _handle_hotplug_event_root(struct work_struct *work)
+{
+       struct acpi_root_bridge *bridge;
+       char objname[64];
+       struct acpi_buffer buffer = { .length = sizeof(objname),
+                                     .pointer = objname };
+       struct acpi_root_hp_work *hp_work;
+       acpi_handle handle;
+       u32 type;
+
+       hp_work = container_of(work, struct acpi_root_hp_work, work);
+       handle = hp_work->handle;
+       type = hp_work->type;
+
+       bridge = acpi_root_handle_to_bridge(handle);
+
+       acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+
+       switch (type) {
+       case ACPI_NOTIFY_BUS_CHECK:
+               /* bus enumerate */
+               printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
+                                objname);
+               if (!bridge) {
+                       handle_root_bridge_insertion(handle);
+                       add_acpi_root_bridge(handle);
+               }
+
+               break;
+
+       case ACPI_NOTIFY_DEVICE_CHECK:
+               /* device check */
+               printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
+                                objname);
+               if (!bridge) {
+                       handle_root_bridge_insertion(handle);
+                       add_acpi_root_bridge(handle);
+               }
+               break;
+
+       default:
+               printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
+                                type, objname);
+               break;
+       }
+
+       kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
+}
+
+static void handle_hotplug_event_root(acpi_handle handle, u32 type,
+                                       void *context)
+{
+       alloc_acpi_root_hp_work(handle, type, context,
+                               _handle_hotplug_event_root);
+}
+
+static acpi_status __init
+find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+       char objname[64];
+       struct acpi_buffer buffer = { .length = sizeof(objname),
+                                     .pointer = objname };
+       int *count = (int *)context;
+
+       if (!acpi_is_root_bridge(handle))
+               return AE_OK;
+
+       (*count)++;
+
+       acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+
+       acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+                               handle_hotplug_event_root, NULL);
+       printk(KERN_DEBUG "acpi root: %s notify handler installed\n", objname);
+
+       add_acpi_root_bridge(handle);
+
+       return AE_OK;
+}
+
+static int __init acpi_pci_root_hp_init(void)
+{
+       int num = 0;
+
+       acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+               ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
+
+       printk(KERN_DEBUG "Found %d acpi root devices\n", num);
+
+       return 0;
+}
+
+subsys_initcall(acpi_pci_root_hp_init);
--

1.7.7

Quoted 5 lines · Yinghai Lu

It is from acpiphp_glue.c

How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys

Quoted 2 lines

 echo "\_SB.PCIB 0" > /proc/acpi/sci/notify
to add it back

Nope.  That might be a way to exercise this for debug purposes (and it only works when /proc/acpi/sci/notify is compiled in), but we definitely don't want normal users to deal with ACPI pathnames like this.

Most of this functionality belongs in the ACPI core, not here.  The core should be handling these notify events and turning them into .add() and .remove() calls to the driver.

But since the ACPI core doesn't do that yet, we have to do it in the driver, like we do for CPU, memory, etc.  That part is fine.  But I think it at least belongs *in* the driver, i.e., in pci_root.c, not in a new pci_root_hp.c file.

There definitely should not be another acpi_walk_namespace() and another list of ACPI host bridges.  The core *does* already walk the namespace and call acpi_pci_root_add() for us.  And the driver already has a list of host bridges (acpi_pci_roots).

Quoted 6 lines · Bjorn Helgaas

Quoted 5 lines · Yinghai Lu

It is from acpiphp_glue.c

How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys

Quoted 7 lines

Quoted 2 lines

 echo "\_SB.PCIB 0" > /proc/acpi/sci/notify
to add it back

Nope.  That might be a way to exercise this for debug purposes (and it only works when /proc/acpi/sci/notify is compiled in), but we definitely don't want normal users to deal with ACPI pathnames like this.

before this patch set, only way is: Users really put one root bus in and press buton etc to trigger notification
to get it work.

Quoted 5 lines

Most of this functionality belongs in the ACPI core, not here.  The core should be handling these notify events and turning them into .add() and .remove() calls to the driver.

Quoted 4 lines

But since the ACPI core doesn't do that yet, we have to do it in the driver, like we do for CPU, memory, etc.  That part is fine.  But I think it at least belongs *in* the driver, i.e., in pci_root.c, not in a new pci_root_hp.c file.

don't want to pollute pci_root.c.

in separated file, we can use CONFIG_HOTPLUG in Makefile instead of pci_root.c

Quoted 4 lines

There definitely should not be another acpi_walk_namespace() and another list of ACPI host bridges.  The core *does* already walk the namespace and call acpi_pci_root_add() for us.  And the driver already has a list of host bridges (acpi_pci_roots).

no, we may not have acpi_pci_root there.

Add missing hot_remove support for root device.

How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys

 echo "\_SB.PCIB 3" > /proc/acpi/sci/notify
to remove root bus

Signed-off-by Yinghai Lu <[email protected]>

Cc Len Brown <[email protected]>

Cc [email protected]

---
 drivers/acpi/pci_root_hp.c |   61 ++++++++++++++++++++++++++++++++++++++++++++

 1 files changed, 61 insertions(+), 0 deletions(-)

drivers/acpi/pci_root_hp.c +15 -0

index dc11e81..f5585f5 100644
@@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_handle handle)
        list_add(&bridge->list, &acpi_root_bridge_list);
  }
 
+static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
+{
+       list_del(&bridge->list);
+       kfree(bridge);
+}
+
  struct acpi_root_hp_work {
        struct work_struct work;
        acpi_handle handle;

@@ -142,6 +148,55 @@ static void handle_root_bridge_insertion(acpi_handle handle)
                printk(KERN_ERR "cannot start bridge\n");
  }
 
+static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
+{
+       acpi_status status;
+       struct acpi_object_list arg_list;
+       union acpi_object arg;
+
+       arg_list.count = 1;
+       arg_list.pointer = &arg;
+       arg.type = ACPI_TYPE_INTEGER;

drivers/acpi/pci_root_hp.c +36 -0

+}
+
+static void handle_root_bridge_removal(acpi_handle handle,
+                struct acpi_root_bridge *bridge)
+{
+       u32 flags = 0;
+       struct acpi_device *device;
+
+       if (bridge) {
+               flags = bridge->flags;
+               remove_acpi_root_bridge(bridge);
+       }
+
+       if (!acpi_bus_get_device(handle, &device)) {
+               int ret_val = acpi_bus_trim(device, 1);
+
+               printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
+       }
+
+       if (flags & ROOT_BRIDGE_HAS_PS3) {
+               acpi_status status;
+
+               status = acpi_evaluate_object(handle, "_PS3", NULL, NULL);
+               if (ACPI_FAILURE(status))
+                       printk(KERN_WARNING "%s: _PS3 failed\n", __func__);
+       }
+       if (flags & ROOT_BRIDGE_HAS_EJ0)
+               acpi_root_evaluate_object(handle, "_EJ0", 1);
+}
+
  static void _handle_hotplug_event_root(struct work_struct *work)
  {
        struct acpi_root_bridge *bridge;

@@ -182,6 +237,12 @@ static void _handle_hotplug_event_root(struct work_struct *work)
                }
                break;
 
+       case ACPI_NOTIFY_EJECT_REQUEST:
+               /* request device eject */
+               printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
+                                objname);
+               handle_root_bridge_removal(handle, bridge);
+               break;

       default:
               printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n", type, objname);
--
1.7.7

Quoted 7 lines · Yinghai Lu

Add missing hot_remove support for root device.

How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys

 echo "\_SB.PCIB 3" > /proc/acpi/sci/notify
to remove root bus

Same comments as before: this is definitely NOT the way users should use this.  If you want to do that for debugging the ACPI notification path, fine, but the normal way is for the platform to generate this notification.  I think the normal way for users to manually remove an ACPI device should be something like:

   echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove

just like we do for PCI devices.

Quoted 43 lines

Signed-off-by Yinghai Lu <[email protected]>

Cc Len Brown <[email protected]>

Cc [email protected]

---
 drivers/acpi/pci_root_hp.c |   61 ++++++++++++++++++++++++++++++++++++++++++++

 1 files changed, 61 insertions(+), 0 deletions(-)

drivers/acpi/pci_root_hp.c +20 -0

index dc11e81..f5585f5 100644
@@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_handle handle)
        list_add(&bridge->list, &acpi_root_bridge_list);
  }
 
+static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
+{
+       list_del(&bridge->list);
+       kfree(bridge);
+}
+
  struct acpi_root_hp_work {
        struct work_struct work;
        acpi_handle handle;

@@ -142,6 +148,55 @@ static void handle_root_bridge_insertion(acpi_handle handle)
                printk(KERN_ERR "cannot start bridge\n");
  }
 
+static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
+{
+       acpi_status status;
+       struct acpi_object_list arg_list;
+       union acpi_object arg;
+
+       arg_list.count = 1;
+       arg_list.pointer = &arg;
+       arg.type = ACPI_TYPE_INTEGER;
+       arg.integer.value = val;
+
+       status = acpi_evaluate_object(handle, cmd, &arg_list, NULL);
+       if (ACPI_FAILURE(status)) {
+               printk(KERN_WARNING "%s: %s to %d failed\n",

drivers/acpi/pci_root_hp.c static void handle_root_bridge_insertion(acpi_handle handle) · line 169

Quoted 5 lines

drivers/acpi/pci_root_hp.c +5 -0

+                                __func__, cmd, val);
+               return -1;
+       }
+
+       return 0;

This function really has nothing to do with host bridges.  Are you sure there's no generic function you can use instead?

Quoted 15 lines · Bjorn Helgaas

Quoted 7 lines · Yinghai Lu

Add missing hot_remove support for root device.

How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys

 echo "\_SB.PCIB 3" > /proc/acpi/sci/notify
to remove root bus

Same comments as before: this is definitely NOT the way users should use this.  If you want to do that for debugging the ACPI notification path, fine, but the normal way is for the platform to generate this notification.  I think the normal way for users to manually remove an ACPI device should be something like:

Quoted 3 lines

   echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove

just like we do for PCI devices.

notify way looks more like really physical hotadd/remove.

pci bus/remove and other is not really, because when you are using /sys, you may still need to hold
one reference to bus or device.

Quoted 43 lines

Quoted 43 lines

Signed-off-by Yinghai Lu <[email protected]>

Cc Len Brown <[email protected]>

Cc [email protected]

---
 drivers/acpi/pci_root_hp.c |   61 ++++++++++++++++++++++++++++++++++++++++++++

 1 files changed, 61 insertions(+), 0 deletions(-)

drivers/acpi/pci_root_hp.c +20 -0

index dc11e81..f5585f5 100644
@@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_handle handle)
        list_add(&bridge->list, &acpi_root_bridge_list);
  }
 
+static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
+{
+       list_del(&bridge->list);
+       kfree(bridge);
+}
+
  struct acpi_root_hp_work {
        struct work_struct work;
        acpi_handle handle;

@@ -142,6 +148,55 @@ static void handle_root_bridge_insertion(acpi_handle handle)
                printk(KERN_ERR "cannot start bridge\n");
  }
 
+static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
+{
+       acpi_status status;
+       struct acpi_object_list arg_list;
+       union acpi_object arg;
+
+       arg_list.count = 1;
+       arg_list.pointer = &arg;
+       arg.type = ACPI_TYPE_INTEGER;
+       arg.integer.value = val;
+
+       status = acpi_evaluate_object(handle, cmd, &arg_list, NULL);
+       if (ACPI_FAILURE(status)) {
+               printk(KERN_WARNING "%s: %s to %d failed\n",

Quoted 8 lines

drivers/acpi/pci_root_hp.c static void handle_root_bridge_insertion(acpi_handle handle) · line 169

Quoted 5 lines

drivers/acpi/pci_root_hp.c +5 -0

+                                __func__, cmd, val);
+               return -1;
+       }
+
+       return 0;

This function really has nothing to do with host bridges.  Are you sure there's no generic function you can use instead?

seems not.

Sure, the 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' way looks more like physical hotadd/remove because it is basically injecting the notify event that the hardware/BIOS should be generating itself.  But it is platform-specific and therefore unacceptable as a general user interface.

Using /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove would be a more generic way.  The PNP0A08:00/remove implementation would basically turn into an acpi_pci_root_remove() call, just like the 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' does.

Quoted 10 lines · Bjorn Helgaas

Sure, the 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' way looks more like physical hotadd/remove because it is basically injecting the notify event that the hardware/BIOS should be generating itself.  But it is platform-specific and therefore unacceptable as a general user interface.

Using /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove would be a more generic way.  The PNP0A08:00/remove implementation would basically turn into an acpi_pci_root_remove() call, just like the 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' does.

if the users can not find _SB.PCIB etc, they may not find LNXSYSTM:00/device:00/PNP0A08:00.

so /sys/class/pci_bus/0000:80/remove is more easy for them?

It will rescan all acpi pci root if related pci root bus get removed before.

Signed-off-by Yinghai <[email protected]>

Cc Len Brown <[email protected]>

Cc [email protected]

---
 drivers/acpi/pci_root_hp.c |   17 +++++++++++++++++ include/linux/pci-acpi.h   |    7 +++++++

 2 files changed, 24 insertions(+), 0 deletions(-)

drivers/acpi/pci_root_hp.c +17 -0

index b453181..ca73d51 100644
@@ -268,3 +268,20 @@ static int __init acpi_pci_root_hp_init(void)
  }
 
  subsys_initcall(acpi_pci_root_hp_init);
+
+static acpi_status
+rescan_root_bridge(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+       if (!acpi_is_root_bridge(handle))
+               return AE_OK;
+
+       handle_root_bridge_insertion(handle);
+
+       return AE_OK;
+}
+
+void acpi_pci_root_rescan(void)
+{
+       acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+               ACPI_UINT32_MAX, rescan_root_bridge, NULL, NULL, NULL);
+}

include/linux/pci-acpi.h +7 -1

index 4462350..ac93634 100644
@@ -35,6 +35,13 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
        return acpi_get_pci_rootbridge_handle(pci_domain_nr(pbus),
                                              pbus->number);
  }
+
+void acpi_pci_root_rescan(void);
+
+#else
+
+static inline void acpi_pci_root_rescan(void) { }
+
  #endif
 
  #ifdef CONFIG_ACPI_APEI
--

1.7.7