[PATCH 00/15] PCI/ACPI: Remove "pci_root" sub-driver support

[PATCH 00/15] PCI/ACPI: Remove "pci_root" sub-driver support

[Myron Stowe]

The "PCI Root Bridge ("pci_root")" driver includes support for attaching
sub-drivers.  acpi_pci_register_driver() and acpi_pci_unregister_driver()
are the interfaces that sub-drivers use to attach themselves.

Currently there are only two callers of acpi_pci_register_driver(): the
"ACPI Hot Plug PCI Controller Driver ("acpiphp")" and "ACPI PCI Slot
Detection Driver ("pci_slot")" sub-drivers.

Sub-drivers may be compiled as modules.  Kernel modules are instantiated
somewhat randomly - the order in which they are linked as the kernel as
built - thus if there are any dependencies on the ordering of attaching
sub-drivers, they can not be effectively dealt with.

This patch series resolves any potential sequencing inter-dependencies by
converting sub-driver functionality to being only supported as statically
built-in to the kernel as part of the "pci_root" driver itself (i.e. no
longer supported as a module).  Inter-dependencies can then be effectively
handled by explicitly sequencing the addition of such functionality.


The series begins with a patch from Yinghai Lu's "PCI, ACPI, x86: pci root
bus hotplug support" series that separates host bridge specific hot-plug
functionality from PCI device hot-plug functionality.  This patch
significantly simplifies a fairly confusing implementation due to the
co-mingling that currently exists.
  Reference: https://lkml.org/lkml/2012/9/19/413

The rest of the patches in this series implement the conversion of the
"PCI Root Bridge" sub-drivers.  This also simplifies the implementation as
is revealed by a 'diffstat' of the respective, conversion related,
patches: 9 files changed, 86 insertions(+), 237 deletions(-)


This series is targeting the 3.9 merge window.  I expect that after
3.8-rc1 is released, a -v2 of this series will be required due to Bill
Pemberton's "[PATCH 000/493] remove CONFIG_HOTPLUG as an option" series.
That said, I wanted to go ahead and get this out now in order to solicit
feedback.
  Reference: https://lkml.org/lkml/2012/11/16/660
---

Myron Stowe (14):
      PCI/ACPI: Remove the "pci_root" driver's sub-driver infrastructure
      PCI/ACPI: Add "pci_slot" functionality statically within "pci_root"
      PCI/ACPI: Convert "pci_slot" sub-driver's functionality to built-in only
      PCI/ACPI: Move where dmi_check_system() is called from
      PCI/ACPI: Fix latent refcount issue in acpi_pci_root_start()
      PCI/acpiphp: Change acpiphp_add_bridge() to return void instead of 0
      PCI/acpiphp: Add "acpiphp" functionality statically within "pci_root"
      PCI/acpiphp: Collapse the initialization call chain of "acpiphp" further
      PCI/acpiphp: Declare acpi_{add,remove}_bridge() as external functions
      PCI/acpiphp: Convert "acpiphp" sub-driver's functionality to built-in only
      PCI/acpiphp: Collapse initialization call chain of "acpiphp" sub-driver
      PCI/acpiphp: Change acpiphp_glue_init() to return void instead of 0
      PCI/acpiphp: Leave the "acpiphp" sub-driver registered and in place
      PCI/acpiphp: Fix coding style of external declarations in acpiphp.h

Yinghai Lu (1):
      PCI, acpiphp: Separate out hot-add support of pci host bridge


 drivers/acpi/Kconfig               |    5 -
 drivers/acpi/Makefile              |    1 
 drivers/acpi/pci_root.c            |   50 +-------
 drivers/acpi/pci_root_hp.c         |  228 ++++++++++++++++++++++++++++++++++++
 drivers/acpi/pci_slot.c            |   84 +++++--------
 drivers/pci/hotplug/Kconfig        |    5 -
 drivers/pci/hotplug/acpiphp.h      |   17 +--
 drivers/pci/hotplug/acpiphp_core.c |   45 -------
 drivers/pci/hotplug/acpiphp_glue.c |  115 +++---------------
 include/linux/acpi.h               |    9 -
 include/linux/pci-acpi.h           |   16 +++
 11 files changed, 312 insertions(+), 263 deletions(-)
 create mode 100644 drivers/acpi/pci_root_hp.c

-- 

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

[Myron Stowe]

From: Yinghai Lu <yinghai@kernel.org>

It causes confusion.

We may only need acpi hp for pci host bridge.

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

-v2: put back pci_root_hp change in one patch
-v3: add pcibios_resource_survey_bus() calling
-v4: remove not needed code with remove_bridge
-v5: put back support for acpiphp support for slots just on root bus.
-v6: change some functions to *_p2p_* to make it more clean.
-v7: split hot_added change out.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/Makefile              |    1 
 drivers/acpi/pci_root_hp.c         |  228 ++++++++++++++++++++++++++++++++++++
 drivers/pci/hotplug/acpiphp_glue.c |   59 ++-------
 3 files changed, 244 insertions(+), 44 deletions(-)
 create mode 100644 drivers/acpi/pci_root_hp.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 82422fe..4edfe41 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -36,6 +36,7 @@ acpi-y				+= processor_core.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
+acpi-$(CONFIG_HOTPLUG)		+= pci_root_hp.o
 acpi-y				+= power.o
 acpi-y				+= event.o
 acpi-y				+= sysfs.o
diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
new file mode 100644
index 0000000..10c12aa
--- /dev/null
+++ b/drivers/acpi/pci_root_hp.c
@@ -0,0 +1,228 @@
+/*
+ * Separated from drivers/pci/hotplug/acpiphp_glue.c
+ *	only support root bridge
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+
+static LIST_HEAD(acpi_root_bridge_list);
+struct acpi_root_bridge {
+	struct list_head list;
+	acpi_handle handle;
+	u32 flags;
+};
+
+/* bridge flags */
+#define ROOT_BRIDGE_HAS_EJ0	(0x00000002)
+#define ROOT_BRIDGE_HAS_PS3	(0x00000080)
+
+#define ACPI_STA_FUNCTIONING	(0x00000008)
+
+static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
+{
+	struct acpi_root_bridge *bridge;
+
+	list_for_each_entry(bridge, &acpi_root_bridge_list, list)
+		if (bridge->handle == handle)
+			return bridge;
+
+	return NULL;
+}
+
+/* allocate and initialize host bridge data structure */
+static void add_acpi_root_bridge(acpi_handle handle)
+{
+	struct acpi_root_bridge *bridge;
+	acpi_handle dummy_handle;
+	acpi_status status;
+
+	/* if the bridge doesn't have _STA, we assume it is always there */
+	status = acpi_get_handle(handle, "_STA", &dummy_handle);
+	if (ACPI_SUCCESS(status)) {
+		unsigned long long tmp;
+
+		status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
+		if (ACPI_FAILURE(status)) {
+			printk(KERN_DEBUG "%s: _STA evaluation failure\n",
+						 __func__);
+			return;
+		}
+		if ((tmp & ACPI_STA_FUNCTIONING) == 0)
+			/* don't register this object */
+			return;
+	}
+
+	bridge = kzalloc(sizeof(struct acpi_root_bridge), GFP_KERNEL);
+	if (!bridge)
+		return;
+
+	bridge->handle = handle;
+
+	if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
+		bridge->flags |= ROOT_BRIDGE_HAS_EJ0;
+	if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
+		bridge->flags |= ROOT_BRIDGE_HAS_PS3;
+
+	list_add(&bridge->list, &acpi_root_bridge_list);
+}
+
+struct acpi_root_hp_work {
+	struct work_struct work;
+	acpi_handle handle;
+	u32 type;
+	void *context;
+};
+
+static void alloc_acpi_root_hp_work(acpi_handle handle, u32 type,
+					void *context,
+					void (*func)(struct work_struct *work))
+{
+	struct acpi_root_hp_work *hp_work;
+	int ret;
+
+	hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
+	if (!hp_work)
+		return;
+
+	hp_work->handle = handle;
+	hp_work->type = type;
+	hp_work->context = context;
+
+	INIT_WORK(&hp_work->work, func);
+	ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
+	if (!ret)
+		kfree(hp_work);
+}
+
+static void handle_root_bridge_insertion(acpi_handle handle)
+{
+	struct acpi_device *device, *pdevice;
+	acpi_handle phandle;
+	int ret_val;
+
+	acpi_get_parent(handle, &phandle);
+	if (acpi_bus_get_device(phandle, &pdevice)) {
+		printk(KERN_DEBUG "no parent device, assuming NULL\n");
+		pdevice = NULL;
+	}
+	if (!acpi_bus_get_device(handle, &device)) {
+		/* check if  pci root_bus is removed */
+		struct acpi_pci_root *root = acpi_driver_data(device);
+		if (pci_find_bus(root->segment, root->secondary.start))
+			return;
+
+		printk(KERN_DEBUG "bus exists... trim\n");
+		/* this shouldn't be in here, so remove
+		 * the bus then re-add it...
+		 */
+		ret_val = acpi_bus_trim(device, 1);
+		printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
+	}
+	if (acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE)) {
+		printk(KERN_ERR "cannot add bridge to acpi list\n");
+		return;
+	}
+	if (acpi_bus_start(device))
+		printk(KERN_ERR "cannot start bridge\n");
+}
+
+static void _handle_hotplug_event_root(struct work_struct *work)
+{
+	struct acpi_root_bridge *bridge;
+	char objname[64];
+	struct acpi_buffer buffer = { .length = sizeof(objname),
+				      .pointer = objname };
+	struct acpi_root_hp_work *hp_work;
+	acpi_handle handle;
+	u32 type;
+
+	hp_work = container_of(work, struct acpi_root_hp_work, work);
+	handle = hp_work->handle;
+	type = hp_work->type;
+
+	bridge = acpi_root_handle_to_bridge(handle);
+
+	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+
+	switch (type) {
+	case ACPI_NOTIFY_BUS_CHECK:
+		/* bus enumerate */
+		printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
+				 objname);
+		if (!bridge) {
+			handle_root_bridge_insertion(handle);
+			add_acpi_root_bridge(handle);
+		}
+
+		break;
+
+	case ACPI_NOTIFY_DEVICE_CHECK:
+		/* device check */
+		printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
+				 objname);
+		if (!bridge) {
+			handle_root_bridge_insertion(handle);
+			add_acpi_root_bridge(handle);
+		}
+		break;
+
+	default:
+		printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
+				 type, objname);
+		break;
+	}
+
+	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
+}
+
+static void handle_hotplug_event_root(acpi_handle handle, u32 type,
+					void *context)
+{
+	alloc_acpi_root_hp_work(handle, type, context,
+				_handle_hotplug_event_root);
+}
+
+static acpi_status __init
+find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	char objname[64];
+	struct acpi_buffer buffer = { .length = sizeof(objname),
+				      .pointer = objname };
+	int *count = (int *)context;
+
+	if (!acpi_is_root_bridge(handle))
+		return AE_OK;
+
+	(*count)++;
+
+	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+
+	acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+				handle_hotplug_event_root, NULL);
+	printk(KERN_DEBUG "acpi root: %s notify handler installed\n", objname);
+
+	add_acpi_root_bridge(handle);
+
+	return AE_OK;
+}
+
+static int __init acpi_pci_root_hp_init(void)
+{
+	int num = 0;
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+		ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
+
+	printk(KERN_DEBUG "Found %d acpi root devices\n", num);
+
+	return 0;
+}
+
+subsys_initcall(acpi_pci_root_hp_init);
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 3d6d4fd..d0369dc 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -543,10 +543,13 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
 	acpi_status status;
 	acpi_handle handle = bridge->handle;
 
