[RFC PATCH v1 1/6] PCI: change PCI device management logic to better follow device model

[RFC PATCH v1 1/6] PCI: change PCI device management logic to better follow device model

[Jiang Liu]

From: Jiang Liu <liuj97@gmail.com>

According to device model documentation, the way to add/remove device
object should be symmetric.

/**
 * device_del - delete device from system.
 * @dev: device.
 *
 * This is the first part of the device unregistration
 * sequence. This removes the device from the lists we control
 * from here, has it removed from the other driver model
 * subsystems it was added to in device_add(), and removes it
 * from the kobject hierarchy.
 *
 * NOTE: this should be called manually _iff_ device_add() was
 * also called manually.
 */

The rule here is to either use
1) device_register()/device_unregister()
or
2) device_initialize()/device_add()/device_del()/put_device().

So change PCI core to follow the rule and get rid of the redundant
pci_dev_get()/pci_dev_put() pair.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/pci/probe.c  |    1 -
 drivers/pci/remove.c |    4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 0840409..dacca26 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1294,7 +1294,6 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
 	device_initialize(&dev->dev);
 	dev->dev.release = pci_release_dev;
-	pci_dev_get(dev);
 
 	dev->dev.dma_mask = &dev->dma_mask;
 	dev->dev.dma_parms = &dev->dma_parms;
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 04a4861..6c07bc5 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -22,7 +22,7 @@ static void pci_stop_dev(struct pci_dev *dev)
 	if (dev->is_added) {
 		pci_proc_detach_device(dev);
 		pci_remove_sysfs_dev_files(dev);
-		device_unregister(&dev->dev);
+		device_del(&dev->dev);
 		dev->is_added = 0;
 	}
 
@@ -40,7 +40,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
 	up_write(&pci_bus_sem);
 
 	pci_free_resources(dev);
