[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);

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

[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] 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);
>
>

[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);
 

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

[Bjorn Helgaas]