-	status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+	if (bridge->type != BRIDGE_TYPE_HOST) {
+		status = acpi_remove_notify_handler(handle,
+					    ACPI_SYSTEM_NOTIFY,
 					    handle_hotplug_event_bridge);
-	if (ACPI_FAILURE(status))
-		err("failed to remove notify handler\n");
+		if (ACPI_FAILURE(status))
+			err("failed to remove notify handler\n");
+	}
 
 	if ((bridge->type != BRIDGE_TYPE_HOST) &&
 	    ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
@@ -630,9 +633,6 @@ static void remove_bridge(struct acpi_pci_root *root)
 	bridge = acpiphp_handle_to_bridge(handle);
 	if (bridge)
 		cleanup_bridge(bridge);
-	else
-		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-					   handle_hotplug_event_bridge);
 }
 
 static int power_on_slot(struct acpiphp_slot *slot)
@@ -1107,18 +1107,12 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
 }
 
 /* Program resources in newly inserted bridge */
-static int acpiphp_configure_bridge (acpi_handle handle)
+static int acpiphp_configure_p2p_bridge(acpi_handle handle)
 {
-	struct pci_bus *bus;
+	struct pci_dev *pdev = acpi_get_pci_dev(handle);
+	struct pci_bus *bus = pdev->subordinate;
 
-	if (acpi_is_root_bridge(handle)) {
-		struct acpi_pci_root *root = acpi_pci_find_root(handle);
-		bus = root->bus;
-	} else {
-		struct pci_dev *pdev = acpi_get_pci_dev(handle);
-		bus = pdev->subordinate;
-		pci_dev_put(pdev);
-	}
+	pci_dev_put(pdev);
 
 	pci_bus_size_bridges(bus);
 	pci_bus_assign_resources(bus);
@@ -1128,7 +1122,7 @@ static int acpiphp_configure_bridge (acpi_handle handle)
 	return 0;
 }
 
-static void handle_bridge_insertion(acpi_handle handle, u32 type)
+static void handle_p2p_bridge_insertion(acpi_handle handle, u32 type)
 {
 	struct acpi_device *device, *pdevice;
 	acpi_handle phandle;
@@ -1148,9 +1142,9 @@ static void handle_bridge_insertion(acpi_handle handle, u32 type)
 		err("cannot add bridge to acpi list\n");
 		return;
 	}
-	if (!acpiphp_configure_bridge(handle) &&
+	if (!acpiphp_configure_p2p_bridge(handle) &&
 		!acpi_bus_start(device))
-		add_bridge(handle);
+		add_p2p_bridge(handle);
 	else
 		err("cannot configure and start bridge\n");
 
@@ -1236,7 +1230,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 
 	if (acpi_bus_get_device(handle, &device)) {
 		/* This bridge must have just been physically inserted */
-		handle_bridge_insertion(handle, type);
+		handle_p2p_bridge_insertion(handle, type);
 		goto out;
 	}
 
@@ -1413,21 +1407,6 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
 			      _handle_hotplug_event_func);
 }
 
-static acpi_status
-find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	int *count = (int *)context;
-
-	if (!acpi_is_root_bridge(handle))
-		return AE_OK;
-
-	(*count)++;
-	acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-				    handle_hotplug_event_bridge, NULL);
-
-	return AE_OK ;
-}
-
 static struct acpi_pci_driver acpi_pci_hp_driver = {
 	.add =		add_bridge,
 	.remove =	remove_bridge,
@@ -1438,15 +1417,7 @@ static struct acpi_pci_driver acpi_pci_hp_driver = {
  */
 int __init acpiphp_glue_init(void)
 {
-	int num = 0;
-
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-			ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
-
-	if (num <= 0)
-		return -1;
-	else
-		acpi_pci_register_driver(&acpi_pci_hp_driver);
+	acpi_pci_register_driver(&acpi_pci_hp_driver);
 
 	return 0;
 }

[PATCH 02/15] PCI/acpiphp: Fix coding style of external declarations in acpiphp.h

[Myron Stowe]

The external declarations trigger warnings from ./scripts/checkpatch.pl
such as:
  WARNING: space prohibited between function name and open parenthesis '('
  #32: FILE: drivers/pci/hotplug/acpiphp.h:194:
  +extern void acpiphp_glue_init (void);

This patch removes the extraneous spaces between the function names and
open parenthesis '('.

No functional change.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/pci/hotplug/acpiphp.h |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index a1afb5b..014ae78 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -191,18 +191,18 @@ extern int acpiphp_register_hotplug_slot(struct acpiphp_slot *slot);
 extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot);
 
 /* acpiphp_glue.c */
-extern int acpiphp_glue_init (void);
-extern void acpiphp_glue_exit (void);
-extern int acpiphp_get_num_slots (void);
+extern int acpiphp_glue_init(void);
+extern void acpiphp_glue_exit(void);
+extern int acpiphp_get_num_slots(void);
 typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data);
 
-extern int acpiphp_enable_slot (struct acpiphp_slot *slot);
-extern int acpiphp_disable_slot (struct acpiphp_slot *slot);
-extern int acpiphp_eject_slot (struct acpiphp_slot *slot);
-extern u8 acpiphp_get_power_status (struct acpiphp_slot *slot);
-extern u8 acpiphp_get_attention_status (struct acpiphp_slot *slot);
-extern u8 acpiphp_get_latch_status (struct acpiphp_slot *slot);
-extern u8 acpiphp_get_adapter_status (struct acpiphp_slot *slot);
+extern int acpiphp_enable_slot(struct acpiphp_slot *slot);
+extern int acpiphp_disable_slot(struct acpiphp_slot *slot);
+extern int acpiphp_eject_slot(struct acpiphp_slot *slot);
+extern u8 acpiphp_get_power_status(struct acpiphp_slot *slot);
+extern u8 acpiphp_get_attention_status(struct acpiphp_slot *slot);
+extern u8 acpiphp_get_latch_status(struct acpiphp_slot *slot);
+extern u8 acpiphp_get_adapter_status(struct acpiphp_slot *slot);
 
 /* variables */
 extern bool acpiphp_debug;