-	pci_dev_put(dev);
+	put_device(&dev->dev);
 }
 
 /**
-- 
1.7.1

[RFC PATCH v1 2/6] PCI: split PCI bus device registration into two stages

[Jiang Liu]

From: Jiang Liu <liuj97@gmail.com>

When handling BUS_NOTIFY_ADD_DEVICE event for a new PCI bridge
device, the notification handler can't hold reference count
to the new PCI bus because the device object for the new bus
(pci_dev->subordinate->dev) hasn't been initialized yet.

Split the PCI bus device registration into two stages as below,
so that the event handler could hold reference counts to the new
PCI bus when handling BUS_NOTIFY_ADD_DEVICE event.

1) device_initialize(&pci_dev->dev)
2) device_initialize(&pci_dev->subordinate->dev)
3) notify BUS_NOTIFY_ADD_DEVICE event for pci_dev
4) device_add(&pci_dev->dev)
5) device_add(&pci_dev->subordinate->dev)

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/pci/bus.c    |    2 +-
 drivers/pci/probe.c  |    1 +
 drivers/pci/remove.c |   10 +++++-----
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 4ce5ef2..e2a0c52 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -187,7 +187,7 @@ int pci_bus_add_child(struct pci_bus *bus)
 	if (bus->bridge)
 		bus->dev.parent = bus->bridge;
 
-	retval = device_register(&bus->dev);
+	retval = device_add(&bus->dev);
 	if (retval)
 		return retval;
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index dacca26..47ffa6c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -667,6 +667,7 @@ struct pci_bus *__ref pci_add_new_bus(struct pci_bus *parent, struct pci_dev *de
 		down_write(&pci_bus_sem);
 		list_add_tail(&child->node, &parent->children);
 		up_write(&pci_bus_sem);
+		device_initialize(&child->dev);
 	}
 	return child;
 }
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 6c07bc5..edbd117 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -70,11 +70,11 @@ void pci_remove_bus(struct pci_bus *pci_bus)
 	list_del(&pci_bus->node);
 	pci_bus_release_busn_res(pci_bus);
 	up_write(&pci_bus_sem);
-	if (!pci_bus->is_added)
-		return;
-
-	pci_remove_legacy_files(pci_bus);
-	device_unregister(&pci_bus->dev);
+	if (pci_bus->is_added) {
+		pci_remove_legacy_files(pci_bus);
+		device_del(&pci_bus->dev);
+	}
+	put_device(&pci_bus->dev);
 }
 EXPORT_SYMBOL(pci_remove_bus);
 
-- 
1.7.1

[RFC PATCH v1 3/6] PCI, ACPI: correctly update binding info when PCI hotplug event happens

[Jiang Liu]

From: Jiang Liu <liuj97@gmail.com>

Currently acpi/pci_bind.c is used to maintain binding relationship
among ACPI and PCI devices. But it's broken when dealing PCI hotplug
events.

For the acpiphp driver, it's designed to update the binding relationship
when PCI hotplug event happensr, but the implementation is broken.
For PCI device hot-adding:
enable_device()
    pci_scan_slot()
    acpiphp_bus_add()
        acpi_bus_add()
	    acpi_pci_bind()
		acpi_get_pci_dev()
		    return NULL because dev->archdata.acpi_handle is
		    still unset
		return without updating actual binding relationship
    pci_bus_add_devices()
	pci_bus_add_device()
	    device_add()
		platform_notify()
		    acpi_bind_one()
			set dev->archdata.acpi_handle

For PCI device hot-removal,
disable_device()
    pci_stop_bus_device()
    __pci_remove_bus_device()
    acpiphp_bus_trim()
	acpi_bus_remove()
	    acpi_pci_unbind()
		return without really unbinding because PCI device has
		already been destroyed

For other PCI hotplug drivers, such as pciehp etc, they even don't
bother to update binding relationship. So hook into acpi_bind_one()
and acpi_unbind_one() in drivers/acpi/glue.c to update PCI/ACPI
binding relationship.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/acpi/glue.c     |    4 ++
 drivers/acpi/internal.h |    2 +
 drivers/acpi/pci_bind.c |  103 ++++++++++++++++++++++++++++++-----------------
 drivers/acpi/pci_root.c |   22 ----------
 4 files changed, 72 insertions(+), 59 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 29a4a5c..1a07c3c 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -160,6 +160,8 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle)
 	}
 	dev->archdata.acpi_handle = handle;
 
+	acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true);
+
 	status = acpi_bus_get_device(handle, &acpi_dev);
 	if (!ACPI_FAILURE(status)) {
 		int ret;
@@ -191,6 +193,8 @@ static int acpi_unbind_one(struct device *dev)
 			sysfs_remove_link(&acpi_dev->dev.kobj, "physical_node");
 		}
 
+		acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, false);
+
 		acpi_detach_data(dev->archdata.acpi_handle,
 				 acpi_glue_data_handler);
 		dev->archdata.acpi_handle = NULL;
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index ca75b9c..5396b20 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -93,4 +93,6 @@ static inline int suspend_nvs_save(void) { return 0; }
 static inline void suspend_nvs_restore(void) {}
 #endif
 
+void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind);
+
 #endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 2ef0409..208e6cd 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -35,54 +35,56 @@
 #define _COMPONENT		ACPI_PCI_COMPONENT
 ACPI_MODULE_NAME("pci_bind");
 
-static int acpi_pci_unbind(struct acpi_device *device)
-{
-	struct pci_dev *dev;
+static int acpi_pci_bind(struct acpi_device *device);
 
-	dev = acpi_get_pci_dev(device->handle);
-	if (!dev)
-		goto out;
-
-	device_set_run_wake(&dev->dev, false);
+static int __acpi_pci_unbind(struct acpi_device *device, struct pci_dev *pdev)
+{
+	device_set_run_wake(&pdev->dev, false);
 	pci_acpi_remove_pm_notifier(device);
 
-	if (!dev->subordinate)
-		goto out;
+	if (pdev->subordinate) {
+		acpi_pci_irq_del_prt(pdev->subordinate);
+		device->ops.bind = NULL;
+		device->ops.unbind = NULL;
+	}
 
-	acpi_pci_irq_del_prt(dev->subordinate);
+	return 0;
+}
 
-	device->ops.bind = NULL;
-	device->ops.unbind = NULL;
+static int acpi_pci_unbind(struct acpi_device *device)
+{
+	int rc = 0;
+	struct pci_dev *pdev;
 
-out:
-	pci_dev_put(dev);
-	return 0;
+	pdev = acpi_get_pci_dev(device->handle);
+	if (pdev) {
+		rc = __acpi_pci_unbind(device, pdev);
+		pci_dev_put(pdev);
+	}
+
+	return rc;
 }
 
-static int acpi_pci_bind(struct acpi_device *device)
+static int __acpi_pci_bind(struct acpi_device *device, struct pci_dev *pdev)
 {
 	acpi_status status;
 	acpi_handle handle;
 	struct pci_bus *bus;
-	struct pci_dev *dev;
 
-	dev = acpi_get_pci_dev(device->handle);
-	if (!dev)
-		return 0;
-
-	pci_acpi_add_pm_notifier(device, dev);
+	pci_acpi_add_pm_notifier(device, pdev);
 	if (device->wakeup.flags.run_wake)
-		device_set_run_wake(&dev->dev, true);
+		device_set_run_wake(&pdev->dev, true);
 
 	/*
 	 * Install the 'bind' function to facilitate callbacks for
 	 * children of the P2P bridge.
 	 */
