[PATCH v3 0/6] acpi,pci: hostbridge hotplug support

[PATCH v3 0/6] acpi,pci: hostbridge hotplug support

[Taku Izumi]

Hi all,
 
  This patchset is 3rd version of the following:
    http://marc.info/?l=linux-pci&m=134457928913775&w=2
    http://marc.info/?l=linux-pci&m=134665909321684&w=2

 v2 -> v3:
   - change acpi_pci_drivers' add/remove interface according to Bjorn's suggestion
   - abandon RCU for protecting acpi_pci_roots list according to Bjorn's suggestion
   - change acpi_pci_find_root implementation to avoid potential deadlock

  * [PATCH v3 1/8] ACPI, PCI: Use normal list for struct acpi_pci_driver
  * [PATCH v3 2/8] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
  * [PATCH v3 3/8] ACPI, PCI: add acpi_pci_drivers protection
  * [PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface
  * [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation
  * [PATCH v3 6/8] ACPI, PCI: add acpi_pci_roots protection
  * [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function
  * [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

-- 
Best regards,
Taku Izumi <izumi.taku@jp.fujitsu.com>

[PATCH v3 1/8] ACPI, PCI: Use normal list for struct acpi_pci_driver

[Taku Izumi]

From: Jiang Liu <jiang.liu@huawei.com>

  ACPI, PCI: Use normal list for struct acpi_pci_driver
    
  Use normal list for struct acpi_pci_driver to simplify code.
    
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   16 +++-------------
 include/linux/acpi.h    |    2 +-
 2 files changed, 4 insertions(+), 14 deletions(-)

Index: Bjorn-next-0903/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0903/drivers/acpi/pci_root.c
@@ -72,8 +72,8 @@ static struct acpi_driver acpi_pci_root_
 };
 
 static LIST_HEAD(acpi_pci_roots);
+static LIST_HEAD(acpi_pci_drivers);
 
-static struct acpi_pci_driver *sub_driver;
 static DEFINE_MUTEX(osc_lock);
 
 int acpi_pci_register_driver(struct acpi_pci_driver *driver)
@@ -81,10 +81,7 @@ int acpi_pci_register_driver(struct acpi
 	int n = 0;
 	struct acpi_pci_root *root;
 
-	struct acpi_pci_driver **pptr = &sub_driver;
-	while (*pptr)
-		pptr = &(*pptr)->next;
-	*pptr = driver;
+	list_add_tail(&driver->node, &acpi_pci_drivers);
 
 	if (!driver->add)
 		return 0;
@@ -103,14 +100,7 @@ void acpi_pci_unregister_driver(struct a
 {
 	struct acpi_pci_root *root;
 
-	struct acpi_pci_driver **pptr = &sub_driver;
-	while (*pptr) {
-		if (*pptr == driver)
-			break;
-		pptr = &(*pptr)->next;
-	}
-	BUG_ON(!*pptr);
-	*pptr = (*pptr)->next;
+	list_del(&driver->node);
 
 	if (!driver->remove)
 		return;
Index: Bjorn-next-0903/include/linux/acpi.h
===================================================================
--- Bjorn-next-0903.orig/include/linux/acpi.h
+++ Bjorn-next-0903/include/linux/acpi.h
@@ -138,7 +138,7 @@ void acpi_penalize_isa_irq(int irq, int 
 void acpi_pci_irq_disable (struct pci_dev *dev);
 
 struct acpi_pci_driver {
-	struct acpi_pci_driver *next;
+	struct list_head node;
 	int (*add)(acpi_handle handle);
 	void (*remove)(acpi_handle handle);
 };

[PATCH v3 2/8] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges

[Taku Izumi]

From: Jiang Liu <jiang.liu@huawei.com>

  ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
    
  When hot-plugging PCI root bridge, acpi_pci_drivers' add()/remove()
  methods should be invoked to notify registered drivers.
    
  -v2: Move add calling to acpi_pci_root_start by Yinghai
    
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: Bjorn-next-0808/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0808.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0808/drivers/acpi/pci_root.c
@@ -626,14 +626,25 @@ end:
 static int acpi_pci_root_start(struct acpi_device *device)
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
+	struct acpi_pci_driver *driver;
+
+	list_for_each_entry(driver, &acpi_pci_drivers, node)
+		if (driver->add)
+			driver->add(device->handle);
 
 	pci_bus_add_devices(root->bus);
+
 	return 0;
 }
 
 static int acpi_pci_root_remove(struct acpi_device *device, int type)
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
+	struct acpi_pci_driver *driver;
+
+	list_for_each_entry(driver, &acpi_pci_drivers, node)
+		if (driver->remove)
+			driver->remove(root->device->handle);
 
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);

[PATCH v3 3/8] ACPI, PCI: add acpi_pci_drivers protection

[Taku Izumi]

Use mutex to protect global acpi_pci_drivers list against PCI
host bridge hotplug operations.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

Index: Bjorn-next-0903/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0903/drivers/acpi/pci_root.c
@@ -27,7 +27,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/types.h>
-#include <linux/spinlock.h>
+#include <linux/mutex.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/pci.h>
@@ -71,6 +71,8 @@ static struct acpi_driver acpi_pci_root_
 		},
 };
 
+/* Lock to protect both acpi_pci_roots and acpi_pci_drivers lists */
+static DEFINE_MUTEX(acpi_pci_root_lock);
 static LIST_HEAD(acpi_pci_roots);
 static LIST_HEAD(acpi_pci_drivers);
 
@@ -81,34 +83,30 @@ int acpi_pci_register_driver(struct acpi
 	int n = 0;
 	struct acpi_pci_root *root;
 
+	mutex_lock(&acpi_pci_root_lock);
 	list_add_tail(&driver->node, &acpi_pci_drivers);
-
-	if (!driver->add)
-		return 0;
-
-	list_for_each_entry(root, &acpi_pci_roots, node) {
-		driver->add(root->device->handle);
-		n++;
-	}
+	if (driver->add)
+		list_for_each_entry(root, &acpi_pci_roots, node) {
+			driver->add(root->device->handle);
+			n++;
+		}
+	mutex_unlock(&acpi_pci_root_lock);
 
 	return n;
 }
-
 EXPORT_SYMBOL(acpi_pci_register_driver);
 
 void acpi_pci_unregister_driver(struct acpi_pci_driver *driver)
 {
 	struct acpi_pci_root *root;
 
+	mutex_lock(&acpi_pci_root_lock);
 	list_del(&driver->node);
-
-	if (!driver->remove)
-		return;
-
-	list_for_each_entry(root, &acpi_pci_roots, node)
-		driver->remove(root->device->handle);
+	if (driver->remove)
+		list_for_each_entry(root, &acpi_pci_roots, node)
+			driver->remove(root->device->handle);
+	mutex_unlock(&acpi_pci_root_lock);
 }
-
 EXPORT_SYMBOL(acpi_pci_unregister_driver);
 
 acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
@@ -628,9 +626,11 @@ static int acpi_pci_root_start(struct ac
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
 			driver->add(device->handle);
+	mutex_unlock(&acpi_pci_root_lock);
 
 	pci_bus_add_devices(root->bus);
 
@@ -642,9 +642,11 @@ static int acpi_pci_root_remove(struct a
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
 			driver->remove(root->device->handle);
+	mutex_unlock(&acpi_pci_root_lock);
 
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);

[PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface

[Taku Izumi]

This patch changes .add/.remove interfaces of acpi_pci_driver.
In the current implementation acpi_handle is passed as a parameter
of .add/.remove interface. However acpi_pci_root structure other
than acpi_handle is more usefull. This enables us to avoid useless 
procedure in each acpi_pci_driver.

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c            |    8 ++++----
 drivers/acpi/pci_slot.c            |   11 ++++++-----
 drivers/pci/hotplug/acpiphp_glue.c |   12 +++++++-----
 include/linux/acpi.h               |    4 ++--
 4 files changed, 19 insertions(+), 16 deletions(-)

Index: Bjorn-next-0903/include/linux/acpi.h
===================================================================
--- Bjorn-next-0903.orig/include/linux/acpi.h
+++ Bjorn-next-0903/include/linux/acpi.h
@@ -139,8 +139,8 @@ void acpi_pci_irq_disable (struct pci_de
 
 struct acpi_pci_driver {
 	struct list_head node;
-	int (*add)(acpi_handle handle);
-	void (*remove)(acpi_handle handle);
+	int (*add)(struct acpi_pci_root *root);
+	void (*remove)(struct acpi_pci_root *root);
 };
 
 int acpi_pci_register_driver(struct acpi_pci_driver *driver);
Index: Bjorn-next-0903/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- Bjorn-next-0903.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ Bjorn-next-0903/drivers/pci/hotplug/acpiphp_glue.c
@@ -382,10 +382,10 @@ static inline void config_p2p_bridge_fla
 
 
 /* allocate and initialize host bridge data structure */
-static void add_host_bridge(acpi_handle *handle)
+static void add_host_bridge(struct acpi_pci_root *root)
 {
 	struct acpiphp_bridge *bridge;
-	struct acpi_pci_root *root = acpi_pci_find_root(handle);
+	acpi_handle handle = root->device->handle;
 
 	bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
 	if (bridge == NULL)
@@ -468,11 +468,12 @@ find_p2p_bridge(acpi_handle handle, u32 
 
 
 /* find hot-pluggable slots, and then find P2P bridge */
-static int add_bridge(acpi_handle handle)
+static int add_bridge(struct acpi_pci_root *root)
 {
 	acpi_status status;
 	unsigned long long tmp;
 	acpi_handle dummy_handle;
+	acpi_handle handle = root->device->handle;
 
 	/* if the bridge doesn't have _STA, we assume it is always there */
 	status = acpi_get_handle(handle, "_STA", &dummy_handle);
@@ -490,7 +491,7 @@ static int add_bridge(acpi_handle handle
 	/* 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);
+		add_host_bridge(root);
 	}
 
 	/* search P2P bridges under this host bridge */
@@ -588,9 +589,10 @@ cleanup_p2p_bridge(acpi_handle handle, u
 	return AE_OK;
 }
 
-static void remove_bridge(acpi_handle handle)
+static void remove_bridge(struct acpi_pci_root *root)
 {
 	struct acpiphp_bridge *bridge;
+	acpi_handle handle = root->device->handle;
 
 	/* cleanup p2p bridges under this host bridge
 	   in a depth-first manner */
Index: Bjorn-next-0903/drivers/acpi/pci_slot.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_slot.c
+++ Bjorn-next-0903/drivers/acpi/pci_slot.c
@@ -67,8 +67,8 @@ struct acpi_pci_slot {
 	struct list_head list;		/* node in the list of slots */
 };
 
-static int acpi_pci_slot_add(acpi_handle handle);
-static void acpi_pci_slot_remove(acpi_handle handle);
+static int acpi_pci_slot_add(struct acpi_pci_root *root);
+static void acpi_pci_slot_remove(struct acpi_pci_root *root);
 
 static LIST_HEAD(slot_list);
 static DEFINE_MUTEX(slot_list_lock);
@@ -295,11 +295,11 @@ walk_root_bridge(acpi_handle handle, acp
  * @handle: points to an acpi_pci_root
  */
 static int
-acpi_pci_slot_add(acpi_handle handle)
+acpi_pci_slot_add(struct acpi_pci_root *root)
 {
 	acpi_status status;
 
-	status = walk_root_bridge(handle, register_slot);
+	status = walk_root_bridge(root->device->handle, register_slot);
 	if (ACPI_FAILURE(status))
 		err("%s: register_slot failure - %d\n", __func__, status);
 
@@ -311,10 +311,11 @@ acpi_pci_slot_add(acpi_handle handle)
  * @handle: points to an acpi_pci_root
  */
 static void
-acpi_pci_slot_remove(acpi_handle handle)
+acpi_pci_slot_remove(struct acpi_pci_root *root)
 {
 	struct acpi_pci_slot *slot, *tmp;
 	struct pci_bus *pbus;
+	acpi_handle handle = root->device->handle;
 
 	mutex_lock(&slot_list_lock);
 	list_for_each_entry_safe(slot, tmp, &slot_list, list) {
Index: Bjorn-next-0903/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0903/drivers/acpi/pci_root.c
@@ -87,7 +87,7 @@ int acpi_pci_register_driver(struct acpi
 	list_add_tail(&driver->node, &acpi_pci_drivers);
 	if (driver->add)
 		list_for_each_entry(root, &acpi_pci_roots, node) {
-			driver->add(root->device->handle);
+			driver->add(root);
 			n++;
 		}
 	mutex_unlock(&acpi_pci_root_lock);
@@ -104,7 +104,7 @@ void acpi_pci_unregister_driver(struct a
 	list_del(&driver->node);
 	if (driver->remove)
 		list_for_each_entry(root, &acpi_pci_roots, node)
-			driver->remove(root->device->handle);
+			driver->remove(root);
 	mutex_unlock(&acpi_pci_root_lock);
 }
 EXPORT_SYMBOL(acpi_pci_unregister_driver);
@@ -629,7 +629,7 @@ static int acpi_pci_root_start(struct ac
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
-			driver->add(device->handle);
+			driver->add(root);
 	mutex_unlock(&acpi_pci_root_lock);
 
 	pci_bus_add_devices(root->bus);
@@ -645,7 +645,7 @@ static int acpi_pci_root_remove(struct a
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
-			driver->remove(root->device->handle);
+			driver->remove(root);
 	mutex_unlock(&acpi_pci_root_lock);
 
 	device_set_run_wake(root->bus->bridge, false);

[PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation

[Taku Izumi]

This patch changes the implementation of acpi_pci_find_root().

We can access acpi_pci_root without scanning acpi_pci_roots list.
If hostbridge hotplug is supported, acpi_pci_roots list will be
protected by mutex. We should not access acpi_pci_roots list 
if preventable to lessen deadlock risk.

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Index: Bjorn-next-0903/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0903/drivers/acpi/pci_root.c
@@ -265,12 +265,15 @@ static acpi_status acpi_pci_osc_support(
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
 {
 	struct acpi_pci_root *root;
+	struct acpi_device *device;
 
-	list_for_each_entry(root, &acpi_pci_roots, node) {
-		if (root->device->handle == handle)
-			return root;
-	}
-	return NULL;
+	if (acpi_bus_get_device(handle, &device) ||
+	    acpi_match_device_ids(device, root_device_ids))
+		return NULL;
+
+	root = acpi_driver_data(device);
+
+	return root;
 }
 EXPORT_SYMBOL_GPL(acpi_pci_find_root);
 

[PATCH v3 6/8] ACPI, PCI: add acpi_pci_roots protection

[Taku Izumi]

Use mutex to protect acpi_pci_roots list against PCI host bridge 
hotplug operations.

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

Index: Bjorn-next-0903/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0903/drivers/acpi/pci_root.c
@@ -112,12 +112,17 @@ EXPORT_SYMBOL(acpi_pci_unregister_driver
 acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
 {
 	struct acpi_pci_root *root;
+	struct acpi_handle *handle = NULL;
 	
+	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(root, &acpi_pci_roots, node)
 		if ((root->segment == (u16) seg) &&
-		    (root->secondary.start == (u16) bus))
-			return root->device->handle;
-	return NULL;		
+		    (root->secondary.start == (u16) bus)) {
+			handle = root->device->handle;
+			break;
+		}
+	mutex_unlock(&acpi_pci_root_lock);
+	return handle;
 }
 
 EXPORT_SYMBOL_GPL(acpi_get_pci_rootbridge_handle);
@@ -509,8 +514,9 @@ static int __devinit acpi_pci_root_add(s
 	 * TBD: Need PCI interface for enumeration/configuration of roots.
 	 */
 
-	/* TBD: Locking */
+	mutex_lock(&acpi_pci_root_lock);
 	list_add_tail(&root->node, &acpi_pci_roots);
+	mutex_unlock(&acpi_pci_root_lock);
 
 	printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
 	       acpi_device_name(device), acpi_device_bid(device),
@@ -529,7 +535,7 @@ static int __devinit acpi_pci_root_add(s
 			    "Bus %04x:%02x not present in PCI namespace\n",
 			    root->segment, (unsigned int)root->secondary.start);
 		result = -ENODEV;
-		goto end;
+		goto out_del_root;
 	}
 
 	/*
@@ -539,7 +545,7 @@ static int __devinit acpi_pci_root_add(s
 	 */
 	result = acpi_pci_bind_root(device);
 	if (result)
-		goto end;
+		goto out_del_root;
 
 	/*
 	 * PCI Routing Table
@@ -617,9 +623,11 @@ static int __devinit acpi_pci_root_add(s
 
 	return 0;
 
+out_del_root:
+	mutex_lock(&acpi_pci_root_lock);
+	list_del(&root->node);
+	mutex_unlock(&acpi_pci_root_lock);
 end:
-	if (!list_empty(&root->node))
-		list_del(&root->node);
 	kfree(root);
 	return result;
 }
@@ -649,11 +657,12 @@ static int acpi_pci_root_remove(struct a
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
 			driver->remove(root);
-	mutex_unlock(&acpi_pci_root_lock);
 
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
+	list_del(&root->node);
+	mutex_unlock(&acpi_pci_root_lock);
 	kfree(root);
 	return 0;
 }

[PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Taku Izumi]

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.

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_bind.c     |    7 +++++++
 drivers/acpi/pci_root.c     |    7 +++++++
 drivers/pci/remove.c        |   27 +++++++++++++++++++++++++++
 include/acpi/acpi_drivers.h |    1 +
 include/linux/pci.h         |    2 ++
 5 files changed, 44 insertions(+)

Index: Bjorn-next-0903/drivers/pci/remove.c
===================================================================
--- Bjorn-next-0903.orig/drivers/pci/remove.c
+++ Bjorn-next-0903/drivers/pci/remove.c
@@ -92,3 +92,30 @@ void pci_stop_and_remove_bus_device(stru
 	pci_destroy_dev(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) {
+		if (dev->subordinate)
+			pci_stop_bus_devices(dev->subordinate);
+		pci_stop_dev(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_stop_and_remove_bus_device(dev);
+
+	pci_remove_bus(root);
+
+	device_unregister(&bridge->dev);
+}
+EXPORT_SYMBOL(pci_remove_host_bridge);
Index: Bjorn-next-0903/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0903/drivers/acpi/pci_root.c
@@ -652,8 +652,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);
@@ -661,6 +663,11 @@ 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_irq_del_prt(root->bus);
+	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-0903/include/linux/pci.h
===================================================================
--- Bjorn-next-0903.orig/include/linux/pci.h
+++ Bjorn-next-0903/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-0903/drivers/acpi/pci_bind.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_bind.c
+++ Bjorn-next-0903/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-0903/include/acpi/acpi_drivers.h
===================================================================
--- Bjorn-next-0903.orig/include/acpi/acpi_drivers.h
+++ Bjorn-next-0903/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 */
 

[PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

[Taku Izumi]

Devices under hot-added hostbridge have no chance to assign resources
and to configure them, so this patch adds such code for hot-added 
hostbridges at acpi_pci_root_start().

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   17 +++++++++++++++++
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 18 insertions(+)

Index: Bjorn-next-0903/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0903/drivers/acpi/pci_root.c
@@ -38,6 +38,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <acpi/apei.h>
+#include <linux/pci_hotplug.h>
 
 #define PREFIX "ACPI: "
 
@@ -459,6 +460,9 @@ static int __devinit acpi_pci_root_add(s
 	if (!root)
 		return -ENOMEM;
 
+	if (system_state != SYSTEM_BOOTING)
+		root->hot_added = true;
+
 	segment = 0;
 	status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL,
 				       &segment);
@@ -636,6 +640,7 @@ static int acpi_pci_root_start(struct ac
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
+	struct pci_dev *pdev;
 
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
@@ -643,6 +648,18 @@ static int acpi_pci_root_start(struct ac
 			driver->add(root);
 	mutex_unlock(&acpi_pci_root_lock);
 
+	/*
+	 * Devices under hot-added hostbridge have no chance to assign
+	 * resources and to configure them, so do that here
+	 */
+	if (root->hot_added) {
+		pci_bus_size_bridges(root->bus);
+		pci_bus_assign_resources(root->bus);
+		list_for_each_entry(pdev, &root->bus->devices, bus_list)
+			pci_configure_slot(pdev);
+		pci_enable_bridges(root->bus);
+	}
+
 	pci_bus_add_devices(root->bus);
 
 	return 0;
Index: Bjorn-next-0903/include/acpi/acpi_bus.h
===================================================================
--- Bjorn-next-0903.orig/include/acpi/acpi_bus.h
+++ Bjorn-next-0903/include/acpi/acpi_bus.h
@@ -407,6 +407,7 @@ struct acpi_pci_root {
 	u32 osc_support_set;	/* _OSC state of support bits */
 	u32 osc_control_set;	/* _OSC state of control bits */
 	phys_addr_t mcfg_addr;
+	bool hot_added;
 };
 
 /* helper */

Re: [PATCH v3 0/6] acpi,pci: hostbridge hotplug support

[Yinghai Lu]

On Mon, Sep 17, 2012 at 11:12 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> Hi all,
>
>   This patchset is 3rd version of the following:
>     http://marc.info/?l=linux-pci&m=134457928913775&w=2
>     http://marc.info/?l=linux-pci&m=134665909321684&w=2
>
>  v2 -> v3:
>    - change acpi_pci_drivers' add/remove interface according to Bjorn's suggestion
>    - abandon RCU for protecting acpi_pci_roots list according to Bjorn's suggestion
>    - change acpi_pci_find_root implementation to avoid potential deadlock
>
>   * [PATCH v3 1/8] ACPI, PCI: Use normal list for struct acpi_pci_driver
>   * [PATCH v3 2/8] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>   * [PATCH v3 3/8] ACPI, PCI: add acpi_pci_drivers protection
>   * [PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface
>   * [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation
>   * [PATCH v3 6/8] ACPI, PCI: add acpi_pci_roots protection
>   * [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function
>   * [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

now it is more similar to my for-pci-root-bus-hotplug-part4 and part5

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-root-bus-hotplug-part5

big difference:
I separated code from acpiphp_glue.c to pci_root_hp.c for root bridge
hotplug support

I thought we all agreed on that acpi root bridge hotplug plug need to
be separated from acpiphp at first.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ab9fd4ccbfb6120a4a4adf9833487d656b0501d3

and CONFIG_HOTPLUG will always be enabled, so root bridge hotplug will
be always enabled.
acpiphp will still be used as one module, and to keep it simple pciehp
will be used for pcie etc.
That make the system BIOS much simple to avoid acpiphp.

Thanks

Yinghai

Re: [PATCH v3 0/6] acpi,pci: hostbridge hotplug support

[Yinghai Lu]

On Tue, Sep 18, 2012 at 11:41 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Sep 17, 2012 at 11:12 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>> Hi all,
>>
>>   This patchset is 3rd version of the following:
>>     http://marc.info/?l=linux-pci&m=134457928913775&w=2
>>     http://marc.info/?l=linux-pci&m=134665909321684&w=2
>>
>>  v2 -> v3:
>>    - change acpi_pci_drivers' add/remove interface according to Bjorn's suggestion
>>    - abandon RCU for protecting acpi_pci_roots list according to Bjorn's suggestion
>>    - change acpi_pci_find_root implementation to avoid potential deadlock
>>
>>   * [PATCH v3 1/8] ACPI, PCI: Use normal list for struct acpi_pci_driver
>>   * [PATCH v3 2/8] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>>   * [PATCH v3 3/8] ACPI, PCI: add acpi_pci_drivers protection
>>   * [PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface
>>   * [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation
>>   * [PATCH v3 6/8] ACPI, PCI: add acpi_pci_roots protection
>>   * [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function
>>   * [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
>
> now it is more similar to my for-pci-root-bus-hotplug-part4 and part5
>
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-root-bus-hotplug-part5
>
> big difference:
> I separated code from acpiphp_glue.c to pci_root_hp.c for root bridge
> hotplug support
>
> I thought we all agreed on that acpi root bridge hotplug plug need to
> be separated from acpiphp at first.
>
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ab9fd4ccbfb6120a4a4adf9833487d656b0501d3
>
> and CONFIG_HOTPLUG will always be enabled, so root bridge hotplug will
> be always enabled.
> acpiphp will still be used as one module, and to keep it simple pciehp
> will be used for pcie etc.
> That make the system BIOS much simple to avoid acpiphp.

also current acpiphp only have root bridge add notification.
and your code do not add any code for remove notification.

So how to you test your code with removal path?

Thanks

Yinghai

Re: [PATCH v3 0/6] acpi,pci: hostbridge hotplug support

[Jiang Liu]

On 2012-9-19 2:41, Yinghai Lu wrote:
> On Mon, Sep 17, 2012 at 11:12 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>> Hi all,
>>
>>   This patchset is 3rd version of the following:
>>     http://marc.info/?l=linux-pci&m=134457928913775&w=2
>>     http://marc.info/?l=linux-pci&m=134665909321684&w=2
>>
>>  v2 -> v3:
>>    - change acpi_pci_drivers' add/remove interface according to Bjorn's suggestion
>>    - abandon RCU for protecting acpi_pci_roots list according to Bjorn's suggestion
>>    - change acpi_pci_find_root implementation to avoid potential deadlock
>>
>>   * [PATCH v3 1/8] ACPI, PCI: Use normal list for struct acpi_pci_driver
>>   * [PATCH v3 2/8] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
>>   * [PATCH v3 3/8] ACPI, PCI: add acpi_pci_drivers protection
>>   * [PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface
>>   * [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation
>>   * [PATCH v3 6/8] ACPI, PCI: add acpi_pci_roots protection
>>   * [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function
>>   * [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
> 
> now it is more similar to my for-pci-root-bus-hotplug-part4 and part5
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-root-bus-hotplug-part5
> 
> big difference:
> I separated code from acpiphp_glue.c to pci_root_hp.c for root bridge
> hotplug support
> 
> I thought we all agreed on that acpi root bridge hotplug plug need to
> be separated from acpiphp at first.
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ab9fd4ccbfb6120a4a4adf9833487d656b0501d3
> 
> and CONFIG_HOTPLUG will always be enabled, so root bridge hotplug will
> be always enabled.
> acpiphp will still be used as one module, and to keep it simple pciehp
> will be used for pcie etc.
> That make the system BIOS much simple to avoid acpiphp.
I think it's a better way to separate host bridge hotplug functionality
from acpiphp driver, so acpiphp only handles PCI device hotplug, which
is much more clear.
--Gerry

Re: [PATCH v3 0/6] acpi,pci: hostbridge hotplug support

[Taku Izumi]

On Tue, 18 Sep 2012 11:57:59 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> On Tue, Sep 18, 2012 at 11:41 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Mon, Sep 17, 2012 at 11:12 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> >> Hi all,
> >>
> >>   This patchset is 3rd version of the following:
> >>     http://marc.info/?l=linux-pci&m=134457928913775&w=2
> >>     http://marc.info/?l=linux-pci&m=134665909321684&w=2
> >>
> >>  v2 -> v3:
> >>    - change acpi_pci_drivers' add/remove interface according to Bjorn's suggestion
> >>    - abandon RCU for protecting acpi_pci_roots list according to Bjorn's suggestion
> >>    - change acpi_pci_find_root implementation to avoid potential deadlock
> >>
> >>   * [PATCH v3 1/8] ACPI, PCI: Use normal list for struct acpi_pci_driver
> >>   * [PATCH v3 2/8] ACPI, PCI: Notify acpi_pci_drivers when hot-plugging PCI root bridges
> >>   * [PATCH v3 3/8] ACPI, PCI: add acpi_pci_drivers protection
> >>   * [PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface
> >>   * [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation
> >>   * [PATCH v3 6/8] ACPI, PCI: add acpi_pci_roots protection
> >>   * [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function
> >>   * [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge
> >
> > now it is more similar to my for-pci-root-bus-hotplug-part4 and part5
> >
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-root-bus-hotplug-part5
> >
> > big difference:
> > I separated code from acpiphp_glue.c to pci_root_hp.c for root bridge
> > hotplug support
> >
> > I thought we all agreed on that acpi root bridge hotplug plug need to
> > be separated from acpiphp at first.
> >
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ab9fd4ccbfb6120a4a4adf9833487d656b0501d3
> >
> > and CONFIG_HOTPLUG will always be enabled, so root bridge hotplug will
> > be always enabled.
> > acpiphp will still be used as one module, and to keep it simple pciehp
> > will be used for pcie etc.
> > That make the system BIOS much simple to avoid acpiphp.
> 
> also current acpiphp only have root bridge add notification.
> and your code do not add any code for remove notification.
> 
> So how to you test your code with removal path?

 You were saying how to hot-remove hostbridge ?
 I tested by ejecting ACPI container which includes hostbridge.
 acpiphp is not the only way to hotplug hostbridge, so I ommit acpiphp
 work from this patchset. This patchset is core part.
  
Best regards,
Taku Izumi <izumi.taku@jp.fujitsu.com>

Re: [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation

[Bjorn Helgaas]

On Tue, Sep 18, 2012 at 12:23 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>
> This patch changes the implementation of acpi_pci_find_root().
>
> We can access acpi_pci_root without scanning acpi_pci_roots list.
> If hostbridge hotplug is supported, acpi_pci_roots list will be
> protected by mutex. We should not access acpi_pci_roots list
> if preventable to lessen deadlock risk.
>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/acpi/pci_root.c |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
> @@ -265,12 +265,15 @@ static acpi_status acpi_pci_osc_support(
>  struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
>  {
>         struct acpi_pci_root *root;
> +       struct acpi_device *device;
>
> -       list_for_each_entry(root, &acpi_pci_roots, node) {
> -               if (root->device->handle == handle)
> -                       return root;
> -       }
> -       return NULL;
> +       if (acpi_bus_get_device(handle, &device) ||
> +           acpi_match_device_ids(device, root_device_ids))

What's the purpose of the acpi_match_device_ids() check?  It's not
obvious, so worth calling it out in the changelog, and maybe even a
comment in the code.

Nice to get rid of the list traversal.

> +               return NULL;
> +
> +       root = acpi_driver_data(device);
> +
> +       return root;
>  }
>  EXPORT_SYMBOL_GPL(acpi_pci_find_root);
>
>

Re: [PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface

[Bjorn Helgaas]

On Tue, Sep 18, 2012 at 12:22 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>
> This patch changes .add/.remove interfaces of acpi_pci_driver.
> In the current implementation acpi_handle is passed as a parameter
> of .add/.remove interface. However acpi_pci_root structure other
> than acpi_handle is more usefull. This enables us to avoid useless
> procedure in each acpi_pci_driver.

Now that .add() gets the struct acpi_pci_root, would you like to clean
up walk_root_bridge()?  I think we could remove the _SEG and _BBN
evaluations as well as the pci_find_bus(), since that information is
all in the struct acpi_pci_root.

This should be a separate patch, if you choose to do it :)

> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/acpi/pci_root.c            |    8 ++++----
>  drivers/acpi/pci_slot.c            |   11 ++++++-----
>  drivers/pci/hotplug/acpiphp_glue.c |   12 +++++++-----
>  include/linux/acpi.h               |    4 ++--
>  4 files changed, 19 insertions(+), 16 deletions(-)
>
> Index: Bjorn-next-0903/include/linux/acpi.h
> ===================================================================
> --- Bjorn-next-0903.orig/include/linux/acpi.h
> +++ Bjorn-next-0903/include/linux/acpi.h
> @@ -139,8 +139,8 @@ void acpi_pci_irq_disable (struct pci_de
>
>  struct acpi_pci_driver {
>         struct list_head node;
> -       int (*add)(acpi_handle handle);
> -       void (*remove)(acpi_handle handle);
> +       int (*add)(struct acpi_pci_root *root);
> +       void (*remove)(struct acpi_pci_root *root);
>  };
>
>  int acpi_pci_register_driver(struct acpi_pci_driver *driver);
> Index: Bjorn-next-0903/drivers/pci/hotplug/acpiphp_glue.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/pci/hotplug/acpiphp_glue.c
> +++ Bjorn-next-0903/drivers/pci/hotplug/acpiphp_glue.c
> @@ -382,10 +382,10 @@ static inline void config_p2p_bridge_fla
>
>
>  /* allocate and initialize host bridge data structure */
> -static void add_host_bridge(acpi_handle *handle)
> +static void add_host_bridge(struct acpi_pci_root *root)
>  {
>         struct acpiphp_bridge *bridge;
> -       struct acpi_pci_root *root = acpi_pci_find_root(handle);
> +       acpi_handle handle = root->device->handle;
>
>         bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
>         if (bridge == NULL)
> @@ -468,11 +468,12 @@ find_p2p_bridge(acpi_handle handle, u32
>
>
>  /* find hot-pluggable slots, and then find P2P bridge */
> -static int add_bridge(acpi_handle handle)
> +static int add_bridge(struct acpi_pci_root *root)
>  {
>         acpi_status status;
>         unsigned long long tmp;
>         acpi_handle dummy_handle;
> +       acpi_handle handle = root->device->handle;
>
>         /* if the bridge doesn't have _STA, we assume it is always there */
>         status = acpi_get_handle(handle, "_STA", &dummy_handle);
> @@ -490,7 +491,7 @@ static int add_bridge(acpi_handle handle
>         /* 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);
> +               add_host_bridge(root);
>         }
>
>         /* search P2P bridges under this host bridge */
> @@ -588,9 +589,10 @@ cleanup_p2p_bridge(acpi_handle handle, u
>         return AE_OK;
>  }
>
> -static void remove_bridge(acpi_handle handle)
> +static void remove_bridge(struct acpi_pci_root *root)
>  {
>         struct acpiphp_bridge *bridge;
> +       acpi_handle handle = root->device->handle;
>
>         /* cleanup p2p bridges under this host bridge
>            in a depth-first manner */
> Index: Bjorn-next-0903/drivers/acpi/pci_slot.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/acpi/pci_slot.c
> +++ Bjorn-next-0903/drivers/acpi/pci_slot.c
> @@ -67,8 +67,8 @@ struct acpi_pci_slot {
>         struct list_head list;          /* node in the list of slots */
>  };
>
> -static int acpi_pci_slot_add(acpi_handle handle);
> -static void acpi_pci_slot_remove(acpi_handle handle);
> +static int acpi_pci_slot_add(struct acpi_pci_root *root);
> +static void acpi_pci_slot_remove(struct acpi_pci_root *root);
>
>  static LIST_HEAD(slot_list);
>  static DEFINE_MUTEX(slot_list_lock);
> @@ -295,11 +295,11 @@ walk_root_bridge(acpi_handle handle, acp
>   * @handle: points to an acpi_pci_root
>   */
>  static int
> -acpi_pci_slot_add(acpi_handle handle)
> +acpi_pci_slot_add(struct acpi_pci_root *root)
>  {
>         acpi_status status;
>
> -       status = walk_root_bridge(handle, register_slot);
> +       status = walk_root_bridge(root->device->handle, register_slot);
>         if (ACPI_FAILURE(status))
>                 err("%s: register_slot failure - %d\n", __func__, status);
>
> @@ -311,10 +311,11 @@ acpi_pci_slot_add(acpi_handle handle)
>   * @handle: points to an acpi_pci_root
>   */
>  static void
> -acpi_pci_slot_remove(acpi_handle handle)
> +acpi_pci_slot_remove(struct acpi_pci_root *root)
>  {
>         struct acpi_pci_slot *slot, *tmp;
>         struct pci_bus *pbus;
> +       acpi_handle handle = root->device->handle;
>
>         mutex_lock(&slot_list_lock);
>         list_for_each_entry_safe(slot, tmp, &slot_list, list) {
> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
> @@ -87,7 +87,7 @@ int acpi_pci_register_driver(struct acpi
>         list_add_tail(&driver->node, &acpi_pci_drivers);
>         if (driver->add)
>                 list_for_each_entry(root, &acpi_pci_roots, node) {
> -                       driver->add(root->device->handle);
> +                       driver->add(root);
>                         n++;
>                 }
>         mutex_unlock(&acpi_pci_root_lock);
> @@ -104,7 +104,7 @@ void acpi_pci_unregister_driver(struct a
>         list_del(&driver->node);
>         if (driver->remove)
>                 list_for_each_entry(root, &acpi_pci_roots, node)
> -                       driver->remove(root->device->handle);
> +                       driver->remove(root);
>         mutex_unlock(&acpi_pci_root_lock);
>  }
>  EXPORT_SYMBOL(acpi_pci_unregister_driver);
> @@ -629,7 +629,7 @@ static int acpi_pci_root_start(struct ac
>         mutex_lock(&acpi_pci_root_lock);
>         list_for_each_entry(driver, &acpi_pci_drivers, node)
>                 if (driver->add)
> -                       driver->add(device->handle);
> +                       driver->add(root);
>         mutex_unlock(&acpi_pci_root_lock);
>
>         pci_bus_add_devices(root->bus);
> @@ -645,7 +645,7 @@ static int acpi_pci_root_remove(struct a
>         mutex_lock(&acpi_pci_root_lock);
>         list_for_each_entry(driver, &acpi_pci_drivers, node)
>                 if (driver->remove)
> -                       driver->remove(root->device->handle);
> +                       driver->remove(root);
>         mutex_unlock(&acpi_pci_root_lock);
>
>         device_set_run_wake(root->bus->bridge, false);
>

Re: [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

[Yinghai Lu]

On Mon, Sep 17, 2012 at 11:26 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>
> Devices under hot-added hostbridge have no chance to assign resources
> and to configure them, so this patch adds such code for hot-added
> hostbridges at acpi_pci_root_start().
>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/acpi/pci_root.c |   17 +++++++++++++++++
>  include/acpi/acpi_bus.h |    1 +
>  2 files changed, 18 insertions(+)
>
> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
> @@ -38,6 +38,7 @@
>  #include <acpi/acpi_bus.h>
>  #include <acpi/acpi_drivers.h>
>  #include <acpi/apei.h>
> +#include <linux/pci_hotplug.h>
>
>  #define PREFIX "ACPI: "
>
> @@ -459,6 +460,9 @@ static int __devinit acpi_pci_root_add(s
>         if (!root)
>                 return -ENOMEM;
>
> +       if (system_state != SYSTEM_BOOTING)
> +               root->hot_added = true;
> +

should be moved down just before return.

>         segment = 0;
>         status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL,
>                                        &segment);
> @@ -636,6 +640,7 @@ static int acpi_pci_root_start(struct ac
>  {
>         struct acpi_pci_root *root = acpi_driver_data(device);
>         struct acpi_pci_driver *driver;
> +       struct pci_dev *pdev;
>
>         mutex_lock(&acpi_pci_root_lock);
>         list_for_each_entry(driver, &acpi_pci_drivers, node)
> @@ -643,6 +648,18 @@ static int acpi_pci_root_start(struct ac
>                         driver->add(root);
>         mutex_unlock(&acpi_pci_root_lock);
>
> +       /*
> +        * Devices under hot-added hostbridge have no chance to assign
> +        * resources and to configure them, so do that here
> +        */
> +       if (root->hot_added) {
> +               pci_bus_size_bridges(root->bus);
> +               pci_bus_assign_resources(root->bus);
> +               list_for_each_entry(pdev, &root->bus->devices, bus_list)
> +                       pci_configure_slot(pdev);
> +               pci_enable_bridges(root->bus);
> +       }

should move that before acpi_pci_drivers.add calling

> +
>         pci_bus_add_devices(root->bus);
>
>         return 0;
> Index: Bjorn-next-0903/include/acpi/acpi_bus.h
> ===================================================================
> --- Bjorn-next-0903.orig/include/acpi/acpi_bus.h
> +++ Bjorn-next-0903/include/acpi/acpi_bus.h
> @@ -407,6 +407,7 @@ struct acpi_pci_root {
>         u32 osc_support_set;    /* _OSC state of support bits */
>         u32 osc_control_set;    /* _OSC state of control bits */
>         phys_addr_t mcfg_addr;
> +       bool hot_added;
>  };
>
>  /* helper */
>

Re: [PATCH v3 0/6] acpi,pci: hostbridge hotplug support

[Yinghai Lu]

On Tue, Sep 18, 2012 at 9:53 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> On Tue, 18 Sep 2012 11:57:59 -0700
>> also current acpiphp only have root bridge add notification.
>> and your code do not add any code for remove notification.
>>
>> So how to you test your code with removal path?
>
>  You were saying how to hot-remove hostbridge ?
>  I tested by ejecting ACPI container which includes hostbridge.
>  acpiphp is not the only way to hotplug hostbridge, so I ommit acpiphp
>  work from this patchset. This patchset is core part.

yes.

so acpi root for pci root bus will have parent? I did not think about.

I reordered my patchset and put updated version for your patch 8 into it.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=3db33bb8b94f3b03dfa097eafb00649643cd893a

the whole tree:

	git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-root-bus-hotplug

Thanks

Yinghai

Re: [PATCH v3 4/8] ACPI, PCI: change acpi_pci_drivers' add/remove interface

[Taku Izumi]

On Wed, 19 Sep 2012 16:46:57 -0600
Bjorn Helgaas <bhelgaas@google.com> wrote:

> This should be a separate patch, if you choose to do it :)

 OK. I'll do this later.

-- 
Taku Izumi <izumi.taku@jp.fujitsu.com>

Re: [PATCH v3 6/8] ACPI, PCI: add acpi_pci_roots protection

[Bjorn Helgaas]

On Tue, Sep 18, 2012 at 12:24 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>
> Use mutex to protect acpi_pci_roots list against PCI host bridge
> hotplug operations.
>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/acpi/pci_root.c |   27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
>
> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
> @@ -112,12 +112,17 @@ EXPORT_SYMBOL(acpi_pci_unregister_driver
>  acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
>  {
>         struct acpi_pci_root *root;
> +       struct acpi_handle *handle = NULL;

This should be "acpi_handle handle", not "struct acpi_handle *handle".

> +       mutex_lock(&acpi_pci_root_lock);
>         list_for_each_entry(root, &acpi_pci_roots, node)
>                 if ((root->segment == (u16) seg) &&
> -                   (root->secondary.start == (u16) bus))
> -                       return root->device->handle;
> -       return NULL;
> +                   (root->secondary.start == (u16) bus)) {
> +                       handle = root->device->handle;
> +                       break;
> +               }
> +       mutex_unlock(&acpi_pci_root_lock);
> +       return handle;
>  }

[PATCH] change signature of walk_root_bridge() function

[Taku Izumi]

This patch changes the function signature of walk_root_bridge().

We can omit _STA, _SEG, and _BBN evaluation by passing not acpi_handle 
but acpi_pci_root. Now that acpi_pci_slot_add() which is the only 
caller of walk_root_bridge() gets acpi_pci_root structure, changing
signature of walk_root_bridge() is reasonable.


Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_slot.c |   35 +++++------------------------------
 1 file changed, 5 insertions(+), 30 deletions(-)

Index: Bjorn-next-0903/drivers/acpi/pci_slot.c
===================================================================
--- Bjorn-next-0903.orig/drivers/acpi/pci_slot.c
+++ Bjorn-next-0903/drivers/acpi/pci_slot.c
@@ -233,45 +233,20 @@ out:
 
 /*
  * walk_root_bridge - generic root bridge walker
- * @handle: points to an acpi_pci_root
+ * @root: poiner of an acpi_pci_root
  * @user_function: user callback for slot objects
  *
  * Call user_function for all objects underneath this root bridge.
  * Walk p2p bridges underneath us and call user_function on those too.
  */
 static int
-walk_root_bridge(acpi_handle handle, acpi_walk_callback user_function)
+walk_root_bridge(struct acpi_pci_root *root, acpi_walk_callback user_function)
 {
-	int seg, bus;
-	unsigned long long tmp;
 	acpi_status status;
-	acpi_handle dummy_handle;
-	struct pci_bus *pci_bus;
+	acpi_handle handle = root->device->handle;
+	struct pci_bus *pci_bus = root->bus;
 	struct callback_args context;
 
-	/* 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)) {
-		status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
-		if (ACPI_FAILURE(status)) {
-			info("%s: _STA evaluation failure\n", __func__);
-			return 0;
-		}
-		if ((tmp & ACPI_STA_DEVICE_FUNCTIONING) == 0)
-			/* don't register this object */
-			return 0;
-	}
-
-	status = acpi_evaluate_integer(handle, "_SEG", NULL, &tmp);
-	seg = ACPI_SUCCESS(status) ? tmp : 0;
-
-	status = acpi_evaluate_integer(handle, "_BBN", NULL, &tmp);
-	bus = ACPI_SUCCESS(status) ? tmp : 0;
-
-	pci_bus = pci_find_bus(seg, bus);
-	if (!pci_bus)
-		return 0;
-
 	context.pci_bus = pci_bus;
 	context.user_function = user_function;
 	context.root_handle = handle;
@@ -299,7 +274,7 @@ acpi_pci_slot_add(struct acpi_pci_root *
 {
 	acpi_status status;
 
-	status = walk_root_bridge(root->device->handle, register_slot);
+	status = walk_root_bridge(root, register_slot);
 	if (ACPI_FAILURE(status))
 		err("%s: register_slot failure - %d\n", __func__, status);
 

Re: [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation

[Taku Izumi]

On Wed, 19 Sep 2012 16:03:27 -0600
Bjorn Helgaas <bhelgaas@google.com> wrote:

> On Tue, Sep 18, 2012 at 12:23 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> >
> > This patch changes the implementation of acpi_pci_find_root().
> >
> > We can access acpi_pci_root without scanning acpi_pci_roots list.
> > If hostbridge hotplug is supported, acpi_pci_roots list will be
> > protected by mutex. We should not access acpi_pci_roots list
> > if preventable to lessen deadlock risk.
> >
> > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> > ---
> >  drivers/acpi/pci_root.c |   13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > Index: Bjorn-next-0903/drivers/acpi/pci_root.c
> > ===================================================================
> > --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
> > +++ Bjorn-next-0903/drivers/acpi/pci_root.c
> > @@ -265,12 +265,15 @@ static acpi_status acpi_pci_osc_support(
> >  struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
> >  {
> >         struct acpi_pci_root *root;
> > +       struct acpi_device *device;
> >
> > -       list_for_each_entry(root, &acpi_pci_roots, node) {
> > -               if (root->device->handle == handle)
> > -                       return root;
> > -       }
> > -       return NULL;
> > +       if (acpi_bus_get_device(handle, &device) ||
> > +           acpi_match_device_ids(device, root_device_ids))
> 
> What's the purpose of the acpi_match_device_ids() check?  It's not
> obvious, so worth calling it out in the changelog, and maybe even a
> comment in the code.

  My intention is to reject acpi_handle which doesn't represent 
  hostbrige. I think this is reasonable...

  Best regards,
  Taku Izumi

> 
> Nice to get rid of the list traversal.
> 
> > +               return NULL;
> > +
> > +       root = acpi_driver_data(device);
> > +
> > +       return root;
> >  }
> >  EXPORT_SYMBOL_GPL(acpi_pci_find_root);
> >
> >
> 


-- 
Taku Izumi <izumi.taku@jp.fujitsu.com>

Re: [PATCH] change signature of walk_root_bridge() function

[Bjorn Helgaas]

On Fri, Sep 21, 2012 at 1:03 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>
> This patch changes the function signature of walk_root_bridge().
>
> We can omit _STA, _SEG, and _BBN evaluation by passing not acpi_handle
> but acpi_pci_root. Now that acpi_pci_slot_add() which is the only
> caller of walk_root_bridge() gets acpi_pci_root structure, changing
> signature of walk_root_bridge() is reasonable.
>
>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/acpi/pci_slot.c |   35 +++++------------------------------
>  1 file changed, 5 insertions(+), 30 deletions(-)

This is beautiful!

> Index: Bjorn-next-0903/drivers/acpi/pci_slot.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/acpi/pci_slot.c
> +++ Bjorn-next-0903/drivers/acpi/pci_slot.c
> @@ -233,45 +233,20 @@ out:
>
>  /*
>   * walk_root_bridge - generic root bridge walker
> - * @handle: points to an acpi_pci_root
> + * @root: poiner of an acpi_pci_root
>   * @user_function: user callback for slot objects
>   *
>   * Call user_function for all objects underneath this root bridge.
>   * Walk p2p bridges underneath us and call user_function on those too.
>   */
>  static int
> -walk_root_bridge(acpi_handle handle, acpi_walk_callback user_function)
> +walk_root_bridge(struct acpi_pci_root *root, acpi_walk_callback user_function)
>  {
> -       int seg, bus;
> -       unsigned long long tmp;
>         acpi_status status;
> -       acpi_handle dummy_handle;
> -       struct pci_bus *pci_bus;
> +       acpi_handle handle = root->device->handle;
> +       struct pci_bus *pci_bus = root->bus;
>         struct callback_args context;
>
> -       /* 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)) {
> -               status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
> -               if (ACPI_FAILURE(status)) {
> -                       info("%s: _STA evaluation failure\n", __func__);
> -                       return 0;
> -               }
> -               if ((tmp & ACPI_STA_DEVICE_FUNCTIONING) == 0)
> -                       /* don't register this object */
> -                       return 0;
> -       }
> -
> -       status = acpi_evaluate_integer(handle, "_SEG", NULL, &tmp);
> -       seg = ACPI_SUCCESS(status) ? tmp : 0;
> -
> -       status = acpi_evaluate_integer(handle, "_BBN", NULL, &tmp);
> -       bus = ACPI_SUCCESS(status) ? tmp : 0;
> -
> -       pci_bus = pci_find_bus(seg, bus);
> -       if (!pci_bus)
> -               return 0;
> -
>         context.pci_bus = pci_bus;
>         context.user_function = user_function;
>         context.root_handle = handle;
> @@ -299,7 +274,7 @@ acpi_pci_slot_add(struct acpi_pci_root *
>  {
>         acpi_status status;
>
> -       status = walk_root_bridge(root->device->handle, register_slot);
> +       status = walk_root_bridge(root, register_slot);
>         if (ACPI_FAILURE(status))
>                 err("%s: register_slot failure - %d\n", __func__, status);
>
>

Re: [PATCH v3 5/8] ACPI, PCI: change acpi_pci_find_root implementation

[Bjorn Helgaas]

On Fri, Sep 21, 2012 at 1:14 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> On Wed, 19 Sep 2012 16:03:27 -0600
> Bjorn Helgaas <bhelgaas@google.com> wrote:
>
>> On Tue, Sep 18, 2012 at 12:23 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>> >
>> > This patch changes the implementation of acpi_pci_find_root().
>> >
>> > We can access acpi_pci_root without scanning acpi_pci_roots list.
>> > If hostbridge hotplug is supported, acpi_pci_roots list will be
>> > protected by mutex. We should not access acpi_pci_roots list
>> > if preventable to lessen deadlock risk.
>> >
>> > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>> > ---
>> >  drivers/acpi/pci_root.c |   13 ++++++++-----
>> >  1 file changed, 8 insertions(+), 5 deletions(-)
>> >
>> > Index: Bjorn-next-0903/drivers/acpi/pci_root.c
>> > ===================================================================
>> > --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
>> > +++ Bjorn-next-0903/drivers/acpi/pci_root.c
>> > @@ -265,12 +265,15 @@ static acpi_status acpi_pci_osc_support(
>> >  struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
>> >  {
>> >         struct acpi_pci_root *root;
>> > +       struct acpi_device *device;
>> >
>> > -       list_for_each_entry(root, &acpi_pci_roots, node) {
>> > -               if (root->device->handle == handle)
>> > -                       return root;
>> > -       }
>> > -       return NULL;
>> > +       if (acpi_bus_get_device(handle, &device) ||
>> > +           acpi_match_device_ids(device, root_device_ids))
>>
>> What's the purpose of the acpi_match_device_ids() check?  It's not
>> obvious, so worth calling it out in the changelog, and maybe even a
>> comment in the code.
>
>   My intention is to reject acpi_handle which doesn't represent
>   hostbrige. I think this is reasonable...

Yes, I understand that part.  I was just trying to figure out why that
check is needed.  Is there a path where we can call
acpi_pci_find_root() with a handle that is *not* a PNP0A03 device?  I
looked at all the callers, and as far as I can tell, the supplied
handle is always for a host bridge device.

But I guess I don't object to the check.  This is just another case of
a lookup that in many cases is unnecessary because we should have the
struct acpi_pci_root * available already.

>> Nice to get rid of the list traversal.
>>
>> > +               return NULL;
>> > +
>> > +       root = acpi_driver_data(device);
>> > +
>> > +       return root;
>> >  }
>> >  EXPORT_SYMBOL_GPL(acpi_pci_find_root);
>> >
>> >
>>
>
>
> --
> Taku Izumi <izumi.taku@jp.fujitsu.com>
>

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Bjorn Helgaas]

On Tue, Sep 18, 2012 at 12:25 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> 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.
>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/acpi/pci_bind.c     |    7 +++++++
>  drivers/acpi/pci_root.c     |    7 +++++++
>  drivers/pci/remove.c        |   27 +++++++++++++++++++++++++++
>  include/acpi/acpi_drivers.h |    1 +
>  include/linux/pci.h         |    2 ++
>  5 files changed, 44 insertions(+)
>
> Index: Bjorn-next-0903/drivers/pci/remove.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/pci/remove.c
> +++ Bjorn-next-0903/drivers/pci/remove.c
> @@ -92,3 +92,30 @@ void pci_stop_and_remove_bus_device(stru
>         pci_destroy_dev(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) {
> +               if (dev->subordinate)
> +                       pci_stop_bus_devices(dev->subordinate);
> +               pci_stop_dev(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_stop_and_remove_bus_device(dev);
> +
> +       pci_remove_bus(root);
> +
> +       device_unregister(&bridge->dev);
> +}
> +EXPORT_SYMBOL(pci_remove_host_bridge);
> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
> @@ -652,8 +652,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);
> @@ -661,6 +663,11 @@ 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_irq_del_prt(root->bus);

acpi_pci_irq_del_prt() does not actually have a dependency on the
struct pci_bus, so I think its interface should be changed so it takes
a segment number and a bus number instead of the "struct pci_bus *".
The same applies to acpi_pci_irq_add_prt().

This basically boils down to reverting 859a3f86ca8 and d9efae3688a.  I
acked those changes at the time, but I think they were a mistake.  The
reason is that passing in the struct pci_bus * ties them into the host
bridge add/remove flow in a way that's not necessary.

If we get rid of the struct pci_bus * dependency, then we can easily
add the _PRT before doing PCI enumeration behind the bridge, and we
can remove the _PRT after removing the PCI devices.  I think this is
one small step toward getting rid of the add/start and stop/remove
split.

> +       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-0903/include/linux/pci.h
> ===================================================================
> --- Bjorn-next-0903.orig/include/linux/pci.h
> +++ Bjorn-next-0903/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-0903/drivers/acpi/pci_bind.c
> ===================================================================
> --- Bjorn-next-0903.orig/drivers/acpi/pci_bind.c
> +++ Bjorn-next-0903/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-0903/include/acpi/acpi_drivers.h
> ===================================================================
> --- Bjorn-next-0903.orig/include/acpi/acpi_drivers.h
> +++ Bjorn-next-0903/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 */
>
>

Re: [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

[Yinghai Lu]

On Wed, Sep 19, 2012 at 10:41 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Sep 17, 2012 at 11:26 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>>
>> Devices under hot-added hostbridge have no chance to assign resources
>> and to configure them, so this patch adds such code for hot-added
>> hostbridges at acpi_pci_root_start().
>>
>> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>> ---
>>  drivers/acpi/pci_root.c |   17 +++++++++++++++++
>>  include/acpi/acpi_bus.h |    1 +
>>  2 files changed, 18 insertions(+)
>>
>> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
>> ===================================================================
>> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
>> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
>> @@ -38,6 +38,7 @@
>>  #include <acpi/acpi_bus.h>
>>  #include <acpi/acpi_drivers.h>
>>  #include <acpi/apei.h>
>> +#include <linux/pci_hotplug.h>
>>
>>  #define PREFIX "ACPI: "
>>
>> @@ -636,6 +640,7 @@ static int acpi_pci_root_start(struct ac
>>  {
>>         struct acpi_pci_root *root = acpi_driver_data(device);
>>         struct acpi_pci_driver *driver;
>> +       struct pci_dev *pdev;
>>
>>         mutex_lock(&acpi_pci_root_lock);
>>         list_for_each_entry(driver, &acpi_pci_drivers, node)
>> @@ -643,6 +648,18 @@ static int acpi_pci_root_start(struct ac
>>                         driver->add(root);
>>         mutex_unlock(&acpi_pci_root_lock);
>>
>> +       /*
>> +        * Devices under hot-added hostbridge have no chance to assign
>> +        * resources and to configure them, so do that here
>> +        */
>> +       if (root->hot_added) {
>> +               pci_bus_size_bridges(root->bus);
>> +               pci_bus_assign_resources(root->bus);
>> +               list_for_each_entry(pdev, &root->bus->devices, bus_list)
>> +                       pci_configure_slot(pdev);

one more problem, this one will have problem when CONFIG_HOTPLUG_PCI=m
or not defined.

Also I do not see the point to call pci_config_slot here - my
patcheset did not do that, So how do you figure that out?

or you just need to have mpss set?

Bjorn, You may need to have a patch that will enable
CONFIG_HOTPLUG_PCI just Greg's enabling CONIFG_HOTPLUG forcely.

Yinghai

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Bjorn Helgaas]

On Fri, Sep 21, 2012 at 2:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, Sep 18, 2012 at 12:25 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> 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.
>>
>> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>> ---
>>  drivers/acpi/pci_bind.c     |    7 +++++++
>>  drivers/acpi/pci_root.c     |    7 +++++++
>>  drivers/pci/remove.c        |   27 +++++++++++++++++++++++++++
>>  include/acpi/acpi_drivers.h |    1 +
>>  include/linux/pci.h         |    2 ++
>>  5 files changed, 44 insertions(+)
>>
>> Index: Bjorn-next-0903/drivers/pci/remove.c
>> ===================================================================
>> --- Bjorn-next-0903.orig/drivers/pci/remove.c
>> +++ Bjorn-next-0903/drivers/pci/remove.c
>> @@ -92,3 +92,30 @@ void pci_stop_and_remove_bus_device(stru
>>         pci_destroy_dev(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) {
>> +               if (dev->subordinate)
>> +                       pci_stop_bus_devices(dev->subordinate);
>> +               pci_stop_dev(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_stop_and_remove_bus_device(dev);
>> +
>> +       pci_remove_bus(root);
>> +
>> +       device_unregister(&bridge->dev);
>> +}
>> +EXPORT_SYMBOL(pci_remove_host_bridge);
>> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
>> ===================================================================
>> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
>> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
>> @@ -652,8 +652,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);
>> @@ -661,6 +663,11 @@ 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_irq_del_prt(root->bus);
>
> acpi_pci_irq_del_prt() does not actually have a dependency on the
> struct pci_bus, so I think its interface should be changed so it takes
> a segment number and a bus number instead of the "struct pci_bus *".
> The same applies to acpi_pci_irq_add_prt().
>
> This basically boils down to reverting 859a3f86ca8 and d9efae3688a.  I
> acked those changes at the time, but I think they were a mistake.  The
> reason is that passing in the struct pci_bus * ties them into the host
> bridge add/remove flow in a way that's not necessary.
>
> If we get rid of the struct pci_bus * dependency, then we can easily
> add the _PRT before doing PCI enumeration behind the bridge, and we
> can remove the _PRT after removing the PCI devices.  I think this is
> one small step toward getting rid of the add/start and stop/remove
> split.

I'm going to work on doing this if nobody else is interested.

>> +       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-0903/include/linux/pci.h
>> ===================================================================
>> --- Bjorn-next-0903.orig/include/linux/pci.h
>> +++ Bjorn-next-0903/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-0903/drivers/acpi/pci_bind.c
>> ===================================================================
>> --- Bjorn-next-0903.orig/drivers/acpi/pci_bind.c
>> +++ Bjorn-next-0903/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-0903/include/acpi/acpi_drivers.h
>> ===================================================================
>> --- Bjorn-next-0903.orig/include/acpi/acpi_drivers.h
>> +++ Bjorn-next-0903/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 */
>>
>>

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Yinghai Lu]

On Thu, Sep 27, 2012 at 9:48 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Sep 21, 2012 at 2:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> If we get rid of the struct pci_bus * dependency, then we can easily
>> add the _PRT before doing PCI enumeration behind the bridge, and we
>> can remove the _PRT after removing the PCI devices.  I think this is
>> one small step toward getting rid of the add/start and stop/remove
>> split.
>
> I'm going to work on doing this if nobody else is interested.

I'm not sure if that is needed.

-Yinghai

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Yinghai Lu]

On Thu, Sep 27, 2012 at 10:23 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Sep 27, 2012 at 9:48 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Fri, Sep 21, 2012 at 2:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> If we get rid of the struct pci_bus * dependency, then we can easily
>>> add the _PRT before doing PCI enumeration behind the bridge, and we
>>> can remove the _PRT after removing the PCI devices.  I think this is
>>> one small step toward getting rid of the add/start and stop/remove
>>> split.
>>
>> I'm going to work on doing this if nobody else is interested.
>
> I'm not sure if that is needed.

After patches that is using pci_host_bridge_bus_type and pci_bus_type
notification, we can remove
acpi_pci_root_start.

please check patches in my for-pci-next branch: it include jiang's 7
patches about notifier, and my kill pci_root_buses and etc.

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-next

-Yinghai

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Bjorn Helgaas]

On Thu, Sep 27, 2012 at 11:23 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Sep 27, 2012 at 9:48 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Fri, Sep 21, 2012 at 2:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> If we get rid of the struct pci_bus * dependency, then we can easily
>>> add the _PRT before doing PCI enumeration behind the bridge, and we
>>> can remove the _PRT after removing the PCI devices.  I think this is
>>> one small step toward getting rid of the add/start and stop/remove
>>> split.
>>
>> I'm going to work on doing this if nobody else is interested.
>
> I'm not sure if that is needed.

That's a useless response.  Do you want to elaborate on *why* you
think this is a bad idea?

I explained the reasons why I think it's a good idea above, but just
to expand on it, we currently have to create the struct pci_bus before
we can add _PRT information.   But adding the _PRT info doesn't
actually depend on the struct pci_bus; it only requires the segment
number and the bus number.  We have that information before we scan
the bus .

I think it's useful to disentangle _PRT interface from the specifics
of PCI (in this case, the struct pci_bus).  We're currently using the
struct pci_bus here just as a convenient way to pass around the
segment/bus number, but I don't think it's the appropriate abstraction
for that.

Do you see a technical problem with it?  Even if it's not *necessary*
in order to make host bridge hotplug work, I think it's worth doing to
make the code more understandable.

Do you see a problem with adding the _PRT info before scanning the bus
or with removing it after deleting the bus?  I'd like the bus scan
code to be able to scan/add/bind drivers all at once in the PCI core.
Today I think we have scan/add _PRT/device_add, where we have to do
this _PRT stuff in the middle, so we have to use two PCI interfaces
rather than one.

It's great if you see a way to remove acpi_pci_root_start() -- that
will be tremendous.  I think the current PCI stop/remove split is a
similar issue, and I hope you can dream up a way to consolidate those,
too.

Bjorn

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Yinghai Lu]

On Thu, Sep 27, 2012 at 11:44 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>
>> I'm not sure if that is needed.
>
> That's a useless response.  Do you want to elaborate on *why* you
> think this is a bad idea?

I did not say that is bad idea.

it is just not needed to waste your time.

>
> I explained the reasons why I think it's a good idea above, but just
> to expand on it, we currently have to create the struct pci_bus before
> we can add _PRT information.   But adding the _PRT info doesn't
> actually depend on the struct pci_bus; it only requires the segment
> number and the bus number.  We have that information before we scan
> the bus .
>
> I think it's useful to disentangle _PRT interface from the specifics
> of PCI (in this case, the struct pci_bus).  We're currently using the
> struct pci_bus here just as a convenient way to pass around the
> segment/bus number, but I don't think it's the appropriate abstraction
> for that.
>
> Do you see a technical problem with it?  Even if it's not *necessary*
> in order to make host bridge hotplug work, I think it's worth doing to
> make the code more understandable.
>
> Do you see a problem with adding the _PRT info before scanning the bus
> or with removing it after deleting the bus?

If the bus is not there, do not need that prt.

So if you find the prt and add it to the list before, later if
scanning fail etc failing path
you will need to clean that prt in the list.


> I'd like the bus scan
> code to be able to scan/add/bind drivers all at once in the PCI core.
> Today I think we have scan/add _PRT/device_add, where we have to do
> this _PRT stuff in the middle, so we have to use two PCI interfaces
> rather than one.

looks like we handle _PRT in pci_host_bridge add/remove notification,
just like Jiang's put that for pci device bus notifier.

>
> It's great if you see a way to remove acpi_pci_root_start() -- that
> will be tremendous.

that will need to more test.

> I think the current PCI stop/remove split is a
> similar issue, and I hope you can dream up a way to consolidate those,
> too.

that is SRIOV handling resign.

I would suggest following way:
use pf as parent for one new domain. and put all VFs on that domain.
and the domain could have 256 buses for those VFs.
and we have 16 bits domain number that could be used.

current got too many patches in my quilt... hate to keep rebasing again.

after pci root bus hotplug and for_each_res topic, will work on that SRIOV one.

-Yinghai

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Taku Izumi]

On Thu, 27 Sep 2012 10:48:09 -0600
Bjorn Helgaas <bhelgaas@google.com> wrote:

> On Fri, Sep 21, 2012 at 2:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Tue, Sep 18, 2012 at 12:25 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> 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.
> >>
> >> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> >> ---
> >>  drivers/acpi/pci_bind.c     |    7 +++++++
> >>  drivers/acpi/pci_root.c     |    7 +++++++
> >>  drivers/pci/remove.c        |   27 +++++++++++++++++++++++++++
> >>  include/acpi/acpi_drivers.h |    1 +
> >>  include/linux/pci.h         |    2 ++
> >>  5 files changed, 44 insertions(+)
> >>
> >> Index: Bjorn-next-0903/drivers/pci/remove.c
> >> ===================================================================
> >> --- Bjorn-next-0903.orig/drivers/pci/remove.c
> >> +++ Bjorn-next-0903/drivers/pci/remove.c
> >> @@ -92,3 +92,30 @@ void pci_stop_and_remove_bus_device(stru
> >>         pci_destroy_dev(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) {
> >> +               if (dev->subordinate)
> >> +                       pci_stop_bus_devices(dev->subordinate);
> >> +               pci_stop_dev(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_stop_and_remove_bus_device(dev);
> >> +
> >> +       pci_remove_bus(root);
> >> +
> >> +       device_unregister(&bridge->dev);
> >> +}
> >> +EXPORT_SYMBOL(pci_remove_host_bridge);
> >> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
> >> ===================================================================
> >> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
> >> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
> >> @@ -652,8 +652,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);
> >> @@ -661,6 +663,11 @@ 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_irq_del_prt(root->bus);
> >
> > acpi_pci_irq_del_prt() does not actually have a dependency on the
> > struct pci_bus, so I think its interface should be changed so it takes
> > a segment number and a bus number instead of the "struct pci_bus *".
> > The same applies to acpi_pci_irq_add_prt().
> >
> > This basically boils down to reverting 859a3f86ca8 and d9efae3688a.  I
> > acked those changes at the time, but I think they were a mistake.  The
> > reason is that passing in the struct pci_bus * ties them into the host
> > bridge add/remove flow in a way that's not necessary.
> >
> > If we get rid of the struct pci_bus * dependency, then we can easily
> > add the _PRT before doing PCI enumeration behind the bridge, and we
> > can remove the _PRT after removing the PCI devices.  I think this is
> > one small step toward getting rid of the add/start and stop/remove
> > split.
> 
> I'm going to work on doing this if nobody else is interested.

  I'll do that. Maybee that is separeted from this patchset.

  Best regards,
  Taku Izumi

> 
> >> +       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-0903/include/linux/pci.h
> >> ===================================================================
> >> --- Bjorn-next-0903.orig/include/linux/pci.h
> >> +++ Bjorn-next-0903/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-0903/drivers/acpi/pci_bind.c
> >> ===================================================================
> >> --- Bjorn-next-0903.orig/drivers/acpi/pci_bind.c
> >> +++ Bjorn-next-0903/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-0903/include/acpi/acpi_drivers.h
> >> ===================================================================
> >> --- Bjorn-next-0903.orig/include/acpi/acpi_drivers.h
> >> +++ Bjorn-next-0903/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 */
> >>
> >>
> 


-- 
Taku Izumi <izumi.taku@jp.fujitsu.com>

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Bjorn Helgaas]

On Thu, Sep 27, 2012 at 6:15 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> On Thu, 27 Sep 2012 10:48:09 -0600
> Bjorn Helgaas <bhelgaas@google.com> wrote:
>
>> On Fri, Sep 21, 2012 at 2:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> > On Tue, Sep 18, 2012 at 12:25 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> 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.
>> >>
>> >> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>> >> ---
>> >>  drivers/acpi/pci_bind.c     |    7 +++++++
>> >>  drivers/acpi/pci_root.c     |    7 +++++++
>> >>  drivers/pci/remove.c        |   27 +++++++++++++++++++++++++++
>> >>  include/acpi/acpi_drivers.h |    1 +
>> >>  include/linux/pci.h         |    2 ++
>> >>  5 files changed, 44 insertions(+)
>> >>
>> >> Index: Bjorn-next-0903/drivers/pci/remove.c
>> >> ===================================================================
>> >> --- Bjorn-next-0903.orig/drivers/pci/remove.c
>> >> +++ Bjorn-next-0903/drivers/pci/remove.c
>> >> @@ -92,3 +92,30 @@ void pci_stop_and_remove_bus_device(stru
>> >>         pci_destroy_dev(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) {
>> >> +               if (dev->subordinate)
>> >> +                       pci_stop_bus_devices(dev->subordinate);
>> >> +               pci_stop_dev(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_stop_and_remove_bus_device(dev);
>> >> +
>> >> +       pci_remove_bus(root);
>> >> +
>> >> +       device_unregister(&bridge->dev);
>> >> +}
>> >> +EXPORT_SYMBOL(pci_remove_host_bridge);
>> >> Index: Bjorn-next-0903/drivers/acpi/pci_root.c
>> >> ===================================================================
>> >> --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
>> >> +++ Bjorn-next-0903/drivers/acpi/pci_root.c
>> >> @@ -652,8 +652,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);
>> >> @@ -661,6 +663,11 @@ 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_irq_del_prt(root->bus);
>> >
>> > acpi_pci_irq_del_prt() does not actually have a dependency on the
>> > struct pci_bus, so I think its interface should be changed so it takes
>> > a segment number and a bus number instead of the "struct pci_bus *".
>> > The same applies to acpi_pci_irq_add_prt().
>> >
>> > This basically boils down to reverting 859a3f86ca8 and d9efae3688a.  I
>> > acked those changes at the time, but I think they were a mistake.  The
>> > reason is that passing in the struct pci_bus * ties them into the host
>> > bridge add/remove flow in a way that's not necessary.
>> >
>> > If we get rid of the struct pci_bus * dependency, then we can easily
>> > add the _PRT before doing PCI enumeration behind the bridge, and we
>> > can remove the _PRT after removing the PCI devices.  I think this is
>> > one small step toward getting rid of the add/start and stop/remove
>> > split.
>>
>> I'm going to work on doing this if nobody else is interested.
>
>   I'll do that. Maybee that is separeted from this patchset.

Great, thanks!  A separate patchset is definitely fine.

>> >> +       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-0903/include/linux/pci.h
>> >> ===================================================================
>> >> --- Bjorn-next-0903.orig/include/linux/pci.h
>> >> +++ Bjorn-next-0903/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-0903/drivers/acpi/pci_bind.c
>> >> ===================================================================
>> >> --- Bjorn-next-0903.orig/drivers/acpi/pci_bind.c
>> >> +++ Bjorn-next-0903/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-0903/include/acpi/acpi_drivers.h
>> >> ===================================================================
>> >> --- Bjorn-next-0903.orig/include/acpi/acpi_drivers.h
>> >> +++ Bjorn-next-0903/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 */
>> >>
>> >>
>>
>
>
> --
> Taku Izumi <izumi.taku@jp.fujitsu.com>
>

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Taku Izumi]

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);
+
+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 */
 

Re: [PATCH v3 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

[Taku Izumi]

Devices under hot-added hostbridge have no chance to assign resources
and to configure them, so this patch adds such code for hot-added 
hostbridges at acpi_pci_root_start().

diff: updated according to Yinhai's comment
      disconnect dependency on CONFIG_HOTPLUG
      

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   13 +++++++++++++
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 14 insertions(+)

Index: Bjorn-next-0925/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0925.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0925/drivers/acpi/pci_root.c
@@ -628,6 +628,9 @@ static int __devinit acpi_pci_root_add(s
 	if (device->wakeup.flags.run_wake)
 		device_set_run_wake(root->bus->bridge, true);
 
+	if (system_state != SYSTEM_BOOTING)
+		root->hot_added = true;
+
 	return 0;
 
 out_del_root:
@@ -644,6 +647,16 @@ static int acpi_pci_root_start(struct ac
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	/*
+	 * Devices under hot-added hostbridge have no chance to assign
+	 * resources and to configure them, so do that here
+	 */
+	if (root->hot_added) {
+		pci_bus_size_bridges(root->bus);
+		pci_bus_assign_resources(root->bus);
+		pci_enable_bridges(root->bus);
+	}
+
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
Index: Bjorn-next-0925/include/acpi/acpi_bus.h
===================================================================
--- Bjorn-next-0925.orig/include/acpi/acpi_bus.h
+++ Bjorn-next-0925/include/acpi/acpi_bus.h
@@ -407,6 +407,7 @@ struct acpi_pci_root {
 	u32 osc_support_set;	/* _OSC state of support bits */
 	u32 osc_control_set;	/* _OSC state of control bits */
 	phys_addr_t mcfg_addr;
+	bool hot_added;
 };
 
 /* helper */

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Bjorn Helgaas]

On Thu, Sep 27, 2012 at 6:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Sep 27, 2012 at 6:15 PM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>> On Thu, 27 Sep 2012 10:48:09 -0600
>> Bjorn Helgaas <bhelgaas@google.com> wrote:
>>
>>> On Fri, Sep 21, 2012 at 2:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> > On Tue, Sep 18, 2012 at 12:25 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:

>>> >> @@ -661,6 +663,11 @@ 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_irq_del_prt(root->bus);
>>> >
>>> > acpi_pci_irq_del_prt() does not actually have a dependency on the
>>> > struct pci_bus, so I think its interface should be changed so it takes
>>> > a segment number and a bus number instead of the "struct pci_bus *".
>>> > The same applies to acpi_pci_irq_add_prt().
>>> >
>>> > This basically boils down to reverting 859a3f86ca8 and d9efae3688a.  I
>>> > acked those changes at the time, but I think they were a mistake.  The
>>> > reason is that passing in the struct pci_bus * ties them into the host
>>> > bridge add/remove flow in a way that's not necessary.
>>> >
>>> > If we get rid of the struct pci_bus * dependency, then we can easily
>>> > add the _PRT before doing PCI enumeration behind the bridge, and we
>>> > can remove the _PRT after removing the PCI devices.  I think this is
>>> > one small step toward getting rid of the add/start and stop/remove
>>> > split.
>>>
>>> I'm going to work on doing this if nobody else is interested.
>>
>>   I'll do that. Maybee that is separeted from this patchset.
>
> Great, thanks!  A separate patchset is definitely fine.

Whether this is a waste of time or not, I don't know.  But I did a
little bit of work on this before you volunteered, Taku.  Sorry, I
should have pointed you at the work-in-progress patches; maybe they
could have saved you a bit of time.  Anyway, here they are:

http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=shortlog;h=refs/heads/pci/bjorn-misc

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Bjorn Helgaas]

On Thu, Sep 27, 2012 at 2:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Sep 27, 2012 at 11:44 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:

>> I explained the reasons why I think it's a good idea above, but just
>> to expand on it, we currently have to create the struct pci_bus before
>> we can add _PRT information.   But adding the _PRT info doesn't
>> actually depend on the struct pci_bus; it only requires the segment
>> number and the bus number.  We have that information before we scan
>> the bus .
>>
>> I think it's useful to disentangle _PRT interface from the specifics
>> of PCI (in this case, the struct pci_bus).  We're currently using the
>> struct pci_bus here just as a convenient way to pass around the
>> segment/bus number, but I don't think it's the appropriate abstraction
>> for that.
>>
>> Do you see a technical problem with it?  Even if it's not *necessary*
>> in order to make host bridge hotplug work, I think it's worth doing to
>> make the code more understandable.
>>
>> Do you see a problem with adding the _PRT info before scanning the bus
>> or with removing it after deleting the bus?
>
> If the bus is not there, do not need that prt.
>
> So if you find the prt and add it to the list before, later if
> scanning fail etc failing path
> you will need to clean that prt in the list.

That's true.  It will add a little bit to the failure paths in
acpi_pci_root_add(), but I don't think that's much of an issue.

>> I'd like the bus scan
>> code to be able to scan/add/bind drivers all at once in the PCI core.
>> Today I think we have scan/add _PRT/device_add, where we have to do
>> this _PRT stuff in the middle, so we have to use two PCI interfaces
>> rather than one.

This is the more important bit.  My longer-term goal is to separate
out the ACPI parts from the PCI parts.  Then we can use more generic
code for the PCI part, which will help unify the architectures.

Today we have this, which is more complicated than it should be.  Note
how we do some ACPI stuff, some PCI stuff, some more ACPI stuff, then
more PCI stuff:

    acpi_pci_root_add
        pci_acpi_scan_root
            pci_scan_child_bus
        acpi_pci_irq_add_prt
        acpi_pci_osc_control_set
    acpi_pci_root_start
        pci_bus_add_devices

I don't think the ACPI/PCI mixture is anything essential dictated by
the way the hardware or firmware works.  I think it's just an artifact
of the current design, and it could be changed.  It would be better to
have this:

    acpi_pci_root_add
        acpi_pci_irq_add_prt
        acpi_pci_osc_control_set
        pci_acpi_scan_root
            pci_scan_root_bus
                pci_scan_child_bus
                pci_bus_add_devices

We can't get to this latter strategy as long as the ACPI interfaces
depend on the struct pci_bus.  So the _PRT change is a small thing in
itself, but I do think it helps enable significant improvements in the
future.

Bjorn

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Yinghai Lu]

On Fri, Sep 28, 2012 at 9:07 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Sep 27, 2012 at 2:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Today we have this, which is more complicated than it should be.  Note
> how we do some ACPI stuff, some PCI stuff, some more ACPI stuff, then
> more PCI stuff:
>
>     acpi_pci_root_add
>         pci_acpi_scan_root
>             pci_scan_child_bus
>         acpi_pci_irq_add_prt
>         acpi_pci_osc_control_set
>     acpi_pci_root_start
>         pci_bus_add_devices
>
> I don't think the ACPI/PCI mixture is anything essential dictated by
> the way the hardware or firmware works.  I think it's just an artifact
> of the current design, and it could be changed.  It would be better to
> have this:
>
>     acpi_pci_root_add
>         acpi_pci_irq_add_prt
>         acpi_pci_osc_control_set
>         pci_acpi_scan_root
>             pci_scan_root_bus
>                 pci_scan_child_bus
>                 pci_bus_add_devices
>
> We can't get to this latter strategy as long as the ACPI interfaces
> depend on the struct pci_bus.  So the _PRT change is a small thing in
> itself, but I do think it helps enable significant improvements in the
> future.

still to handle to those fallback path like create_bus and scan bus failure.

in my for-pci-next branch, with Jiang's patches and mine, now we achieved at

    acpi_pci_root_add
        pci_acpi_scan_root
            pci_scan_root_bus
                pci_scan_child_bus
        acpi_pci_osc_control_set
        pci_bus_add_devices

acpi_pci_irq_add_prt is called later during acpi binding that is
triggered by adding to device tree.
thought os_control set via pci_host_bridge add interface..

with those BUS ADD notification, we can pass bus safely, and without
considering about cleanup PRT and OSC setting.

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Bjorn Helgaas]

On Fri, Sep 28, 2012 at 10:19 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 28, 2012 at 9:07 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Thu, Sep 27, 2012 at 2:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Today we have this, which is more complicated than it should be.  Note
>> how we do some ACPI stuff, some PCI stuff, some more ACPI stuff, then
>> more PCI stuff:
>>
>>     acpi_pci_root_add
>>         pci_acpi_scan_root
>>             pci_scan_child_bus
>>         acpi_pci_irq_add_prt
>>         acpi_pci_osc_control_set
>>     acpi_pci_root_start
>>         pci_bus_add_devices
>>
>> I don't think the ACPI/PCI mixture is anything essential dictated by
>> the way the hardware or firmware works.  I think it's just an artifact
>> of the current design, and it could be changed.  It would be better to
>> have this:
>>
>>     acpi_pci_root_add
>>         acpi_pci_irq_add_prt
>>         acpi_pci_osc_control_set
>>         pci_acpi_scan_root
>>             pci_scan_root_bus
>>                 pci_scan_child_bus
>>                 pci_bus_add_devices
>>
>> We can't get to this latter strategy as long as the ACPI interfaces
>> depend on the struct pci_bus.  So the _PRT change is a small thing in
>> itself, but I do think it helps enable significant improvements in the
>> future.
>
> still to handle to those fallback path like create_bus and scan bus failure.
>
> in my for-pci-next branch, with Jiang's patches and mine, now we achieved at
>
>     acpi_pci_root_add
>         pci_acpi_scan_root
>             pci_scan_root_bus
>                 pci_scan_child_bus
>         acpi_pci_osc_control_set
>         pci_bus_add_devices
>
> acpi_pci_irq_add_prt is called later during acpi binding that is
> triggered by adding to device tree.
> thought os_control set via pci_host_bridge add interface..
>
> with those BUS ADD notification, we can pass bus safely, and without
> considering about cleanup PRT and OSC setting.

I haven't looked at those patches yet.

Is there a reason why acpi_pci_osc_control_set() needs to be done
after pci_scan_child_bus()?  The argument that it might make the error
path somewhat simpler is not very convincing to me.  Having the arch
code call both pci_scan_child_bus() and pci_bus_add_devices() is a
much more fundamental complexity -- it makes x86 and ia64 different
from many architectures, and it exposes the intermediate state where
"devices have been enumerated but not added" to a lot more code.

It doesn't sound like an improvement to call acpi_pci_irq_add_prt()
using a bus add notifier.  At least for the host bridge case, it's
clear, simple, and straightforward to call it in acpi_pci_root_add().
Notifiers are useful in some cases, but they definitely make the code
harder to follow.

Re: [PATCH v3 7/8] ACPI, PCI: add hostbridge removal function

[Bjorn Helgaas]

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 */
>  
> 

(no subject)

[Yinghai Lu]

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

[PATCH 1/8] PCI: Separate out pci_assign_unassigned_bus_resources()

[Yinghai Lu]

It is main portion of pci_rescan_bus().

Separate it out and prepare to use it for pci root bus hot add later.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/setup-bus.c |   32 ++++++++++++++++++--------------
 include/linux/pci.h     |    1 +
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 1e808ca..1f34929 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1550,25 +1550,12 @@ enable_all:
 }
 EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
 
-#ifdef CONFIG_HOTPLUG
-/**
- * pci_rescan_bus - scan a PCI bus for devices.
- * @bus: PCI bus to scan
- *
- * Scan a PCI bus and child buses for new devices, adds them,
- * and enables them.
- *
- * Returns the max number of subordinate bus discovered.
- */
-unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
 {
-	unsigned int max;
 	struct pci_dev *dev;
 	LIST_HEAD(add_list); /* list of resources that
 					want additional resources */
 
-	max = pci_scan_child_bus(bus);
-
 	down_read(&pci_bus_sem);
 	list_for_each_entry(dev, &bus->devices, bus_list)
 		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
@@ -1581,6 +1568,23 @@ unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
 	BUG_ON(!list_empty(&add_list));
 
 	pci_enable_bridges(bus);
+}
+#ifdef CONFIG_HOTPLUG
+/**
+ * pci_rescan_bus - scan a PCI bus for devices.
+ * @bus: PCI bus to scan
+ *
+ * Scan a PCI bus and child buses for new devices, adds them,
+ * and enables them.
+ *
+ * Returns the max number of subordinate bus discovered.
+ */
+unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+{
+	unsigned int max;
+
+	max = pci_scan_child_bus(bus);
+	pci_assign_unassigned_bus_resources(bus);
 	pci_bus_add_devices(bus);
 
 	return max;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index be1de01..505c05a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -980,6 +980,7 @@ void pci_bus_size_bridges(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);
+void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
 void pdev_enable_device(struct pci_dev *);
 int pci_enable_resources(struct pci_dev *, int mask);
 void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
-- 
1.7.7

[PATCH 2/8] PCI: Move pci_rescan_bus() back to probe.c

[Yinghai Lu]

We have pci_assign_unassigned_bus_resources() in as global function now.

Move back pci_rescan_bus to probe.c where it should be.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c     |   21 +++++++++++++++++++++
 drivers/pci/setup-bus.c |   22 ----------------------
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ec909af..65f62e3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1890,6 +1890,27 @@ unsigned int __ref pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
 	return max;
 }
 
+/**
+ * pci_rescan_bus - scan a PCI bus for devices.
+ * @bus: PCI bus to scan
+ *
+ * Scan a PCI bus and child buses for new devices, adds them,
+ * and enables them.
+ *
+ * Returns the max number of subordinate bus discovered.
+ */
+unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+{
+	unsigned int max;
+
+	max = pci_scan_child_bus(bus);
+	pci_assign_unassigned_bus_resources(bus);
+	pci_bus_add_devices(bus);
+
+	return max;
+}
+EXPORT_SYMBOL_GPL(pci_rescan_bus);
+
 EXPORT_SYMBOL(pci_add_new_bus);
 EXPORT_SYMBOL(pci_scan_slot);
 EXPORT_SYMBOL(pci_scan_bridge);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 1f34929..59e6c55 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1569,25 +1569,3 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
 
 	pci_enable_bridges(bus);
 }
-#ifdef CONFIG_HOTPLUG
-/**
- * pci_rescan_bus - scan a PCI bus for devices.
- * @bus: PCI bus to scan
- *
- * Scan a PCI bus and child buses for new devices, adds them,
- * and enables them.
- *
- * Returns the max number of subordinate bus discovered.
- */
-unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
-{
-	unsigned int max;
-
-	max = pci_scan_child_bus(bus);
-	pci_assign_unassigned_bus_resources(bus);
-	pci_bus_add_devices(bus);
-
-	return max;
-}
-EXPORT_SYMBOL_GPL(pci_rescan_bus);
-#endif
-- 
1.7.7

[PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res

[Yinghai Lu]

So could use assign_unassigned_bus_res pci root bus add

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c     |    1 +
 drivers/pci/setup-bus.c |    2 --
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 65f62e3..59cf1ba 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1905,6 +1905,7 @@ unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
 
 	max = pci_scan_child_bus(bus);
 	pci_assign_unassigned_bus_resources(bus);
+	pci_enable_bridges(bus);
 	pci_bus_add_devices(bus);
 
 	return max;
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 59e6c55..6d3591d 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1566,6 +1566,4 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
 	up_read(&pci_bus_sem);
 	__pci_bus_assign_resources(bus, &add_list, NULL);
 	BUG_ON(!list_empty(&add_list));
-
-	pci_enable_bridges(bus);
 }
-- 
1.7.7

[PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus

[Yinghai Lu]

After we get hot-added ioapic registered.
pci_enable_bridges will try to enable ioapic irq for pci bridges.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/acpi/pci_root.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index bce469c..27adbfd 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -644,12 +644,19 @@ static int acpi_pci_root_start(struct acpi_device *device)
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	if (system_state != SYSTEM_BOOTING)
+		pci_assign_unassigned_bus_resources(root->bus);
+
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
 			driver->add(root);
 	mutex_unlock(&acpi_pci_root_lock);
 
+	/* need to after hot-added ioapic is registered */
+	if (system_state != SYSTEM_BOOTING)
+		pci_enable_bridges(root->bus);
+
 	pci_bus_add_devices(root->bus);
 
 	return 0;
-- 
1.7.7

[PATCH 5/8] PCI: Add pci_stop/remove_root_bus()

[Yinghai Lu]

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

-v2: clear pci_bridge's subordinate.
-v3: only handle root bus. and also put Jiang's get/put pair in
-v4: fold pci_stop/remove_bus_devices in... reducing confusing.
-v5: split device_register/unregister to avoid extra get...
     also remove extra blank line.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/remove.c |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/pci.h  |    2 ++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 513972f..7c0fd92 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -111,3 +111,39 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
 	pci_remove_bus_device(dev);
 }
 EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
+
+void pci_stop_root_bus(struct pci_bus *bus)
+{
+	struct pci_dev *child, *tmp;
+	struct pci_host_bridge *host_bridge;
+
+	if (!pci_is_root_bus(bus))
+		return;
+
+	host_bridge = to_pci_host_bridge(bus->bridge);
+	list_for_each_entry_safe_reverse(child, tmp,
+					 &bus->devices, bus_list)
+		pci_stop_bus_device(child);
+
+	/* stop the host bridge */
+	device_del(&host_bridge->dev);
+}
+
+void pci_remove_root_bus(struct pci_bus *bus)
+{
+	struct pci_dev *child, *tmp;
+	struct pci_host_bridge *host_bridge;
+
+	if (!pci_is_root_bus(bus))
+		return;
+
+	host_bridge = to_pci_host_bridge(bus->bridge);
+	list_for_each_entry_safe(child, tmp,
+				 &bus->devices, bus_list)
+		pci_remove_bus_device(child);
+	pci_remove_bus(bus);
+	host_bridge->bus = NULL;
+
+	/* remove the host bridge */
+	put_device(&host_bridge->dev);
+}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 505c05a..a5cd03b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -734,6 +734,8 @@ extern struct pci_dev *pci_dev_get(struct pci_dev *dev);
 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);
+void pci_stop_root_bus(struct pci_bus *bus);
+void pci_remove_root_bus(struct pci_bus *bus);
 void pci_setup_cardbus(struct pci_bus *bus);
 extern void pci_sort_breadthfirst(void);
 #define dev_is_pci(d) ((d)->bus == &pci_bus_type)
-- 
1.7.7

[PATCH 6/8] PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus

[Yinghai Lu]

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

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/pci_root.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 27adbfd..aed0953 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -667,14 +667,20 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	pci_stop_root_bus(root->bus);
+
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
 			driver->remove(root);
+	mutex_unlock(&acpi_pci_root_lock);
 
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
+	pci_remove_root_bus(root->bus);
+
+	mutex_lock(&acpi_pci_root_lock);
 	list_del(&root->node);
 	mutex_unlock(&acpi_pci_root_lock);
 	kfree(root);
-- 
1.7.7

[PATCH 7/8] PCI, ACPI: delete root bus prt during hot remove path

[Yinghai Lu]

corresponding to add path.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/acpi/pci_root.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index aed0953..a27cbb57 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -664,6 +664,8 @@ static int acpi_pci_root_start(struct acpi_device *device)
 
 static int acpi_pci_root_remove(struct acpi_device *device, int type)
 {
+	acpi_status status;
+	acpi_handle handle;
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
@@ -678,6 +680,10 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
+	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
+	if (ACPI_SUCCESS(status))
+		acpi_pci_irq_del_prt(root->bus);
+
 	pci_remove_root_bus(root->bus);
 
 	mutex_lock(&acpi_pci_root_lock);
-- 
1.7.7

[PATCH 8/8] PCI, ACPI: remove acpi_root_driver in reserse order

[Yinghai Lu]

aka stop acpi root driver in this sequence: acpiphp, iommu, ioapic.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/acpi/pci_root.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index a27cbb57..012f40d 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -672,7 +672,7 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 	pci_stop_root_bus(root->bus);
 
 	mutex_lock(&acpi_pci_root_lock);
-	list_for_each_entry(driver, &acpi_pci_drivers, node)
+	list_for_each_entry_reverse(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
 			driver->remove(root);
 	mutex_unlock(&acpi_pci_root_lock);
-- 
1.7.7

[PATCH v3,RESEND 7/8] ACPI, PCI: add hostbridge removal function

[Taku Izumi]

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);
+
+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 */
 

[PATCH v3,RESEND 8/8] ACPI, PCI: add resoruce-assign code for devices under hot-added hostbridge

[Taku Izumi]

Devices under hot-added hostbridge have no chance to assign resources
and to configure them, so this patch adds such code for hot-added 
hostbridges at acpi_pci_root_start().

diff: updated according to Yinhai's comment
      disconnect dependency on CONFIG_HOTPLUG
      

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/acpi/pci_root.c |   13 +++++++++++++
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 14 insertions(+)

Index: Bjorn-next-0925/drivers/acpi/pci_root.c
===================================================================
--- Bjorn-next-0925.orig/drivers/acpi/pci_root.c
+++ Bjorn-next-0925/drivers/acpi/pci_root.c
@@ -628,6 +628,9 @@ static int __devinit acpi_pci_root_add(s
 	if (device->wakeup.flags.run_wake)
 		device_set_run_wake(root->bus->bridge, true);
 
+	if (system_state != SYSTEM_BOOTING)
+		root->hot_added = true;
+
 	return 0;
 
 out_del_root:
@@ -644,6 +647,16 @@ static int acpi_pci_root_start(struct ac
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	/*
+	 * Devices under hot-added hostbridge have no chance to assign
+	 * resources and to configure them, so do that here
+	 */
+	if (root->hot_added) {
+		pci_bus_size_bridges(root->bus);
+		pci_bus_assign_resources(root->bus);
+		pci_enable_bridges(root->bus);
+	}
+
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
Index: Bjorn-next-0925/include/acpi/acpi_bus.h
===================================================================
--- Bjorn-next-0925.orig/include/acpi/acpi_bus.h
+++ Bjorn-next-0925/include/acpi/acpi_bus.h
@@ -407,6 +407,7 @@ struct acpi_pci_root {
 	u32 osc_support_set;	/* _OSC state of support bits */
 	u32 osc_control_set;	/* _OSC state of control bits */
 	phys_addr_t mcfg_addr;
+	bool hot_added;
 };
 
 /* helper */

Re:

[Rafael J. Wysocki]

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


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

Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res

[Taku Izumi]

On Tue, 30 Oct 2012 10:42:40 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> So could use assign_unassigned_bus_res pci root bus add

  This description doesn't match the following patch...
  This is for 4/8?

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c     |    1 +
>  drivers/pci/setup-bus.c |    2 --
>  2 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 65f62e3..59cf1ba 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1905,6 +1905,7 @@ unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
>  
>  	max = pci_scan_child_bus(bus);
>  	pci_assign_unassigned_bus_resources(bus);
> +	pci_enable_bridges(bus);
>  	pci_bus_add_devices(bus);
>  
>  	return max;
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 59e6c55..6d3591d 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1566,6 +1566,4 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
>  	up_read(&pci_bus_sem);
>  	__pci_bus_assign_resources(bus, &add_list, NULL);
>  	BUG_ON(!list_empty(&add_list));
> -
> -	pci_enable_bridges(bus);
>  }
> -- 
> 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
> 


-- 
Taku Izumi <izumi.taku@jp.fujitsu.com>

Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res

[Yinghai Lu]

On Fri, Nov 2, 2012 at 3:02 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> On Tue, 30 Oct 2012 10:42:40 -0700
> Yinghai Lu <yinghai@kernel.org> wrote:
>
>> So could use assign_unassigned_bus_res pci root bus add
>
>   This description doesn't match the following patch...
>   This is for 4/8?

yes, this patch is preparation one for 4/8.

[PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Yinghai Lu]

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(-)

-- 
1.7.7

[PATCH 1/8] PCI, x86: Separate out pcibios_allocate_bridge_resources()

[Yinghai Lu]

Thus pcibios_allocate_bus_resources() could more simple and clean.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: x86@kernel.org
---
 arch/x86/pci/i386.c |   46 ++++++++++++++++++++++++----------------------
 1 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index dd8ca6f..9800362 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -193,34 +193,36 @@ EXPORT_SYMBOL(pcibios_align_resource);
  *	    as well.
  */
 
-static void __init pcibios_allocate_bus_resources(struct list_head *bus_list)
+static void __init pcibios_allocate_bridge_resources(struct pci_dev *dev)
 {
-	struct pci_bus *bus;
-	struct pci_dev *dev;
 	int idx;
 	struct resource *r;
 
+	for (idx = PCI_BRIDGE_RESOURCES; idx < PCI_NUM_RESOURCES; idx++) {
+		r = &dev->resource[idx];
+		if (!r->flags)
+			continue;
+		if (!r->start || pci_claim_resource(dev, idx) < 0) {
+			/*
+			 * Something is wrong with the region.
+			 * Invalidate the resource to prevent
+			 * child resource allocations in this
+			 * range.
+			 */
+			r->start = r->end = 0;
+			r->flags = 0;
+		}
+	}
+}
+
+static void __init pcibios_allocate_bus_resources(struct list_head *bus_list)
+{
+	struct pci_bus *bus;
+
 	/* Depth-First Search on bus tree */
 	list_for_each_entry(bus, bus_list, node) {
-		if ((dev = bus->self)) {
-			for (idx = PCI_BRIDGE_RESOURCES;
-			    idx < PCI_NUM_RESOURCES; idx++) {
-				r = &dev->resource[idx];
-				if (!r->flags)
-					continue;
-				if (!r->start ||
-				    pci_claim_resource(dev, idx) < 0) {
-					/*
-					 * Something is wrong with the region.
-					 * Invalidate the resource to prevent
-					 * child resource allocations in this
-					 * range.
-					 */
-					r->start = r->end = 0;
-					r->flags = 0;
-				}
-			}
-		}
+		if (bus->self)
+			pcibios_allocate_bridge_resources(bus->self);
 		pcibios_allocate_bus_resources(&bus->children);
 	}
 }
-- 
1.7.7

[PATCH 2/8] PCI, x86: Separate out pcibios_allocate_dev_resources()

[Yinghai Lu]

Thus pcibios_allocate_resources() could more simple and clean.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: x86@kernel.org
---
 arch/x86/pci/i386.c |   42 +++++++++++++++++++++++-------------------
 1 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 9800362..5817cf2 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -232,9 +232,8 @@ struct pci_check_idx_range {
 	int end;
 };
 
-static void __init pcibios_allocate_resources(int pass)
+static void __init pcibios_allocate_dev_resources(struct pci_dev *dev, int pass)
 {
-	struct pci_dev *dev = NULL;
 	int idx, disabled, i;
 	u16 command;
 	struct resource *r;
@@ -246,14 +245,13 @@ static void __init pcibios_allocate_resources(int pass)
 #endif
 	};
 
-	for_each_pci_dev(dev) {
-		pci_read_config_word(dev, PCI_COMMAND, &command);
-		for (i = 0; i < ARRAY_SIZE(idx_range); i++)
+	pci_read_config_word(dev, PCI_COMMAND, &command);
+	for (i = 0; i < ARRAY_SIZE(idx_range); i++)
 		for (idx = idx_range[i].start; idx <= idx_range[i].end; idx++) {
 			r = &dev->resource[idx];
-			if (r->parent)		/* Already allocated */
+			if (r->parent)	/* Already allocated */
 				continue;
-			if (!r->start)		/* Address not assigned at all */
+			if (!r->start)	/* Address not assigned at all */
 				continue;
 			if (r->flags & IORESOURCE_IO)
 				disabled = !(command & PCI_COMMAND_IO);
@@ -272,23 +270,29 @@ static void __init pcibios_allocate_resources(int pass)
 				}
 			}
 		}
-		if (!pass) {
-			r = &dev->resource[PCI_ROM_RESOURCE];
-			if (r->flags & IORESOURCE_ROM_ENABLE) {
-				/* Turn the ROM off, leave the resource region,
-				 * but keep it unregistered. */
-				u32 reg;
-				dev_dbg(&dev->dev, "disabling ROM %pR\n", r);
-				r->flags &= ~IORESOURCE_ROM_ENABLE;
-				pci_read_config_dword(dev,
-						dev->rom_base_reg, &reg);
-				pci_write_config_dword(dev, dev->rom_base_reg,
+	if (!pass) {
+		r = &dev->resource[PCI_ROM_RESOURCE];
+		if (r->flags & IORESOURCE_ROM_ENABLE) {
+			/* Turn the ROM off, leave the resource region,
+			 * but keep it unregistered. */
+			u32 reg;
+			dev_dbg(&dev->dev, "disabling ROM %pR\n", r);
+			r->flags &= ~IORESOURCE_ROM_ENABLE;
+			pci_read_config_dword(dev, dev->rom_base_reg, &reg);
+			pci_write_config_dword(dev, dev->rom_base_reg,
 						reg & ~PCI_ROM_ADDRESS_ENABLE);
-			}
 		}
 	}
 }
 
+static void __init pcibios_allocate_resources(int pass)
+{
+	struct pci_dev *dev = NULL;
+
+	for_each_pci_dev(dev)
+		pcibios_allocate_dev_resources(dev, pass);
+}
+
 static int __init pcibios_assign_resources(void)
 {
 	struct pci_dev *dev = NULL;
-- 
1.7.7

[PATCH 3/8] PCI, x86: Let pcibios_allocate_bus_resources() take bus instead

[Yinghai Lu]

Will need call the same code for one single root bus during hot add.
So make it take bus instead of bus list.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: x86@kernel.org
---
 arch/x86/pci/i386.c |   38 +++++++++++++++++++++++++-------------
 1 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 5817cf2..84696ed 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -215,16 +215,15 @@ static void __init pcibios_allocate_bridge_resources(struct pci_dev *dev)
 	}
 }
 
-static void __init pcibios_allocate_bus_resources(struct list_head *bus_list)
+static void __init pcibios_allocate_bus_resources(struct pci_bus *bus)
 {
-	struct pci_bus *bus;
+	struct pci_bus *child;
 
 	/* Depth-First Search on bus tree */
-	list_for_each_entry(bus, bus_list, node) {
-		if (bus->self)
-			pcibios_allocate_bridge_resources(bus->self);
-		pcibios_allocate_bus_resources(&bus->children);
-	}
+	if (bus->self)
+		pcibios_allocate_bridge_resources(bus->self);
+	list_for_each_entry(child, &bus->children, node)
+		pcibios_allocate_bus_resources(child);
 }
 
 struct pci_check_idx_range {
@@ -285,12 +284,18 @@ static void __init pcibios_allocate_dev_resources(struct pci_dev *dev, int pass)
 	}
 }
 
-static void __init pcibios_allocate_resources(int pass)
+static void __init pcibios_allocate_resources(struct pci_bus *bus, int pass)
 {
-	struct pci_dev *dev = NULL;
+	struct pci_dev *dev;
+	struct pci_bus *child;
 
-	for_each_pci_dev(dev)
+	list_for_each_entry(dev, &bus->devices, bus_list) {
 		pcibios_allocate_dev_resources(dev, pass);
+
+		child = dev->subordinate;
+		if (child)
+			pcibios_allocate_resources(child, pass);
+	}
 }
 
 static int __init pcibios_assign_resources(void)
@@ -323,10 +328,17 @@ static int __init pcibios_assign_resources(void)
 
 void __init pcibios_resource_survey(void)
 {
+	struct pci_bus *bus;
+
 	DBG("PCI: Allocating resources\n");
-	pcibios_allocate_bus_resources(&pci_root_buses);
-	pcibios_allocate_resources(0);
-	pcibios_allocate_resources(1);
+
+	list_for_each_entry(bus, &pci_root_buses, node)
+		pcibios_allocate_bus_resources(bus);
+
+	list_for_each_entry(bus, &pci_root_buses, node)
+		pcibios_allocate_resources(bus, 0);
+	list_for_each_entry(bus, &pci_root_buses, node)
+		pcibios_allocate_resources(bus, 1);
 
 	e820_reserve_resources_late();
 	/*
-- 
1.7.7

[PATCH 4/8] PCI, x86: Separate out rom resource claim

[Yinghai Lu]

So could use it with hot-added root bus later.

-v2: remove extra functions.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: x86@kernel.org
---
 arch/x86/pci/i386.c |   52 ++++++++++++++++++++++++++++++++++----------------
 1 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 84696ed..42dd755 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -298,27 +298,45 @@ static void __init pcibios_allocate_resources(struct pci_bus *bus, int pass)
 	}
 }
 
-static int __init pcibios_assign_resources(void)
+static void __init pcibios_allocate_dev_rom_resource(struct pci_dev *dev)
 {
-	struct pci_dev *dev = NULL;
 	struct resource *r;
 
-	if (!(pci_probe & PCI_ASSIGN_ROMS)) {
-		/*
-		 * Try to use BIOS settings for ROMs, otherwise let
-		 * pci_assign_unassigned_resources() allocate the new
-		 * addresses.
-		 */
-		for_each_pci_dev(dev) {
-			r = &dev->resource[PCI_ROM_RESOURCE];
-			if (!r->flags || !r->start)
-				continue;
-			if (pci_claim_resource(dev, PCI_ROM_RESOURCE) < 0) {
-				r->end -= r->start;
-				r->start = 0;
-			}
-		}
+	/*
+	 * Try to use BIOS settings for ROMs, otherwise let
+	 * pci_assign_unassigned_resources() allocate the new
+	 * addresses.
+	 */
+	r = &dev->resource[PCI_ROM_RESOURCE];
+	if (!r->flags || !r->start)
+		return;
+
+	if (pci_claim_resource(dev, PCI_ROM_RESOURCE) < 0) {
+		r->end -= r->start;
+		r->start = 0;
 	}
+}
+static void __init pcibios_allocate_rom_resources(struct pci_bus *bus)
+{
+	struct pci_dev *dev;
+	struct pci_bus *child;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		pcibios_allocate_dev_rom_resource(dev);
+
+		child = dev->subordinate;
+		if (child)
+			pcibios_allocate_rom_resources(child);
+	}
+}
+
+static int __init pcibios_assign_resources(void)
+{
+	struct pci_bus *bus;
+
+	if (!(pci_probe & PCI_ASSIGN_ROMS))
+		list_for_each_entry(bus, &pci_root_buses, node)
+			pcibios_allocate_rom_resources(bus);
 
 	pci_assign_unassigned_resources();
 	pcibios_fw_addr_list_del();
-- 
1.7.7

[PATCH 5/8] PCI, x86: Add pcibios_fw_addr_done

[Yinghai Lu]

Use it to skip not needed function after pcibios_fw_addr_list_del is called.

for pci root bus hot add, we will need to use pcibios_allocate_dev_resources(),
and don't want to mess up with fw_addr, for hot-add removing path.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: x86@kernel.org
---
 arch/x86/pci/i386.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 42dd755..edfc376 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -51,6 +51,7 @@ struct pcibios_fwaddrmap {
 
 static LIST_HEAD(pcibios_fwaddrmappings);
 static DEFINE_SPINLOCK(pcibios_fwaddrmap_lock);
+static bool pcibios_fw_addr_done;
 
 /* Must be called with 'pcibios_fwaddrmap_lock' lock held. */
 static struct pcibios_fwaddrmap *pcibios_fwaddrmap_lookup(struct pci_dev *dev)
@@ -72,6 +73,9 @@ pcibios_save_fw_addr(struct pci_dev *dev, int idx, resource_size_t fw_addr)
 	unsigned long flags;
 	struct pcibios_fwaddrmap *map;
 
+	if (pcibios_fw_addr_done)
+		return;
+
 	spin_lock_irqsave(&pcibios_fwaddrmap_lock, flags);
 	map = pcibios_fwaddrmap_lookup(dev);
 	if (!map) {
@@ -97,6 +101,9 @@ resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx)
 	struct pcibios_fwaddrmap *map;
 	resource_size_t fw_addr = 0;
 
+	if (pcibios_fw_addr_done)
+		return fw_addr;
+
 	spin_lock_irqsave(&pcibios_fwaddrmap_lock, flags);
 	map = pcibios_fwaddrmap_lookup(dev);
 	if (map)
@@ -106,7 +113,7 @@ resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx)
 	return fw_addr;
 }
 
-static void pcibios_fw_addr_list_del(void)
+static void __init pcibios_fw_addr_list_del(void)
 {
 	unsigned long flags;
 	struct pcibios_fwaddrmap *entry, *next;
@@ -118,6 +125,7 @@ static void pcibios_fw_addr_list_del(void)
 		kfree(entry);
 	}
 	spin_unlock_irqrestore(&pcibios_fwaddrmap_lock, flags);
+	pcibios_fw_addr_done = true;
 }
 
 static int
-- 
1.7.7

[PATCH 6/8] PCI, x86: Remove __init for hw/fw allocated functions

[Yinghai Lu]

will need it for hot add path.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: x86@kernel.org
---
 arch/x86/pci/i386.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index edfc376..1806e91 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -201,7 +201,7 @@ EXPORT_SYMBOL(pcibios_align_resource);
  *	    as well.
  */
 
-static void __init pcibios_allocate_bridge_resources(struct pci_dev *dev)
+static void pcibios_allocate_bridge_resources(struct pci_dev *dev)
 {
 	int idx;
 	struct resource *r;
@@ -223,7 +223,7 @@ static void __init pcibios_allocate_bridge_resources(struct pci_dev *dev)
 	}
 }
 
-static void __init pcibios_allocate_bus_resources(struct pci_bus *bus)
+static void pcibios_allocate_bus_resources(struct pci_bus *bus)
 {
 	struct pci_bus *child;
 
@@ -239,7 +239,7 @@ struct pci_check_idx_range {
 	int end;
 };
 
-static void __init pcibios_allocate_dev_resources(struct pci_dev *dev, int pass)
+static void pcibios_allocate_dev_resources(struct pci_dev *dev, int pass)
 {
 	int idx, disabled, i;
 	u16 command;
@@ -292,7 +292,7 @@ static void __init pcibios_allocate_dev_resources(struct pci_dev *dev, int pass)
 	}
 }
 
-static void __init pcibios_allocate_resources(struct pci_bus *bus, int pass)
+static void pcibios_allocate_resources(struct pci_bus *bus, int pass)
 {
 	struct pci_dev *dev;
 	struct pci_bus *child;
@@ -306,7 +306,7 @@ static void __init pcibios_allocate_resources(struct pci_bus *bus, int pass)
 	}
 }
 
-static void __init pcibios_allocate_dev_rom_resource(struct pci_dev *dev)
+static void pcibios_allocate_dev_rom_resource(struct pci_dev *dev)
 {
 	struct resource *r;
 
@@ -324,7 +324,7 @@ static void __init pcibios_allocate_dev_rom_resource(struct pci_dev *dev)
 		r->start = 0;
 	}
 }
-static void __init pcibios_allocate_rom_resources(struct pci_bus *bus)
+static void pcibios_allocate_rom_resources(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
 	struct pci_bus *child;
-- 
1.7.7

[PATCH 7/8] PCI, x86: Claim FW allocated resources in hot add path.

[Yinghai Lu]

During testing remove/rescan root bus 00, found
[  338.142574] bus: 'pci': really_probe: probing driver ata_piix with device 0000:00:01.1
[  338.146788] ata_piix 0000:00:01.1: device not available (can't reserve [io  0x01f0-0x01f7])
[  338.150565] ata_piix: probe of 0000:00:01.1 failed with error -22

because that fixed resource is not claimed.
For bootint path it is claimed in  from
        arch/x86/pci/i386.c::pcibios_allocate_resources()

Claim those resources, so on the remove/rescan will still use old
resources.

It is some kind honoring FW setting in the registers during hot add.
esp root-bus hot add is through acpi, BIOS has chance to set some registers
before handing over.

-v2: add rom resource claiming.
-v3: separate __init removing to another patch, also
   put pci_probe checking with caller from rom resource allocating

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: x86@kernel.org
---
 arch/x86/pci/i386.c |   13 +++++++++++++
 drivers/pci/bus.c   |    2 ++
 include/linux/pci.h |    1 +
 3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 1806e91..e0b7f30 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -352,6 +352,19 @@ static int __init pcibios_assign_resources(void)
 	return 0;
 }
 
+void pcibios_resource_survey_bus(struct pci_bus *bus)
+{
+	dev_printk(KERN_DEBUG, &bus->dev, "Allocating resources\n");
+
+	pcibios_allocate_bus_resources(bus);
+
+	pcibios_allocate_resources(bus, 0);
+	pcibios_allocate_resources(bus, 1);
+
+	if (!(pci_probe & PCI_ASSIGN_ROMS))
+		pcibios_allocate_rom_resources(bus);
+}
+
 void __init pcibios_resource_survey(void)
 {
 	struct pci_bus *bus;
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 6241fd0..a85247d 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -158,6 +158,8 @@ pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
 	return ret;
 }
 
+void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { }
+
 /**
  * pci_bus_add_device - add a single device
  * @dev: device to add
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 7860942..f30af2f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -654,6 +654,7 @@ extern struct list_head pci_root_buses;	/* list of all known PCI buses */
 /* Some device drivers need know if pci is initiated */
 extern int no_pci_devices(void);
 
+void pcibios_resource_survey_bus(struct pci_bus *bus);
 void pcibios_fixup_bus(struct pci_bus *);
 int __must_check pcibios_enable_device(struct pci_dev *, int mask);
 /* Architecture specific versions may override this (weak) */
-- 
1.7.7

[PATCH 8/8] PCI, ACPI: reserve fw allocated resource for hot added root bus

[Yinghai Lu]

FW could already allocate resource in pci bar registers, and we need to
reserve it before try to allocate another one.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/acpi/pci_root.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 012f40d..bc09567 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -644,8 +644,10 @@ static int acpi_pci_root_start(struct acpi_device *device)
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
-	if (system_state != SYSTEM_BOOTING)
+	if (system_state != SYSTEM_BOOTING) {
+		pcibios_resource_survey_bus(root->bus);
 		pci_assign_unassigned_bus_resources(root->bus);
+	}
 
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
-- 
1.7.7

Re:

[Bjorn Helgaas]

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!

Re:

[Yinghai Lu]

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

RE:

[Taku Izumi]

  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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Yinghai Lu]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Bjorn Helgaas]

[+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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Bjorn Helgaas]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Yinghai Lu]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Bjorn Helgaas]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Yinghai Lu]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Bjorn Helgaas]

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.

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Yinghai Lu]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Rafael J. Wysocki]

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


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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Benjamin Herrenschmidt]

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.

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Bjorn Helgaas]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Rafael J. Wysocki]

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


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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Bjorn Helgaas]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Bjorn Helgaas]

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

Re: [PATCH 0/8] PCI, ACPI, x86: Reserve fw allocated resource for hot-add root bus

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

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

[-- Attachment #2: root_bus_ioport_skip_2_a.patch --]
[-- Type: application/octet-stream, Size: 1298 bytes --]

Subject: [PATCH] PCI: check pci bus address for unassigned res

We should compare res->start with root bus window offset.
Otherwise will have problem with arch that support hostbridge
resource offset.

BenH pointed out that during reviewing patchset that separate
assign unassigned to per root buses.

According to Bjorn, have it in separated patch.

Use pcibios_resource_to_bus to get region at first, and check
region.start instead.

Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/setup-bus.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -1420,9 +1420,14 @@ static int check_unassigned_resources(st
 
 	for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) {
 		struct resource *r = &dev->resource[i];
+		struct pci_bus_region region;
 
 		/* Not assigned, or rejected by kernel ? */
-		if (r->flags && !r->start) {
+		if (!r->flags)
+			continue;
+
+		pcibios_resource_to_bus(dev, &region, r);
+		if (!region.start) {
 			(*unassigned)++;
 			return 1; /* return early from pci_walk_bus */
 		}