[PATCH 03/15] PCI/acpiphp: Leave the "acpiphp" sub-driver registered and in place

[Myron Stowe]

The "ACPI Hot Plug PCI Controller Driver ("acpiphp")" can be either
statically built-in or a kernel module.  If statically built-in, the
sub-driver needs to remain registered and in place in order to accommodate
any future hot-plug insertion events of devices it supports.

As currently implemented, the sub-driver may unregister itself during its
initialization.  This patch changes that behavior so that the sub-driver
remains registered and in place.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/pci/hotplug/acpiphp.h      |    1 -
 drivers/pci/hotplug/acpiphp_core.c |   16 +---------------
 drivers/pci/hotplug/acpiphp_glue.c |   21 ---------------------
 3 files changed, 1 insertions(+), 37 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index 014ae78..4b798e9 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -193,7 +193,6 @@ extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot);
 /* acpiphp_glue.c */
 extern int acpiphp_glue_init(void);
 extern void acpiphp_glue_exit(void);
-extern int acpiphp_get_num_slots(void);
 typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data);
 
 extern int acpiphp_enable_slot(struct acpiphp_slot *slot);
diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
index 96316b7..3350b09 100644
--- a/drivers/pci/hotplug/acpiphp_core.c
+++ b/drivers/pci/hotplug/acpiphp_core.c
@@ -50,7 +50,6 @@
 bool acpiphp_debug;
 
 /* local variables */
-static int num_slots;
 static struct acpiphp_attention_info *attention_info;
 
 #define DRIVER_VERSION	"0.5"
@@ -274,21 +273,8 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
 
 static int __init init_acpi(void)
 {
-	int retval;
-
 	/* initialize internal data structure etc. */
-	retval = acpiphp_glue_init();
-
-	/* read initial number of slots */
-	if (!retval) {
-		num_slots = acpiphp_get_num_slots();
-		if (num_slots == 0) {
-			acpiphp_glue_exit();
-			retval = -ENODEV;
-		}
-	}
-
-	return retval;
+	return acpiphp_glue_init();
 }
 
 /**
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index d0369dc..a378a48 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1435,27 +1435,6 @@ void  acpiphp_glue_exit(void)
 
 
 /**
- * acpiphp_get_num_slots - count number of slots in a system
- */
-int __init acpiphp_get_num_slots(void)
-{
-	struct acpiphp_bridge *bridge;
-	int num_slots = 0;
-
-	list_for_each_entry(bridge, &bridge_list, list) {
-		dbg("Bus %04x:%02x has %d slot%s\n",
-				pci_domain_nr(bridge->pci_bus),
-				bridge->pci_bus->number, bridge->nr_slots,
-				bridge->nr_slots == 1 ? "" : "s");
-		num_slots += bridge->nr_slots;
-	}
-
-	dbg("Total %d slots\n", num_slots);
-	return num_slots;
-}
-
-
-/**
  * acpiphp_enable_slot - power on slot
  * @slot: ACPI PHP slot
  */

[PATCH 04/15] PCI/acpiphp: Change acpiphp_glue_init() to return void instead of 0

[Myron Stowe]

acpiphp_glue_init() always returns 0.  This patch changes the return
type from an 'int' to 'void'.

No functional change.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/pci/hotplug/acpiphp.h      |    2 +-
 drivers/pci/hotplug/acpiphp_core.c |    4 +++-
 drivers/pci/hotplug/acpiphp_glue.c |    4 +---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index 4b798e9..ecd772c 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -191,7 +191,7 @@ extern int acpiphp_register_hotplug_slot(struct acpiphp_slot *slot);
 extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot);
 
 /* acpiphp_glue.c */
-extern int acpiphp_glue_init(void);
+extern void acpiphp_glue_init(void);
 extern void acpiphp_glue_exit(void);
 typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data);
 
diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
index 3350b09..011a538 100644
--- a/drivers/pci/hotplug/acpiphp_core.c
+++ b/drivers/pci/hotplug/acpiphp_core.c
@@ -274,7 +274,9 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
 static int __init init_acpi(void)
 {
 	/* initialize internal data structure etc. */
-	return acpiphp_glue_init();
+	acpiphp_glue_init();
+
+	return 0;
 }
 
 /**
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index a378a48..50b58cb 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1415,11 +1415,9 @@ static struct acpi_pci_driver acpi_pci_hp_driver = {
 /**
  * acpiphp_glue_init - initializes all PCI hotplug - ACPI glue data structures
  */
-int __init acpiphp_glue_init(void)
+void __init acpiphp_glue_init(void)
 {
 	acpi_pci_register_driver(&acpi_pci_hp_driver);
-
-	return 0;
 }
 
 

[PATCH 05/15] PCI/acpiphp: Collapse initialization call chain of "acpiphp" sub-driver

[Myron Stowe]

This patch collapses the initialization call chain of "acpiphp" by calling
acpiphp_glue_init() directly within acphphp_init(), removing the
intervening init_acpi() call.

No functional change.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/pci/hotplug/acpiphp_core.c |   24 +++++++-----------------
 1 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
index 011a538..1f4e228 100644
--- a/drivers/pci/hotplug/acpiphp_core.c
+++ b/drivers/pci/hotplug/acpiphp_core.c
@@ -271,14 +271,6 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
 	return 0;
 }
 
-static int __init init_acpi(void)
-{
-	/* initialize internal data structure etc. */
-	acpiphp_glue_init();
-
-	return 0;
-}
-
 /**
  * release_slot - free up the memory used by a slot
  * @hotplug_slot: slot to free
@@ -363,21 +355,19 @@ static int __init acpiphp_init(void)
 {
 	info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
 
-	if (acpi_pci_disabled)
-		return 0;
+	if (!acpi_pci_disabled)
+		/* initialize internal data structure etc. */
+		acpiphp_glue_init();
 
-	/* read all the ACPI info from the system */
-	return init_acpi();
+	return 0;
 }
 
 
 static void __exit acpiphp_exit(void)
 {
-	if (acpi_pci_disabled)
-		return;
-
-	/* deallocate internal data structures etc. */
-	acpiphp_glue_exit();
+	if (!acpi_pci_disabled)
+		/* deallocate internal data structures etc. */
+		acpiphp_glue_exit();
 }
 
 module_init(acpiphp_init);

[PATCH 06/15] PCI/acpiphp: Convert "acpiphp" sub-driver's functionality to built-in only

[Myron Stowe]

The "ACPI Hot Plug PCI Controller ("acpiphp")" sub-driver may be compiled
as a module.  Kernel modules are instantiated somewhat randomly - the
order in which they are linked as the kernel as built - thus if there are
any dependencies on the ordering of attaching sub-drivers, they can not be
effectively dealt with.

This patch series resolves any potential sequencing inter-dependencies by
converting "acpiphp" sub-driver's functionality to being only supported as
statically built-in to the kernel.  Inter-dependencies can then be
effectively handled by explicitly sequencing the addition of such
functionality.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/pci/hotplug/Kconfig |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/hotplug/Kconfig b/drivers/pci/hotplug/Kconfig
index b0e46de..3bf2b82 100644
--- a/drivers/pci/hotplug/Kconfig
+++ b/drivers/pci/hotplug/Kconfig
@@ -52,15 +52,12 @@ config HOTPLUG_PCI_IBM
 	  When in doubt, say N.
 
 config HOTPLUG_PCI_ACPI
-	tristate "ACPI PCI Hotplug driver"
+	bool "ACPI PCI Hotplug driver"
 	depends on (!ACPI_DOCK && ACPI) || (ACPI_DOCK)
 	help
 	  Say Y here if you have a system that supports PCI Hotplug using
 	  ACPI.
 
-	  To compile this driver as a module, choose M here: the
-	  module will be called acpiphp.
-
 	  When in doubt, say N.
 
 config HOTPLUG_PCI_ACPI_IBM

[PATCH 07/15] PCI/acpiphp: Declare acpi_{add, remove}_bridge() as external functions

[Myron Stowe]

This patch renames, and changes to external function types, add_bridge()
and remove_bridge() so that they can used directly as the add and remove
interfaces of "acpiphp".

No functional change.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/pci/hotplug/acpiphp.h      |    3 +++
 drivers/pci/hotplug/acpiphp_glue.c |   10 +++++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index ecd772c..37f968e 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -191,6 +191,9 @@ extern int acpiphp_register_hotplug_slot(struct acpiphp_slot *slot);
 extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot);
 
 /* acpiphp_glue.c */
+extern struct acpi_pci_driver acpi_pci_hp_driver;
+extern int acpiphp_add_bridge(struct acpi_pci_root *root);
+extern void acpiphp_remove_bridge(struct acpi_pci_root *root);
 extern void acpiphp_glue_init(void);
 extern void acpiphp_glue_exit(void);
 typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data);
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 50b58cb..f96b13c 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -489,7 +489,7 @@ find_p2p_bridge(acpi_handle handle, u32 lvl, void *context, void **rv)
 
 
 /* find hot-pluggable slots, and then find P2P bridge */