-	if (dev->subordinate) {
+	if (pdev->subordinate) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Device %04x:%02x:%02x.%d is a PCI bridge\n",
-				  pci_domain_nr(dev->bus), dev->bus->number,
-				  PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn)));
+				  pci_domain_nr(pdev->bus), pdev->bus->number,
+				  PCI_SLOT(pdev->devfn),
+				  PCI_FUNC(pdev->devfn)));
 		device->ops.bind = acpi_pci_bind;
 		device->ops.unbind = acpi_pci_unbind;
 	}
@@ -96,19 +98,29 @@ static int acpi_pci_bind(struct acpi_device *device)
 	 * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
 	 */
 	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
-	if (ACPI_FAILURE(status))
-		goto out;
+	if (ACPI_SUCCESS(status)) {
+		if (pdev->subordinate)
+			bus = pdev->subordinate;
+		else
+			bus = pdev->bus;
+		acpi_pci_irq_add_prt(device->handle, bus);
+	}
+
+	return 0;
+}
 
-	if (dev->subordinate)
-		bus = dev->subordinate;
-	else
-		bus = dev->bus;
+static int acpi_pci_bind(struct acpi_device *device)
+{
+	int rc = 0;
+	struct pci_dev *pdev;
 
-	acpi_pci_irq_add_prt(device->handle, bus);
+	pdev = acpi_get_pci_dev(device->handle);
+	if (pdev) {
+		rc = __acpi_pci_bind(device, pdev);
+		pci_dev_put(pdev);
+	}
 
-out:
-	pci_dev_put(dev);
-	return 0;
+	return rc;
 }
 
 int acpi_pci_bind_root(struct acpi_device *device)
@@ -118,3 +130,20 @@ int acpi_pci_bind_root(struct acpi_device *device)
 
 	return 0;
 }
+
+void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind)
+{
+	struct acpi_device *acpi_dev;
+
+	if (!dev_is_pci(dev))
+		return;
+	if (acpi_bus_get_device(handle, &acpi_dev))
+		return;
+	if (!acpi_dev || !acpi_dev->parent)
+		return;
+
+	if (bind && acpi_dev->parent->ops.bind)
+		__acpi_pci_bind(acpi_dev, to_pci_dev(dev));
+	else if (acpi_dev->parent->ops.unbind)
+		__acpi_pci_unbind(acpi_dev, to_pci_dev(dev));
+}
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 7aff631..cd2f7af 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -195,21 +195,6 @@ static acpi_status try_get_root_bridge_busnr(acpi_handle handle,
 	return AE_OK;
 }
 