-static int add_bridge(struct acpi_pci_root *root)
+int acpiphp_add_bridge(struct acpi_pci_root *root)
 {
 	acpi_status status;
 	unsigned long long tmp;
@@ -613,7 +613,7 @@ cleanup_p2p_bridge(acpi_handle handle, u32 lvl, void *context, void **rv)
 	return AE_OK;
 }
 
-static void remove_bridge(struct acpi_pci_root *root)
+void acpiphp_remove_bridge(struct acpi_pci_root *root)
 {
 	struct acpiphp_bridge *bridge;
 	acpi_handle handle = root->device->handle;
@@ -1407,9 +1407,9 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
 			      _handle_hotplug_event_func);
 }
 
-static struct acpi_pci_driver acpi_pci_hp_driver = {
-	.add =		add_bridge,
-	.remove =	remove_bridge,
+struct acpi_pci_driver acpi_pci_hp_driver = {
+	.add =		acpiphp_add_bridge,
+	.remove =	acpiphp_remove_bridge,
 };
 
 /**

[PATCH 08/15] PCI/acpiphp: Collapse the initialization call chain of "acpiphp" further

[Myron Stowe]

This patch collapses the initialization call chain of "acpiphp" by calling
acpi_pci_[un]register_driver() directly within acpiphp_{init,exit}(),
removing the intervening acpiphp_glue_{init,exit}() call.

No functional change.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/pci/hotplug/acpiphp.h      |    2 --
 drivers/pci/hotplug/acpiphp_core.c |    4 ++--
 drivers/pci/hotplug/acpiphp_glue.c |   20 --------------------
 3 files changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index 37f968e..bf62ac8 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -194,8 +194,6 @@ extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot);
 extern struct acpi_pci_driver acpi_pci_hp_driver;
 extern int acpiphp_add_bridge(struct acpi_pci_root *root);
 extern void acpiphp_remove_bridge(struct acpi_pci_root *root);
-extern void acpiphp_glue_init(void);
-extern void acpiphp_glue_exit(void);
 typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data);
 
 extern int acpiphp_enable_slot(struct acpiphp_slot *slot);
diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
index 1f4e228..5536b42 100644
--- a/drivers/pci/hotplug/acpiphp_core.c
+++ b/drivers/pci/hotplug/acpiphp_core.c
@@ -357,7 +357,7 @@ static int __init acpiphp_init(void)
 
 	if (!acpi_pci_disabled)
 		/* initialize internal data structure etc. */
-		acpiphp_glue_init();
+		acpi_pci_register_driver(&acpi_pci_hp_driver);
 
 	return 0;
 }
@@ -367,7 +367,7 @@ static void __exit acpiphp_exit(void)
 {
 	if (!acpi_pci_disabled)
 		/* deallocate internal data structures etc. */
-		acpiphp_glue_exit();
+		acpi_pci_unregister_driver(&acpi_pci_hp_driver);
 }
 
 module_init(acpiphp_init);
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index f96b13c..3d5d7da 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1413,26 +1413,6 @@ struct acpi_pci_driver acpi_pci_hp_driver = {
 };
 
 /**
- * acpiphp_glue_init - initializes all PCI hotplug - ACPI glue data structures
- */
-void __init acpiphp_glue_init(void)
-{
-	acpi_pci_register_driver(&acpi_pci_hp_driver);
-}
-
-
-/**
- * acpiphp_glue_exit - terminates all PCI hotplug - ACPI glue data structures
- *
- * This function frees all data allocated in acpiphp_glue_init().
- */
-void  acpiphp_glue_exit(void)
-{
-	acpi_pci_unregister_driver(&acpi_pci_hp_driver);
-}
-
-
-/**
  * acpiphp_enable_slot - power on slot
  * @slot: ACPI PHP slot
  */

[PATCH 09/15] PCI/acpiphp: Add "acpiphp" functionality statically within "pci_root"

[Myron Stowe]

With the "ACPI Hot Plug PCI Controller Driver ("acpiphp")" conversion in
place - being statically built-in to the kernel and no longer capable of
being a kernel module - remove the use of acpi_pci_[un]register_driver()
and add its functionality from directly within the "pci_root" driver.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/pci_root.c            |    2 ++
 drivers/pci/hotplug/acpiphp.h      |    3 ---
 drivers/pci/hotplug/acpiphp_core.c |   23 -----------------------
 drivers/pci/hotplug/acpiphp_glue.c |    5 -----
 include/linux/pci-acpi.h           |    8 ++++++++
 5 files changed, 10 insertions(+), 31 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index bce469c..d890322 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -648,6 +648,7 @@ static int acpi_pci_root_start(struct acpi_device *device)
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
 			driver->add(root);
+	acpiphp_add_bridge(root);
 	mutex_unlock(&acpi_pci_root_lock);
 
 	pci_bus_add_devices(root->bus);
@@ -664,6 +665,7 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
 			driver->remove(root);
+	acpiphp_remove_bridge(root);
 
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index bf62ac8..13428c9 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -191,9 +191,6 @@ extern int acpiphp_register_hotplug_slot(struct acpiphp_slot *slot);
 extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot);
 
 /* acpiphp_glue.c */
-extern struct acpi_pci_driver acpi_pci_hp_driver;
-extern int acpiphp_add_bridge(struct acpi_pci_root *root);
-extern void acpiphp_remove_bridge(struct acpi_pci_root *root);
 typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data);
 
 extern int acpiphp_enable_slot(struct acpiphp_slot *slot);
diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
index 5536b42..fb97e8b 100644
--- a/drivers/pci/hotplug/acpiphp_core.c
+++ b/drivers/pci/hotplug/acpiphp_core.c
@@ -349,26 +349,3 @@ void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *acpiphp_slot)
 	if (retval)
 		err("pci_hp_deregister failed with error %d\n", retval);
 }
-
-
-static int __init acpiphp_init(void)
-{
-	info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
-
-	if (!acpi_pci_disabled)
-		/* initialize internal data structure etc. */
-		acpi_pci_register_driver(&acpi_pci_hp_driver);
-
-	return 0;
-}
-
-
-static void __exit acpiphp_exit(void)
-{
-	if (!acpi_pci_disabled)
-		/* deallocate internal data structures etc. */
-		acpi_pci_unregister_driver(&acpi_pci_hp_driver);
-}
-
-module_init(acpiphp_init);
-module_exit(acpiphp_exit);
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 3d5d7da..a4218cb 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1407,11 +1407,6 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
 			      _handle_hotplug_event_func);
 }
 
-struct acpi_pci_driver acpi_pci_hp_driver = {
-	.add =		acpiphp_add_bridge,
-	.remove =	acpiphp_remove_bridge,
-};
-
 /**
  * acpiphp_enable_slot - power on slot
  * @slot: ACPI PHP slot
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 9a22b5e..3197070 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -49,4 +49,12 @@ extern bool aer_acpi_firmware_first(void);
 static inline bool aer_acpi_firmware_first(void) { return false; }
 #endif
 
+#ifdef CONFIG_HOTPLUG_PCI_ACPI
+extern int acpiphp_add_bridge(struct acpi_pci_root *root);
+extern void acpiphp_remove_bridge(struct acpi_pci_root *root);
+#else /* !CONFIG_HOTPLUG_PCI_ACPI */
+static inline int acpiphp_add_bridge(struct acpi_pci_root *) { return 0; }
+static inline void acpiphp_remove_bridge(struct acpi_pci_root *) { }
+#endif /* !CONFIG_HOTPLUG_PCI_ACPI */
+
 #endif	/* _PCI_ACPI_H_ */

[PATCH 10/15] PCI/acpiphp: Change acpiphp_add_bridge() to return void instead of 0

[Myron Stowe]

acpiphp_add_bridge() always returns 0.  This patch changes the return
type from an 'int' to 'void'.

No functional change.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/pci/hotplug/acpiphp_glue.c |    8 +++-----
 include/linux/pci-acpi.h           |    4 ++--
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index a4218cb..fe2aac6 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -489,7 +489,7 @@ find_p2p_bridge(acpi_handle handle, u32 lvl, void *context, void **rv)
 
 
 /* find hot-pluggable slots, and then find P2P bridge */
-int acpiphp_add_bridge(struct acpi_pci_root *root)
+void acpiphp_add_bridge(struct acpi_pci_root *root)
 {
 	acpi_status status;
 	unsigned long long tmp;
@@ -502,11 +502,11 @@ int acpiphp_add_bridge(struct acpi_pci_root *root)
 		status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
 		if (ACPI_FAILURE(status)) {
 			dbg("%s: _STA evaluation failure\n", __func__);
-			return 0;
+			return;
 		}
 		if ((tmp & ACPI_STA_FUNCTIONING) == 0)
 			/* don't register this object */
-			return 0;
+			return;
 	}
 
 	/* check if this bridge has ejectable slots */
@@ -521,8 +521,6 @@ int acpiphp_add_bridge(struct acpi_pci_root *root)
 
 	if (ACPI_FAILURE(status))
 		warn("find_p2p_bridge failed (error code = 0x%x)\n", status);
-
-	return 0;
 }
 
 static struct acpiphp_bridge *acpiphp_handle_to_bridge(acpi_handle handle)
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 3197070..bc5b763 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -50,10 +50,10 @@ static inline bool aer_acpi_firmware_first(void) { return false; }
 #endif
 
 #ifdef CONFIG_HOTPLUG_PCI_ACPI
-extern int acpiphp_add_bridge(struct acpi_pci_root *root);
+extern void acpiphp_add_bridge(struct acpi_pci_root *root);
 extern void acpiphp_remove_bridge(struct acpi_pci_root *root);
 #else /* !CONFIG_HOTPLUG_PCI_ACPI */
-static inline int acpiphp_add_bridge(struct acpi_pci_root *) { return 0; }
+static inline void acpiphp_add_bridge(struct acpi_pci_root *) { }
 static inline void acpiphp_remove_bridge(struct acpi_pci_root *) { }
 #endif /* !CONFIG_HOTPLUG_PCI_ACPI */
 

[PATCH 11/15] PCI/ACPI: Fix latent refcount issue in acpi_pci_root_start()

[Myron Stowe]

During early boot, the kernel performs ACPI enumeration in which host
bridges are discovered (subsys_initcall(acpi_pci_root_init)).  Later
drivers, both built-in and modules such as the "ACPI PCI Slot Detection
Driver ("pci_slot")", are loaded (module_init(acpi_pci_slot_init)) thus we
end up with the following call chain:
  acpi_pci_root_start
    list_for_each_entry(..., acpi_pci_drivers, ...)
      driver->add(root)		# always no-op; list empty
    pci_bus_add_devices
  acpi_pci_slot_init		# module_init
    acpi_pci_register_driver(acpi_pci_slot_driver)
      list_for_each_entry(..., &acpi_pci_roots, ...)
        driver->add(root)	# acpi_pci_slot_add()

Note that for host bridges present at boot time the 'acpi_pci_drivers'
list is always empty when acpi_pci_root_start() runs.

However, during a Host Bridge hot-add event, the "pci_slot" sub-driver is
already on the 'acpi_pci_drivers' list and we end up calling
acpi_pci_slot_add() before pci_bus_add_devices() and encounter the
following refcount WARNING:
  calling acpi_pci_slot_add():
  pci_bus 0000:03: dev 00, created physical slot 1
  ------------[ cut here ]------------
  WARNING: at include/linux/kref.h:42 kobject_get+0x32/0x40()
  Call Trace:
   [<ffffffff812541d2>] kobject_get+0x32/0x40
   [<ffffffff8133f0f9>] get_device+0x19/0x20
   [<ffffffff812d9f11>] register_slot+0x216/0x26d
   [<ffffffff812ce92f>] acpi_walk_namespace+0x8a/0xc4
   [<ffffffff812d9cb9>] walk_p2p_bridge+0xf1/0x133
   [<ffffffff812ce92f>] acpi_walk_namespace+0x8a/0xc4
   [<ffffffff812d9b71>] acpi_pci_slot_add+0xe0/0x137
   [<ffffffff812b8705>] acpi_pci_root_start+0x3e/0x59

This patch fixes this latent issue by moving up pci_bus_add_devices() so
that the refcount will be initialized before subsequent references, via
driver additions from the 'acpi_pci_drivers' list, occur.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/pci_root.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index d890322..f9be8fb 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -644,6 +644,8 @@ static int acpi_pci_root_start(struct acpi_device *device)
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	pci_bus_add_devices(root->bus);
+
 	mutex_lock(&acpi_pci_root_lock);
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->add)
@@ -651,8 +653,6 @@ static int acpi_pci_root_start(struct acpi_device *device)
 	acpiphp_add_bridge(root);
 	mutex_unlock(&acpi_pci_root_lock);
 
-	pci_bus_add_devices(root->bus);
-
 	return 0;
 }
 

[PATCH 12/15] PCI/ACPI: Move where dmi_check_system() is called from

[Myron Stowe]

This patch moves the call of dmi_check_system() from acpi_pci_slot_init()
to acpi_pci_slot_add().  This is in preparation for converting the
"pci_slot" sub-driver to being statically built into the kernel.

No functional change.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/pci_slot.c |   57 ++++++++++++++++++++++++++---------------------
 1 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index d22585f..efa58af 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -265,6 +265,31 @@ walk_root_bridge(struct acpi_pci_root *root, acpi_walk_callback user_function)
 	return status;
 }
 
+static int do_sta_before_sun(const struct dmi_system_id *d)
+{
+	info("%s detected: will evaluate _STA before calling _SUN\n", d->ident);
+	check_sta_before_sun = 1;
+	return 0;
+}
+
+static struct dmi_system_id acpi_pci_slot_dmi_table[] = {
+	/*
+	 * Fujitsu Primequest machines will return 1023 to indicate an
+	 * error if the _SUN method is evaluated on SxFy objects that
+	 * are not present (as indicated by _STA), so for those machines,
+	 * we want to check _STA before evaluating _SUN.
+	 */
+	{
+	 .callback = do_sta_before_sun,
+	 .ident = "Fujitsu PRIMEQUEST",
+	 .matches = {
+		DMI_MATCH(DMI_BIOS_VENDOR, "FUJITSU LIMITED"),
+		DMI_MATCH(DMI_BIOS_VERSION, "PRIMEQUEST"),
+		},
+	},
+	{}
+};
+
 /*
  * acpi_pci_slot_add
  * @handle: points to an acpi_pci_root
@@ -272,8 +297,14 @@ walk_root_bridge(struct acpi_pci_root *root, acpi_walk_callback user_function)
 static int
 acpi_pci_slot_add(struct acpi_pci_root *root)
 {
+	static bool inited = false;
 	acpi_status status;
 
+	if (!inited) {
+		dmi_check_system(acpi_pci_slot_dmi_table);
+		inited = true;
+	}
+
 	status = walk_root_bridge(root, register_slot);
 	if (ACPI_FAILURE(status))
 		err("%s: register_slot failure - %d\n", __func__, status);
@@ -305,35 +336,9 @@ acpi_pci_slot_remove(struct acpi_pci_root *root)
 	mutex_unlock(&slot_list_lock);
 }
 
-static int do_sta_before_sun(const struct dmi_system_id *d)
-{
-	info("%s detected: will evaluate _STA before calling _SUN\n", d->ident);
-	check_sta_before_sun = 1;
-	return 0;
-}
-
-static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = {
-	/*
-	 * Fujitsu Primequest machines will return 1023 to indicate an
-	 * error if the _SUN method is evaluated on SxFy objects that
-	 * are not present (as indicated by _STA), so for those machines,
-	 * we want to check _STA before evaluating _SUN.
-	 */
-	{
-	 .callback = do_sta_before_sun,
-	 .ident = "Fujitsu PRIMEQUEST",
-	 .matches = {
-		DMI_MATCH(DMI_BIOS_VENDOR, "FUJITSU LIMITED"),
-		DMI_MATCH(DMI_BIOS_VERSION, "PRIMEQUEST"),
-		},
-	},
-	{}
-};
-
 static int __init
 acpi_pci_slot_init(void)
 {
-	dmi_check_system(acpi_pci_slot_dmi_table);
 	acpi_pci_register_driver(&acpi_pci_slot_driver);
 	return 0;
 }

[PATCH 13/15] PCI/ACPI: Convert "pci_slot" sub-driver's functionality to built-in only

[Myron Stowe]

The "ACPI PCI Slot Detection ("pci_slot")" sub-driver may be compiled as a
module.  Kernel modules are instantiated somewhat randomly - the order in
which they are linked as the kernel as built - thus if there are any
dependencies on the ordering of attaching sub-drivers, they can not be
effectively dealt with.

This patch series resolves any potential sequencing inter-dependencies by
converting "pci_slot" sub-driver's functionality to being only supported
as statically built-in to the kernel.  Inter-dependencies can then be
effectively handled by explicitly sequencing the addition of such
functionality.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/Kconfig |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 119d58d..988bb13 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -293,7 +293,7 @@ config ACPI_DEBUG_FUNC_TRACE
 	  is about half of the penalty and is rarely useful.
 
 config ACPI_PCI_SLOT
-	tristate "PCI slot detection driver"
+	bool "PCI slot detection driver"
 	depends on SYSFS
 	default n
 	help
@@ -302,9 +302,6 @@ config ACPI_PCI_SLOT
 	  i.e., segment/bus/device/function tuples, with physical slots in
 	  the system.  If you are unsure, say N.
 
-	  To compile this driver as a module, choose M here:
-	  the module will be called pci_slot.
-
 config X86_PM_TIMER
 	bool "Power Management Timer Support" if EXPERT
 	depends on X86