-static void acpi_pci_bridge_scan(struct acpi_device *device)
-{
-	int status;
-	struct acpi_device *child = NULL;
-
-	if (device->flags.bus_address)
-		if (device->parent && device->parent->ops.bind) {
-			status = device->parent->ops.bind(device);
-			if (!status) {
-				list_for_each_entry(child, &device->children, node)
-					acpi_pci_bridge_scan(child);
-			}
-		}
-}
-
 static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
 
 static acpi_status acpi_pci_run_osc(acpi_handle handle,
@@ -456,7 +441,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	int result;
 	struct acpi_pci_root *root;
 	acpi_handle handle;
-	struct acpi_device *child;
 	u32 flags, base_flags;
 
 	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
@@ -557,12 +541,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	if (ACPI_SUCCESS(status))
 		result = acpi_pci_irq_add_prt(device->handle, root->bus);
 
-	/*
-	 * Scan and bind all _ADR-Based Devices
-	 */
-	list_for_each_entry(child, &device->children, node)
-		acpi_pci_bridge_scan(child);
-
 	/* Indicate support for various _OSC capabilities. */
 	if (pci_ext_cfg_avail(root->bus->self))
 		flags |= OSC_EXT_PCI_CONFIG_SUPPORT;
-- 
1.7.1

[RFC PATCH v1 4/6] PCI, ACPI: update PCI slot information when PCI hotplug event happens

[Jiang Liu]

From: Jiang Liu <liuj97@gmail.com>

Currently the pci_slot driver doesn't update PCI slot information
when PCI device hotplug event happens, which may cause memory leak
and returning stale information to user.

So hook the BUS_NOTIFY_ADD_DEVICE/BUS_NOTIFY_DEL_DEVICE events to
update PCI slot information when PCI hotplug event happens.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/acpi/pci_slot.c |   84 +++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index e50e31a..22cc00e 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -32,6 +32,7 @@
 #include <linux/acpi.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
+#include <linux/pci-acpi.h>
 #include <linux/dmi.h>
 
 static bool debug;
@@ -123,12 +124,7 @@ struct callback_args {
 /*
  * register_slot
  *
- * Called once for each SxFy object in the namespace. Don't worry about
- * calling pci_create_slot multiple times for the same pci_bus:device,
- * since each subsequent call simply bumps the refcount on the pci_slot.
- *
- * The number of calls to pci_destroy_slot from unregister_slot is
- * symmetrical.
+ * Called once for each SxFy object in the namespace.
  */
 static acpi_status
 register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
@@ -145,6 +141,18 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	if (device < 0)
 		return AE_OK;
 
+	/* Avoid duplicated records for the same slot */
+	mutex_lock(&slot_list_lock);
+	list_for_each_entry(slot, &slot_list, list) {
+		pci_slot = slot->pci_slot;
+		if (pci_slot && pci_slot->bus == pci_bus &&
+		    pci_slot->number == device) {
+			mutex_unlock(&slot_list_lock);
+			return AE_OK;
+		}
+	}
+	mutex_unlock(&slot_list_lock);
+
 	slot = kmalloc(sizeof(*slot), GFP_KERNEL);
 	if (!slot) {
 		err("%s: cannot allocate memory\n", __func__);
@@ -354,17 +362,81 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = {
 	{}
 };
 
+static void acpi_pci_slot_notify_add(struct pci_dev *dev)
+{
+	acpi_handle handle;
+	struct callback_args context;
+
+	if (!dev->subordinate)
+		return;
+
+	handle = DEVICE_ACPI_HANDLE(&dev->dev);
+	context.root_handle = acpi_find_root_bridge_handle(dev);
+	if (handle && context.root_handle) {
+		context.pci_bus = dev->subordinate;
+		context.user_function = register_slot;
+		acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
+				    register_slot, NULL, &context, NULL);
+	}
+}
+
+static void acpi_pci_slot_notify_del(struct pci_dev *dev)
+{
+	struct acpi_pci_slot *slot, *tmp;
+	struct pci_bus *bus = dev->subordinate;
+
+	if (!bus)
+		return;
+
+	mutex_lock(&slot_list_lock);
+	list_for_each_entry_safe(slot, tmp, &slot_list, list) {
+		if (slot->pci_slot && slot->pci_slot->bus == bus) {
+			list_del(&slot->list);
+			pci_destroy_slot(slot->pci_slot);
+			put_device(&bus->dev);
+			kfree(slot);
+		}
+	}
+	mutex_unlock(&slot_list_lock);
+}
+
+static int acpi_pci_slot_notify_fn(struct notifier_block *nb,
+				   unsigned long event, void *data)
+{
+	struct device *dev = data;
+
+	switch (event) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		acpi_pci_slot_notify_add(to_pci_dev(dev));
+		break;
+	case BUS_NOTIFY_DEL_DEVICE:
+		acpi_pci_slot_notify_del(to_pci_dev(dev));
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_pci_slot_notifier = {
+	.notifier_call = &acpi_pci_slot_notify_fn,
+};
+
 static int __init
 acpi_pci_slot_init(void)
 {
 	dmi_check_system(acpi_pci_slot_dmi_table);
 	acpi_pci_register_driver(&acpi_pci_slot_driver);
+	bus_register_notifier(&pci_bus_type, &acpi_pci_slot_notifier);
+
 	return 0;
 }
 
 static void __exit
 acpi_pci_slot_exit(void)
 {
+	bus_unregister_notifier(&pci_bus_type, &acpi_pci_slot_notifier);
 	acpi_pci_unregister_driver(&acpi_pci_slot_driver);
 }
 
-- 
1.7.1

[RFC PATCH v1 5/6] PCI, ACPI: update ACPI hotplug slot information when PCI hotplug event happens

[Jiang Liu]

From: Jiang Liu <liuj97@gmail.com>

Currently the acpiphp driver fails to update hotplug slot information under
several conditions, such as:
1) The bridge device is removed through /sys/bus/pci/devices/.../remove
2) The bridge device is added/removed by PCI hotplug driver other than the
   acpiphp driver itself. For example, if an IO expansion box with ACPI
   hotplug slots available is hot-added by the pciehp driver, all ACPI
   hotplug slots won't be discovered by the acpiphp driver.

So hook the BUS_NOTIFY_ADD_DEVICE/BUS_NOTIFY_DEL_DEVICE events to
update ACPI hotplug slot information when PCI hotplug event happens.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/pci/hotplug/acpiphp_glue.c |   64 ++++++++++++++++++++++++++++++++++--
 1 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 73af337..44d055c 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -61,6 +61,7 @@ static void handle_hotplug_event_bridge (acpi_handle, u32, void *);
 static void acpiphp_sanitize_bus(struct pci_bus *bus);
 static void acpiphp_set_hpp_values(struct pci_bus *bus);
 static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context);