[PATCH 14/15] PCI/ACPI: Add "pci_slot" functionality statically within "pci_root"

[Myron Stowe]

With the "ACPI PCI Slot Detection Driver ("pci_slot")" conversion in place
- being statically built-in to the kernel and no longer capable of being a
kernel module - remove the use of acpi_pci_[un]register_driver() and add
its functionality from directly within the "pci_root" driver.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/pci_root.c  |    2 ++
 drivers/acpi/pci_slot.c  |   27 ++-------------------------
 include/linux/pci-acpi.h |    8 ++++++++
 3 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index f9be8fb..42566e2 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -651,6 +651,7 @@ static int acpi_pci_root_start(struct acpi_device *device)
 		if (driver->add)
 			driver->add(root);
 	acpiphp_add_bridge(root);
+	acpi_pci_slot_add(root);
 	mutex_unlock(&acpi_pci_root_lock);
 
 	return 0;
@@ -665,6 +666,7 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
 		if (driver->remove)
 			driver->remove(root);
+	acpi_pci_slot_remove(root);
 	acpiphp_remove_bridge(root);
 
 	device_set_run_wake(root->bus->bridge, false);
diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index efa58af..b3944a8 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -67,15 +67,8 @@ struct acpi_pci_slot {
 	struct list_head list;		/* node in the list of slots */
 };
 
-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);
-static struct acpi_pci_driver acpi_pci_slot_driver = {
-	.add = acpi_pci_slot_add,
-	.remove = acpi_pci_slot_remove,
-};
 
 static int
 check_slot(acpi_handle handle, unsigned long long *sun)
@@ -294,7 +287,7 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] = {
  * acpi_pci_slot_add
  * @handle: points to an acpi_pci_root
  */
-static int
+int
 acpi_pci_slot_add(struct acpi_pci_root *root)
 {
 	static bool inited = false;
@@ -316,7 +309,7 @@ acpi_pci_slot_add(struct acpi_pci_root *root)
  * acpi_pci_slot_remove
  * @handle: points to an acpi_pci_root
  */
-static void
+void
 acpi_pci_slot_remove(struct acpi_pci_root *root)
 {
 	struct acpi_pci_slot *slot, *tmp;
@@ -335,19 +328,3 @@ acpi_pci_slot_remove(struct acpi_pci_root *root)
 	}
 	mutex_unlock(&slot_list_lock);
 }
-
-static int __init
-acpi_pci_slot_init(void)
-{
-	acpi_pci_register_driver(&acpi_pci_slot_driver);
-	return 0;
-}
-
-static void __exit
-acpi_pci_slot_exit(void)
-{
-	acpi_pci_unregister_driver(&acpi_pci_slot_driver);
-}
-
-module_init(acpi_pci_slot_init);
-module_exit(acpi_pci_slot_exit);
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index bc5b763..339ea97 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -57,4 +57,12 @@ static inline void acpiphp_add_bridge(struct acpi_pci_root *) { }
 static inline void acpiphp_remove_bridge(struct acpi_pci_root *) { }
 #endif /* !CONFIG_HOTPLUG_PCI_ACPI */
 
+#ifdef CONFIG_ACPI_PCI_SLOT
+extern int acpi_pci_slot_add(struct acpi_pci_root *root);
+extern void acpi_pci_slot_remove(struct acpi_pci_root *root);
+#else /* !CONFIG_ACPI_PCI_SLOT */
+static inline int acpi_pci_slot_add(struct acpi_pci_root *) { return 0; }
+static inline void acpi_pci_slot_remove(struct acpi_pci_root *) { }
+#endif /* !CONFIG_ACPI_PCI_SLOT */
+
 #endif	/* _PCI_ACPI_H_ */

[PATCH 15/15] PCI/ACPI: Remove the "pci_root" driver's sub-driver infrastructure

[Myron Stowe]

Both sub-drivers of the "PCI Root Bridge ("pci_bridge")" driver, "acpiphp"
and "pci_slot", have been converted to statically built-in functionality
added directly from within "pci_bridge".

With the conversions there are no remaining usages of the 'struct
acpi_pci_driver' list based infrastructure.  This patch removes it.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/pci_root.c |   42 +-----------------------------------------
 include/linux/acpi.h    |    9 ---------
 2 files changed, 1 insertions(+), 50 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 42566e2..7cde9dc 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -71,44 +71,12 @@ static struct acpi_driver acpi_pci_root_driver = {
 		},
 };
 
-/* Lock to protect both acpi_pci_roots and acpi_pci_drivers lists */
+/* Lock to protect acpi_pci_roots list */
 static DEFINE_MUTEX(acpi_pci_root_lock);
 static LIST_HEAD(acpi_pci_roots);
-static LIST_HEAD(acpi_pci_drivers);
 
 static DEFINE_MUTEX(osc_lock);
 