+static struct acpiphp_bridge *acpiphp_handle_to_bridge(acpi_handle handle);
 
 /* callback routine to check for the existence of a pci dock device */
 static acpi_status
@@ -377,7 +378,7 @@ static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge)
 
 
 /* allocate and initialize host bridge data structure */
-static void add_host_bridge(acpi_handle *handle)
+static void add_host_bridge(acpi_handle handle)
 {
 	struct acpiphp_bridge *bridge;
 	struct acpi_pci_root *root = acpi_pci_find_root(handle);
@@ -396,12 +397,14 @@ static void add_host_bridge(acpi_handle *handle)
 	init_bridge_misc(bridge);
 }
 
-
 /* allocate and initialize PCI-to-PCI bridge data structure */
-static void add_p2p_bridge(acpi_handle *handle)
+static void add_p2p_bridge(acpi_handle handle)
 {
 	struct acpiphp_bridge *bridge;
 
+	if (acpiphp_handle_to_bridge(handle))
+		return;
+
 	bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
 	if (bridge == NULL) {
 		err("out of memory\n");
@@ -1419,6 +1422,58 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
 	return AE_OK ;
 }
 
+static void acpiphp_hp_notify_add(struct pci_dev *dev)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+	if (!dev->subordinate || !handle)
+		return;
+
+	/* check if this bridge has ejectable slots */
+	if (detect_ejectable_slots(handle) > 0) {
+		dbg("found PCI-to-PCI bridge at PCI %s\n", pci_name(dev));
+		add_p2p_bridge(handle);
+	}
+}
+
+static void acpiphp_hp_notify_del(struct pci_dev *dev)
+{
+	struct acpiphp_bridge *bridge, *tmp;
+	struct pci_bus *bus = dev->subordinate;
+
+	if (!bus)
+		return;
+
+	list_for_each_entry_safe(bridge, tmp, &bridge_list, list)
+		if (bridge->pci_bus == bus) {
+			cleanup_bridge(bridge);
+			break;
+		}
+}
+
+static int acpi_pci_hp_notify_fn(struct notifier_block *nb,
+				 unsigned long event, void *data)
+{
+	struct device *dev = data;
+
+	switch (event) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		acpiphp_hp_notify_add(to_pci_dev(dev));
+		break;
+	case BUS_NOTIFY_DEL_DEVICE:
+		acpiphp_hp_notify_del(to_pci_dev(dev));
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_pci_hp_notifier = {
+	.notifier_call = &acpi_pci_hp_notify_fn,
+};
+
 static struct acpi_pci_driver acpi_pci_hp_driver = {
 	.add =		add_bridge,
 	.remove =	remove_bridge,
@@ -1439,6 +1494,8 @@ int __init acpiphp_glue_init(void)
 	else
 		acpi_pci_register_driver(&acpi_pci_hp_driver);
 
+	bus_register_notifier(&pci_bus_type, &acpi_pci_hp_notifier);
+
 	return 0;
 }
 
@@ -1450,6 +1507,7 @@ int __init acpiphp_glue_init(void)
  */
 void  acpiphp_glue_exit(void)
 {
+	bus_unregister_notifier(&pci_bus_type, &acpi_pci_hp_notifier);
 	acpi_pci_unregister_driver(&acpi_pci_hp_driver);
 }
 
-- 
1.7.1

[RFC PATCH v1 6/6] PCI: update AER configuration when PCI hotplug event happens

[Jiang Liu]

From: Jiang Liu <liuj97@gmail.com>

The AER driver only configures downstream PCIe devices at driver
binding time and all hot-added PCIe devices won't be managed by
the AER driver.  So hook PCI device hotplug events to setup AER
configuration for hot-added PCIe devices.

So hook the BUS_NOTIFY_ADD_DEVICE/BUS_NOTIFY_DEL_DEVICE events to
setup AER configuration for hot-added PCIe devices.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/pci/pcie/aer/aerdrv.c |   72 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 71 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c
index 58ad791..7033410 100644
--- a/drivers/pci/pcie/aer/aerdrv.c
+++ b/drivers/pci/pcie/aer/aerdrv.c
@@ -410,6 +410,69 @@ static void aer_error_resume(struct pci_dev *dev)
 	pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, status);
 }
 
+static int pci_root_port_aer_support(struct pci_dev *dev)
+{
+	int rc, pos;
+	u32 reg32 = 0;
+
+	if (!pci_is_pcie(dev))
+		return false;
+
+	if (!pci_aer_available() || aer_acpi_firmware_first())
+		return false;
+
+	/* Search for root port */
+	while (dev->pcie_type != PCI_EXP_TYPE_ROOT_PORT) {
+		dev = dev->bus->self;
+		if (!dev)
+			return false;
+	}
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
+	if (!pos)
+		return false;
+
+	rc = pci_read_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, &reg32);
+	if (rc || reg32 == 0xFFFFFFFF)
+		return false;
+
+	if (reg32 & (PCI_ERR_ROOT_CMD_COR_EN |
+		     PCI_ERR_ROOT_CMD_NONFATAL_EN |
+		     PCI_ERR_ROOT_CMD_FATAL_EN))
+		return true;
+
+	return false;
+}
+
+static void acpi_pcie_aer_notify_dev(struct pci_dev *dev, bool enable)
+{
+	if (pci_root_port_aer_support(dev))
+		set_device_error_reporting(dev, &enable);
+}
+
+static int acpi_pcie_aer_notify_fn(struct notifier_block *nb,
+		unsigned long event, void *data)
+{
+	struct device *dev = data;
+
+	switch (event) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		acpi_pcie_aer_notify_dev(to_pci_dev(dev), true);
+		break;
+	case BUS_NOTIFY_DEL_DEVICE:
+		acpi_pcie_aer_notify_dev(to_pci_dev(dev), false);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_pcie_aer_notifier = {
+	.notifier_call = &acpi_pcie_aer_notify_fn,
+};
+
 /**
  * aer_service_init - register AER root service driver
  *
@@ -417,9 +480,15 @@ static void aer_error_resume(struct pci_dev *dev)
  */
 static int __init aer_service_init(void)
 {
+	int ret;
+
 	if (!pci_aer_available() || aer_acpi_firmware_first())
 		return -ENXIO;
-	return pcie_port_service_register(&aerdriver);
+	ret = pcie_port_service_register(&aerdriver);
+	if (!ret)
+		bus_register_notifier(&pci_bus_type, &acpi_pcie_aer_notifier);
+
+	return ret;
 }
 
 /**
@@ -429,6 +498,7 @@ static int __init aer_service_init(void)
  */
 static void __exit aer_service_exit(void)
 {
+	bus_unregister_notifier(&pci_bus_type, &acpi_pcie_aer_notifier);
 	pcie_port_service_unregister(&aerdriver);
 }
 
-- 
1.7.1

Re: [RFC PATCH v1 1/6] PCI: change PCI device management logic to better follow device model

[Taku Izumi]

Hi Jiang,

   Looks good for me.

 Best regards,
 Taku Izumi

On Mon, 04 Jun 2012 13:16:57 +0800
Jiang Liu <jiang.liu@huawei.com> wrote:

> From: Jiang Liu <liuj97@gmail.com>
> 
> According to device model documentation, the way to add/remove device
> object should be symmetric.
> 
> /**
>  * device_del - delete device from system.
>  * @dev: device.
>  *
>  * This is the first part of the device unregistration
>  * sequence. This removes the device from the lists we control
>  * from here, has it removed from the other driver model
>  * subsystems it was added to in device_add(), and removes it
>  * from the kobject hierarchy.
>  *
>  * NOTE: this should be called manually _iff_ device_add() was
>  * also called manually.
>  */
> 
> The rule here is to either use
> 1) device_register()/device_unregister()
> or
> 2) device_initialize()/device_add()/device_del()/put_device().
> 
> So change PCI core to follow the rule and get rid of the redundant
> pci_dev_get()/pci_dev_put() pair.
> 
> Signed-off-by: Jiang Liu <liuj97@gmail.com>
> ---
>  drivers/pci/probe.c  |    1 -
>  drivers/pci/remove.c |    4 ++--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 0840409..dacca26 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1294,7 +1294,6 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
>  	device_initialize(&dev->dev);
>  	dev->dev.release = pci_release_dev;
> -	pci_dev_get(dev);
>  
>  	dev->dev.dma_mask = &dev->dma_mask;
>  	dev->dev.dma_parms = &dev->dma_parms;
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 04a4861..6c07bc5 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -22,7 +22,7 @@ static void pci_stop_dev(struct pci_dev *dev)
>  	if (dev->is_added) {
>  		pci_proc_detach_device(dev);
>  		pci_remove_sysfs_dev_files(dev);
> -		device_unregister(&dev->dev);
> +		device_del(&dev->dev);
>  		dev->is_added = 0;
>  	}
>  
> @@ -40,7 +40,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
>  	up_write(&pci_bus_sem);
>  
>  	pci_free_resources(dev);
> -	pci_dev_put(dev);
> +	put_device(&dev->dev);
>  }
>  
>  /**
> -- 
> 1.7.1
> 
> 
> 

Re: [RFC PATCH v1 1/6] PCI: change PCI device management logic to better follow device model

[Bjorn Helgaas]

On Sun, Jun 3, 2012 at 11:16 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
> From: Jiang Liu <liuj97@gmail.com>
>
> According to device model documentation, the way to add/remove device
> object should be symmetric.

I think this 6-patch series has been folded into your "[RFC PATCH v1
00/22] introduce PCI bus lock to serialize PCI hotplug operations"
series posted August 8, right?

I'll ignore these 6 patches on that assumption, so let me know if otherwise.

> /**
>  * device_del - delete device from system.
>  * @dev: device.
>  *
>  * This is the first part of the device unregistration
>  * sequence. This removes the device from the lists we control
>  * from here, has it removed from the other driver model
>  * subsystems it was added to in device_add(), and removes it
>  * from the kobject hierarchy.
>  *
>  * NOTE: this should be called manually _iff_ device_add() was
>  * also called manually.
>  */
>
> The rule here is to either use
> 1) device_register()/device_unregister()
> or
> 2) device_initialize()/device_add()/device_del()/put_device().
>
> So change PCI core to follow the rule and get rid of the redundant
> pci_dev_get()/pci_dev_put() pair.
>
> Signed-off-by: Jiang Liu <liuj97@gmail.com>
> ---
>  drivers/pci/probe.c  |    1 -
>  drivers/pci/remove.c |    4 ++--
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 0840409..dacca26 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1294,7 +1294,6 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
>         device_initialize(&dev->dev);
>         dev->dev.release = pci_release_dev;
> -       pci_dev_get(dev);
>
>         dev->dev.dma_mask = &dev->dma_mask;
>         dev->dev.dma_parms = &dev->dma_parms;
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 04a4861..6c07bc5 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -22,7 +22,7 @@ static void pci_stop_dev(struct pci_dev *dev)
>         if (dev->is_added) {
>                 pci_proc_detach_device(dev);
>                 pci_remove_sysfs_dev_files(dev);
> -               device_unregister(&dev->dev);
> +               device_del(&dev->dev);
>                 dev->is_added = 0;
>         }
>
> @@ -40,7 +40,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
>         up_write(&pci_bus_sem);
>
>         pci_free_resources(dev);
> -       pci_dev_put(dev);
> +       put_device(&dev->dev);
>  }
>
>  /**
> --
> 1.7.1
>
>