-int acpi_pci_register_driver(struct acpi_pci_driver *driver)
-{
-	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)
-		list_for_each_entry(root, &acpi_pci_roots, node) {
-			driver->add(root);
-			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)
-		list_for_each_entry(root, &acpi_pci_roots, node)
-			driver->remove(root);
-	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)
 {
 	struct acpi_pci_root *root;
@@ -642,14 +610,10 @@ 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;
 
 	pci_bus_add_devices(root->bus);
 
 	mutex_lock(&acpi_pci_root_lock);
-	list_for_each_entry(driver, &acpi_pci_drivers, node)
-		if (driver->add)
-			driver->add(root);
 	acpiphp_add_bridge(root);
 	acpi_pci_slot_add(root);
 	mutex_unlock(&acpi_pci_root_lock);
@@ -660,12 +624,8 @@ static int acpi_pci_root_start(struct acpi_device *device)
 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;
 
 	mutex_lock(&acpi_pci_root_lock);
-	list_for_each_entry(driver, &acpi_pci_drivers, node)
-		if (driver->remove)
-			driver->remove(root);
 	acpi_pci_slot_remove(root);
 	acpiphp_remove_bridge(root);
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 90be989..8e2b65f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -137,15 +137,6 @@ void acpi_penalize_isa_irq(int irq, int active);
 
 void acpi_pci_irq_disable (struct pci_dev *dev);
 
-struct acpi_pci_driver {
-	struct list_head node;
-	int (*add)(struct acpi_pci_root *root);
-	void (*remove)(struct acpi_pci_root *root);
-};
-
-int acpi_pci_register_driver(struct acpi_pci_driver *driver);
-void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
-
 extern int ec_read(u8 addr, u8 *val);
 extern int ec_write(u8 addr, u8 val);
 extern int ec_transaction(u8 command,

Re: [PATCH 06/15] PCI/acpiphp: Convert "acpiphp" sub-driver's functionality to built-in only

[Yinghai Lu]

On Thu, Dec 6, 2012 at 10:25 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
> The "ACPI Hot Plug PCI Controller ("acpiphp")" sub-driver may be compiled
> as a module.  Kernel modules are instantiated somewhat randomly - the
> order in which they are linked as the kernel as built - thus if there are
> any dependencies on the ordering of attaching sub-drivers, they can not be
> effectively dealt with.
>
> This patch series resolves any potential sequencing inter-dependencies by
> converting "acpiphp" sub-driver's functionality to being only supported as
> statically built-in to the kernel.  Inter-dependencies can then be
> effectively handled by explicitly sequencing the addition of such
> functionality.

some slots may support both acpiphp and pciehp.

if make acpiphp to be built-in, user do not have choice anymore.

Thanks

Yinghai

Re: [PATCH 06/15] PCI/acpiphp: Convert "acpiphp" sub-driver's functionality to built-in only

[Myron Stowe]

On Thu, 2012-12-06 at 22:48 -0800, Yinghai Lu wrote:
> On Thu, Dec 6, 2012 at 10:25 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
> > The "ACPI Hot Plug PCI Controller ("acpiphp")" sub-driver may be compiled
> > as a module.  Kernel modules are instantiated somewhat randomly - the
> > order in which they are linked as the kernel as built - thus if there are
> > any dependencies on the ordering of attaching sub-drivers, they can not be
> > effectively dealt with.
> >
> > This patch series resolves any potential sequencing inter-dependencies by
> > converting "acpiphp" sub-driver's functionality to being only supported as
> > statically built-in to the kernel.  Inter-dependencies can then be
> > effectively handled by explicitly sequencing the addition of such
> > functionality.
> 
> some slots may support both acpiphp and pciehp.
> 
> if make acpiphp to be built-in, user do not have choice anymore.

What area are you worried about and how do we exercise that choice
today?

Here is my current understanding to set a little context -

I see the following call chains with respect to pciehp and acpiphp:

  pcied_init			# pciehp module_init
    pciehp_firmware_init
      pciehp_acpi_slot_detection_init
        parse_detect_mode	#pcie, acpi, auto (default)
    pcie_port_service_register


  acpiphp_init					# module_init
    init_acpi
      acpiphp_glue_init
        acpi_pci_register_driver(&acpi_pci_hp_driver)
          list_for_each_entry(root, &acpi_pci_roots, node)
            driver->add				# for \_SB_.PCI0
              add_bridge
                add_host_bridge
                  bridge = kzalloc(struct acpiphp_bridge)
                  bridge->handle = handle	# \_SB_.PCI0
                  bridge->pci_bus = root_bus	#pci_bus 0000:00
                  init_bridge_misc
                    acpi_walk_namespace(..., register_slot, ...)
                      register_slot
                        if (device_is_managed_by_native_pciehp(pdev))
                          return AE_OK

Does pciehp influence device_is_managed_by_native_pciehp()?  If not, it
seems that the acpiphp driver will never attach.

Looking at ./drivers/pci/hotplug/Makefile, CONFIG_HOTPLUG_PCI_PCIE comes
before CONFIG_HOTPLUG_PCI_ACPI so if pciehp and acpiphp are both
configured as modules then the load order would seem to be:
  acpiphp, pciehp: acpiphp ignores, pciehp claims
  pciehp, acpiphp: pciehp claims, acpiphp ignores

To complete the matrix taking built-in vs. module into account -
  pciehp=y, acpiphp=y
    pciehp, acpiphp: pciehp claims, acpiphp ignores

  pciehp=y, acpiphp=m
    pciehp, acpiphp: pciehp claims, acpiphp ignores

  pciehp=m, acpiphp=y
    acpiphp, pciehp: acpiphp ignores, pciehp claims

The above combinations all seem to end up with pciehp claiming the slot
which raises the question: How do we get acpiphp to claim a slot today?

Thanks,
 Myron 
> 
> Thanks
> 
> Yinghai

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

[Bjorn Helgaas]

On Thu, Dec 6, 2012 at 11:25 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
> From: Yinghai Lu <yinghai@kernel.org>
>
> It causes confusion.

I completely agree that acpiphp causes confusion  :)

> We may only need acpi hp for pci host bridge.
>
> Split host bridge hot-add support to pci_root_hp, and keep acpiphp simple.
>
> -v2: put back pci_root_hp change in one patch
> -v3: add pcibios_resource_survey_bus() calling
> -v4: remove not needed code with remove_bridge
> -v5: put back support for acpiphp support for slots just on root bus.
> -v6: change some functions to *_p2p_* to make it more clean.
> -v7: split hot_added change out.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
> ---
>
>  drivers/acpi/Makefile              |    1
>  drivers/acpi/pci_root_hp.c         |  228 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/hotplug/acpiphp_glue.c |   59 ++-------
>  3 files changed, 244 insertions(+), 44 deletions(-)
>  create mode 100644 drivers/acpi/pci_root_hp.c
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 82422fe..4edfe41 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -36,6 +36,7 @@ acpi-y                                += processor_core.o
>  acpi-y                         += ec.o
>  acpi-$(CONFIG_ACPI_DOCK)       += dock.o
>  acpi-y                         += pci_root.o pci_link.o pci_irq.o pci_bind.o
> +acpi-$(CONFIG_HOTPLUG)         += pci_root_hp.o

I absolutely, 110% agree with splitting out the host bridge hotplug
code from the PCI device hotplug code.

But I don't want to put this in a separate file; I think it should go
directly in pci_root.c.  Putting it in a separate file gives the
illusion that hotplug is something we only do in unusual
circumstances.  But that's wrong -- even at boot-time we should be
using the same hot-plug flow as we do later.

Plus, I want people to be forced to look at the ugliness of this code
every time they look at pci_root.c.  Maybe that will help get it
cleaned up eventually.

For example, as soon as you put this code in pci_root.c instead of
pci_root_hp.c, it becomes obvious that we're keeping a list of host
bridges in both places, and we probably don't need two lists.  And it
seems dubious that acpi_pci_root_hp_init() is an initcall that walks
the namespace looking for host bridges, when acpi_pci_root_add()
already exists and is called for every host bridge.

Bjorn

>  acpi-y                         += power.o
>  acpi-y                         += event.o
>  acpi-y                         += sysfs.o
> diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
> new file mode 100644
> index 0000000..10c12aa
> --- /dev/null
> +++ b/drivers/acpi/pci_root_hp.c
> @@ -0,0 +1,228 @@
> +/*
> + * Separated from drivers/pci/hotplug/acpiphp_glue.c
> + *     only support root bridge
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +
> +static LIST_HEAD(acpi_root_bridge_list);
> +struct acpi_root_bridge {
> +       struct list_head list;
> +       acpi_handle handle;
> +       u32 flags;
> +};
> +
> +/* bridge flags */
> +#define ROOT_BRIDGE_HAS_EJ0    (0x00000002)
> +#define ROOT_BRIDGE_HAS_PS3    (0x00000080)
> +
> +#define ACPI_STA_FUNCTIONING   (0x00000008)
> +
> +static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
> +{
> +       struct acpi_root_bridge *bridge;
> +
> +       list_for_each_entry(bridge, &acpi_root_bridge_list, list)
> +               if (bridge->handle == handle)
> +                       return bridge;
> +
> +       return NULL;
> +}
> +
> +/* allocate and initialize host bridge data structure */
> +static void add_acpi_root_bridge(acpi_handle handle)
> +{
> +       struct acpi_root_bridge *bridge;
> +       acpi_handle dummy_handle;
> +       acpi_status status;
> +
> +       /* if the bridge doesn't have _STA, we assume it is always there */
> +       status = acpi_get_handle(handle, "_STA", &dummy_handle);
> +       if (ACPI_SUCCESS(status)) {
> +               unsigned long long tmp;
> +
> +               status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
> +               if (ACPI_FAILURE(status)) {
> +                       printk(KERN_DEBUG "%s: _STA evaluation failure\n",
> +                                                __func__);
> +                       return;
> +               }
> +               if ((tmp & ACPI_STA_FUNCTIONING) == 0)
> +                       /* don't register this object */
> +                       return;
> +       }
> +
> +       bridge = kzalloc(sizeof(struct acpi_root_bridge), GFP_KERNEL);
> +       if (!bridge)
> +               return;
> +
> +       bridge->handle = handle;
> +
> +       if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
> +               bridge->flags |= ROOT_BRIDGE_HAS_EJ0;
> +       if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
> +               bridge->flags |= ROOT_BRIDGE_HAS_PS3;
> +
> +       list_add(&bridge->list, &acpi_root_bridge_list);
> +}
> +
> +struct acpi_root_hp_work {
> +       struct work_struct work;
> +       acpi_handle handle;
> +       u32 type;
> +       void *context;
> +};
> +
> +static void alloc_acpi_root_hp_work(acpi_handle handle, u32 type,
> +                                       void *context,
> +                                       void (*func)(struct work_struct *work))
> +{
> +       struct acpi_root_hp_work *hp_work;
> +       int ret;
> +
> +       hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
> +       if (!hp_work)
> +               return;
> +
> +       hp_work->handle = handle;
> +       hp_work->type = type;
> +       hp_work->context = context;
> +
> +       INIT_WORK(&hp_work->work, func);
> +       ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
> +       if (!ret)
> +               kfree(hp_work);
> +}
> +
> +static void handle_root_bridge_insertion(acpi_handle handle)
> +{
> +       struct acpi_device *device, *pdevice;
> +       acpi_handle phandle;
> +       int ret_val;
> +
> +       acpi_get_parent(handle, &phandle);
> +       if (acpi_bus_get_device(phandle, &pdevice)) {
> +               printk(KERN_DEBUG "no parent device, assuming NULL\n");
> +               pdevice = NULL;
> +       }
> +       if (!acpi_bus_get_device(handle, &device)) {
> +               /* check if  pci root_bus is removed */
> +               struct acpi_pci_root *root = acpi_driver_data(device);
> +               if (pci_find_bus(root->segment, root->secondary.start))
> +                       return;
> +
> +               printk(KERN_DEBUG "bus exists... trim\n");
> +               /* this shouldn't be in here, so remove
> +                * the bus then re-add it...
> +                */
> +               ret_val = acpi_bus_trim(device, 1);
> +               printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
> +       }
> +       if (acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE)) {
> +               printk(KERN_ERR "cannot add bridge to acpi list\n");
> +               return;
> +       }
> +       if (acpi_bus_start(device))
> +               printk(KERN_ERR "cannot start bridge\n");
> +}
> +
> +static void _handle_hotplug_event_root(struct work_struct *work)
> +{
> +       struct acpi_root_bridge *bridge;
> +       char objname[64];
> +       struct acpi_buffer buffer = { .length = sizeof(objname),
> +                                     .pointer = objname };
> +       struct acpi_root_hp_work *hp_work;
> +       acpi_handle handle;
> +       u32 type;
> +
> +       hp_work = container_of(work, struct acpi_root_hp_work, work);
> +       handle = hp_work->handle;
> +       type = hp_work->type;
> +
> +       bridge = acpi_root_handle_to_bridge(handle);
> +
> +       acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> +
> +       switch (type) {
> +       case ACPI_NOTIFY_BUS_CHECK:
> +               /* bus enumerate */
> +               printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
> +                                objname);
> +               if (!bridge) {
> +                       handle_root_bridge_insertion(handle);
> +                       add_acpi_root_bridge(handle);
> +               }
> +
> +               break;
> +
> +       case ACPI_NOTIFY_DEVICE_CHECK:
> +               /* device check */
> +               printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
> +                                objname);
> +               if (!bridge) {
> +                       handle_root_bridge_insertion(handle);
> +                       add_acpi_root_bridge(handle);
> +               }
> +               break;
> +
> +       default:
> +               printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
> +                                type, objname);
> +               break;
> +       }
> +
> +       kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
> +}
> +
> +static void handle_hotplug_event_root(acpi_handle handle, u32 type,
> +                                       void *context)
> +{
> +       alloc_acpi_root_hp_work(handle, type, context,
> +                               _handle_hotplug_event_root);
> +}
> +
> +static acpi_status __init
> +find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
> +{
> +       char objname[64];
> +       struct acpi_buffer buffer = { .length = sizeof(objname),
> +                                     .pointer = objname };
> +       int *count = (int *)context;
> +
> +       if (!acpi_is_root_bridge(handle))
> +               return AE_OK;
> +
> +       (*count)++;
> +
> +       acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> +
> +       acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> +                               handle_hotplug_event_root, NULL);
> +       printk(KERN_DEBUG "acpi root: %s notify handler installed\n", objname);
> +
> +       add_acpi_root_bridge(handle);
> +
> +       return AE_OK;
> +}
> +
> +static int __init acpi_pci_root_hp_init(void)
> +{
> +       int num = 0;
> +
> +       acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> +               ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
> +
> +       printk(KERN_DEBUG "Found %d acpi root devices\n", num);
> +
> +       return 0;
> +}
> +
> +subsys_initcall(acpi_pci_root_hp_init);
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 3d6d4fd..d0369dc 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -543,10 +543,13 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
>         acpi_status status;
>         acpi_handle handle = bridge->handle;
>
> -       status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> +       if (bridge->type != BRIDGE_TYPE_HOST) {
> +               status = acpi_remove_notify_handler(handle,
> +                                           ACPI_SYSTEM_NOTIFY,
>                                             handle_hotplug_event_bridge);
> -       if (ACPI_FAILURE(status))
> -               err("failed to remove notify handler\n");
> +               if (ACPI_FAILURE(status))
> +                       err("failed to remove notify handler\n");
> +       }
>
>         if ((bridge->type != BRIDGE_TYPE_HOST) &&
>             ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
> @@ -630,9 +633,6 @@ static void remove_bridge(struct acpi_pci_root *root)
>         bridge = acpiphp_handle_to_bridge(handle);
>         if (bridge)
>                 cleanup_bridge(bridge);
> -       else
> -               acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> -                                          handle_hotplug_event_bridge);
>  }
>
>  static int power_on_slot(struct acpiphp_slot *slot)
> @@ -1107,18 +1107,12 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
>  }
>
>  /* Program resources in newly inserted bridge */
> -static int acpiphp_configure_bridge (acpi_handle handle)
> +static int acpiphp_configure_p2p_bridge(acpi_handle handle)
>  {
> -       struct pci_bus *bus;
> +       struct pci_dev *pdev = acpi_get_pci_dev(handle);
> +       struct pci_bus *bus = pdev->subordinate;
>
> -       if (acpi_is_root_bridge(handle)) {
> -               struct acpi_pci_root *root = acpi_pci_find_root(handle);
> -               bus = root->bus;
> -       } else {
> -               struct pci_dev *pdev = acpi_get_pci_dev(handle);
> -               bus = pdev->subordinate;
> -               pci_dev_put(pdev);
> -       }
> +       pci_dev_put(pdev);
>
>         pci_bus_size_bridges(bus);
>         pci_bus_assign_resources(bus);
> @@ -1128,7 +1122,7 @@ static int acpiphp_configure_bridge (acpi_handle handle)
>         return 0;
>  }
>
> -static void handle_bridge_insertion(acpi_handle handle, u32 type)
> +static void handle_p2p_bridge_insertion(acpi_handle handle, u32 type)
>  {
>         struct acpi_device *device, *pdevice;
>         acpi_handle phandle;
> @@ -1148,9 +1142,9 @@ static void handle_bridge_insertion(acpi_handle handle, u32 type)
>                 err("cannot add bridge to acpi list\n");
>                 return;
>         }
> -       if (!acpiphp_configure_bridge(handle) &&
> +       if (!acpiphp_configure_p2p_bridge(handle) &&
>                 !acpi_bus_start(device))
> -               add_bridge(handle);
> +               add_p2p_bridge(handle);
>         else
>                 err("cannot configure and start bridge\n");
>
> @@ -1236,7 +1230,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
>
>         if (acpi_bus_get_device(handle, &device)) {
>                 /* This bridge must have just been physically inserted */
> -               handle_bridge_insertion(handle, type);
> +               handle_p2p_bridge_insertion(handle, type);
>                 goto out;
>         }
>
> @@ -1413,21 +1407,6 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
>                               _handle_hotplug_event_func);
>  }
>
> -static acpi_status
> -find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
> -{
> -       int *count = (int *)context;
> -
> -       if (!acpi_is_root_bridge(handle))
> -               return AE_OK;
> -
> -       (*count)++;
> -       acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> -                                   handle_hotplug_event_bridge, NULL);
> -
> -       return AE_OK ;
> -}
> -
>  static struct acpi_pci_driver acpi_pci_hp_driver = {
>         .add =          add_bridge,
>         .remove =       remove_bridge,
> @@ -1438,15 +1417,7 @@ static struct acpi_pci_driver acpi_pci_hp_driver = {
>   */
>  int __init acpiphp_glue_init(void)
>  {
> -       int num = 0;
> -
> -       acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> -                       ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
> -
> -       if (num <= 0)
> -               return -1;
> -       else
> -               acpi_pci_register_driver(&acpi_pci_hp_driver);
> +       acpi_pci_register_driver(&acpi_pci_hp_driver);
>
>         return 0;
>  }
>

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

[Rafael J. Wysocki]

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

Agreed.

Thanks,
Rafael


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

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

[Yinghai Lu]

On Fri, Dec 7, 2012 at 11:32 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Dec 6, 2012 at 11:25 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
>> From: Yinghai Lu <yinghai@kernel.org>
>>
>>
> For example, as soon as you put this code in pci_root.c instead of
> pci_root_hp.c, it becomes obvious that we're keeping a list of host
> bridges in both places, and we probably don't need two lists.  And it
> seems dubious that acpi_pci_root_hp_init() is an initcall that walks
> the namespace looking for host bridges, when acpi_pci_root_add()
> already exists and is called for every host bridge.

removed the duplicated interface in following patch.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-split-pci-root-hp-2

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

| [PATCH] PCI, ACPI: remove acpi_root_bridge in pci_root_hp
|
| Tang noticed that hotplug through container will not update acpi_root_bridge
| list.
|
| After closely checking, we don't need that for struct for tracking and
| could use acpi_pci_root directly.

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

[Bjorn Helgaas]

On Fri, Dec 7, 2012 at 2:30 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Dec 7, 2012 at 11:32 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Thu, Dec 6, 2012 at 11:25 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
>>> From: Yinghai Lu <yinghai@kernel.org>
>>>
>>>
>> For example, as soon as you put this code in pci_root.c instead of
>> pci_root_hp.c, it becomes obvious that we're keeping a list of host
>> bridges in both places, and we probably don't need two lists.  And it
>> seems dubious that acpi_pci_root_hp_init() is an initcall that walks
>> the namespace looking for host bridges, when acpi_pci_root_add()
>> already exists and is called for every host bridge.
>
> removed the duplicated interface in following patch.
>
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-split-pci-root-hp-2
>
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=a1d1ef3f5e4932bc672d96dedd11c5d58f7f20f5
>
> | [PATCH] PCI, ACPI: remove acpi_root_bridge in pci_root_hp
> |
> | Tang noticed that hotplug through container will not update acpi_root_bridge
> | list.
> |
> | After closely checking, we don't need that for struct for tracking and
> | could use acpi_pci_root directly.

Sorry I missed that; I had only looked at a few patches adjacent to
the one that split out this code.

I still would like the code to be all in pci_root.c.

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

[Yinghai Lu]

On Fri, Dec 7, 2012 at 1:36 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Dec 7, 2012 at 2:30 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>
> Sorry I missed that; I had only looked at a few patches adjacent to
> the one that split out this code.
>
> I still would like the code to be all in pci_root.c.

ok, I will rebase that to put them in pci_root.c
and send them out after you take

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

Yinghai