linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 4/9] ACPI/pci_bind: correctly update binding relationship for PCI hotplug
       [not found] <1347678312-11124-1-git-send-email-jiang.liu@huawei.com>
@ 2012-09-15  3:05 ` Jiang Liu
  2012-09-15  3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
  2012-09-15  3:05 ` [PATCH v2 6/9] ACPI/pci_slot: update PCI slot information when PCI hotplug event happens Jiang Liu
  2 siblings, 0 replies; 9+ messages in thread
From: Jiang Liu @ 2012-09-15  3:05 UTC (permalink / raw)
  To: Bjorn Helgaas, Len Brown
  Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
	Jiang Liu, linux-kernel, linux-pci, linux-acpi

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

Currently pci_bind.c is used to maintain binding relationship between
ACPI and PCI devices. But it's broken when handling PCI hotplug events.

For the acpiphp driver, it's designed to update the binding relationship
when PCI hotplug event happens, 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, they even don't bother to update binding
relationships. So hook into acpi_bind_one()/acpi_unbind_one() in
drivers/acpi/glue.c to update PCI<->ACPI binding relationship.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/acpi/glue.c     |    6 ++-
 drivers/acpi/internal.h |    2 +
 drivers/acpi/pci_bind.c |  101 +++++++++++++++++++++++++++++++----------------
 drivers/acpi/pci_root.c |   40 +++++--------------
 4 files changed, 81 insertions(+), 68 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 243ee85..4904700 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -121,7 +121,6 @@ acpi_handle acpi_get_child(acpi_handle parent, u64 address)
 			    1, do_acpi_find_child, NULL, &find, NULL);
 	return find.handle;
 }
-
 EXPORT_SYMBOL(acpi_get_child);
 
 /* Link ACPI devices with physical devices */
@@ -142,7 +141,6 @@ struct device *acpi_get_physical_device(acpi_handle handle)
 		return get_device(dev);
 	return NULL;
 }
-
 EXPORT_SYMBOL(acpi_get_physical_device);
 
 static int acpi_bind_one(struct device *dev, acpi_handle handle)
@@ -162,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;
@@ -193,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..320e698 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -35,40 +35,41 @@
 #define _COMPONENT		ACPI_PCI_COMPONENT
 ACPI_MODULE_NAME("pci_bind");
 
-static int acpi_pci_unbind(struct acpi_device *device)
-{
-	struct pci_dev *dev;
-
-	dev = acpi_get_pci_dev(device->handle);
-	if (!dev)
-		goto out;
+static int acpi_pci_bind_cb(struct acpi_device *device);
 
+static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev)
+{
 	device_set_run_wake(&dev->dev, false);
 	pci_acpi_remove_pm_notifier(device);
 
-	if (!dev->subordinate)
-		goto out;
+	if (dev->subordinate) {
+		acpi_pci_irq_del_prt(dev->subordinate);
+		device->ops.bind = NULL;
+		device->ops.unbind = NULL;
+	}
+
+	return 0;
+}
 
-	acpi_pci_irq_del_prt(dev->subordinate);
+static int acpi_pci_unbind_cb(struct acpi_device *device)
+{
+	int rc = 0;
+	struct pci_dev *dev;
 
-	device->ops.bind = NULL;
-	device->ops.unbind = NULL;
+	dev = acpi_get_pci_dev(device->handle);
+	if (dev) {
+		rc = acpi_pci_unbind(device, dev);
+		pci_dev_put(dev);
+	}
 
-out:
-	pci_dev_put(dev);
-	return 0;
+	return rc;
 }
 
-static int acpi_pci_bind(struct acpi_device *device)
+static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
 {
 	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);
 	if (device->wakeup.flags.run_wake)
@@ -83,8 +84,8 @@ static int acpi_pci_bind(struct acpi_device *device)
 				  "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)));
-		device->ops.bind = acpi_pci_bind;
-		device->ops.unbind = acpi_pci_unbind;
+		device->ops.bind = acpi_pci_bind_cb;
+		device->ops.unbind = acpi_pci_unbind_cb;
 	}
 
 	/*
@@ -96,25 +97,55 @@ 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 (dev->subordinate)
+			bus = dev->subordinate;
+		else
+			bus = dev->bus;
+		acpi_pci_irq_add_prt(device->handle, bus);
+	}
 
-	if (dev->subordinate)
-		bus = dev->subordinate;
-	else
-		bus = dev->bus;
+	return 0;
+}
 
-	acpi_pci_irq_add_prt(device->handle, bus);
+static int acpi_pci_bind_cb(struct acpi_device *device)
+{
+	int rc = 0;
+	struct pci_dev *dev;
 
-out:
-	pci_dev_put(dev);
-	return 0;
+	dev = acpi_get_pci_dev(device->handle);
+	if (dev) {
+		rc = acpi_pci_bind(device, dev);
+		pci_dev_put(dev);
+	}
+
+	return rc;
 }
 
 int acpi_pci_bind_root(struct acpi_device *device)
 {
-	device->ops.bind = acpi_pci_bind;
-	device->ops.unbind = acpi_pci_unbind;
+	device->ops.bind = acpi_pci_bind_cb;
+	device->ops.unbind = acpi_pci_unbind_cb;
 
 	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) || !acpi_dev)
+		return;
+
+	if (acpi_dev->parent) {
+		if (bind) {
+			if (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 72a2c98..7509034 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);
@@ -521,6 +505,15 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	/* TBD: Locking */
 	list_add_tail(&root->node, &acpi_pci_roots);
 
+	/*
+	 * Attach ACPI-PCI Context
+	 * -----------------------
+	 * Thus binding the ACPI and PCI devices.
+	 */
+	result = acpi_pci_bind_root(device);
+	if (result)
+		goto end;
+
 	printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
 	       acpi_device_name(device), acpi_device_bid(device),
 	       root->segment, &root->secondary);
@@ -542,15 +535,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	}
 
 	/*
-	 * Attach ACPI-PCI Context
-	 * -----------------------
-	 * Thus binding the ACPI and PCI devices.
-	 */
-	result = acpi_pci_bind_root(device);
-	if (result)
-		goto end;
-
-	/*
 	 * PCI Routing Table
 	 * -----------------
 	 * Evaluate and parse _PRT, if exists.
@@ -559,12 +543,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.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
       [not found] <1347678312-11124-1-git-send-email-jiang.liu@huawei.com>
  2012-09-15  3:05 ` [PATCH v2 4/9] ACPI/pci_bind: correctly update binding relationship for PCI hotplug Jiang Liu
@ 2012-09-15  3:05 ` Jiang Liu
  2012-09-15 18:53   ` Yinghai Lu
  2012-09-15 23:27   ` Yinghai Lu
  2012-09-15  3:05 ` [PATCH v2 6/9] ACPI/pci_slot: update PCI slot information when PCI hotplug event happens Jiang Liu
  2 siblings, 2 replies; 9+ messages in thread
From: Jiang Liu @ 2012-09-15  3:05 UTC (permalink / raw)
  To: Bjorn Helgaas, Len Brown
  Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
	Jiang Liu, linux-kernel, linux-pci, linux-acpi

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

Now ACPI devices are created before/destroyed after corresponding PCI
devices, and acpi_platform_notify/acpi_platform_notify_remove will
update PCI<->ACPI binding relationship when creating/destroying PCI
devices, there's no need to invoke bind/unbind callbacks from ACPI
device probe/destroy routines anymore. So remove bind/unbind callbacks
from acpi_device_ops.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/acpi/pci_bind.c     |  100 +++++++++----------------------------------
 drivers/acpi/pci_root.c     |    9 ----
 drivers/acpi/scan.c         |   21 +--------
 include/acpi/acpi_bus.h     |    4 --
 include/acpi/acpi_drivers.h |    1 -
 5 files changed, 23 insertions(+), 112 deletions(-)

diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 320e698..d18316a 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -35,57 +35,31 @@
 #define _COMPONENT		ACPI_PCI_COMPONENT
 ACPI_MODULE_NAME("pci_bind");
 
-static int acpi_pci_bind_cb(struct acpi_device *device);
-
-static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev)
+static int acpi_pci_unbind(acpi_handle handle, struct pci_dev *dev)
 {
-	device_set_run_wake(&dev->dev, false);
-	pci_acpi_remove_pm_notifier(device);
+	struct acpi_device *acpi_dev;
 
-	if (dev->subordinate) {
-		acpi_pci_irq_del_prt(dev->subordinate);
-		device->ops.bind = NULL;
-		device->ops.unbind = NULL;
+	if (!acpi_bus_get_device(handle, &acpi_dev) && acpi_dev) {
+		device_set_run_wake(&dev->dev, false);
+		pci_acpi_remove_pm_notifier(acpi_dev);
 	}
 
-	return 0;
-}
-
-static int acpi_pci_unbind_cb(struct acpi_device *device)
-{
-	int rc = 0;
-	struct pci_dev *dev;
-
-	dev = acpi_get_pci_dev(device->handle);
-	if (dev) {
-		rc = acpi_pci_unbind(device, dev);
-		pci_dev_put(dev);
-	}
+	if (dev->subordinate)
+		acpi_pci_irq_del_prt(dev->subordinate);
 
-	return rc;
+	return 0;
 }
 
-static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
+static int acpi_pci_bind(acpi_handle handle, struct pci_dev *dev)
 {
 	acpi_status status;
-	acpi_handle handle;
 	struct pci_bus *bus;
+	struct acpi_device *acpi_dev;
 
-	pci_acpi_add_pm_notifier(device, dev);
-	if (device->wakeup.flags.run_wake)
-		device_set_run_wake(&dev->dev, true);
-
-	/*
-	 * Install the 'bind' function to facilitate callbacks for
-	 * children of the P2P bridge.
-	 */
-	if (dev->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)));
-		device->ops.bind = acpi_pci_bind_cb;
-		device->ops.unbind = acpi_pci_unbind_cb;
+	if (!acpi_bus_get_device(handle, &acpi_dev) && acpi_dev) {
+		pci_acpi_add_pm_notifier(acpi_dev, dev);
+		if (acpi_dev->wakeup.flags.run_wake)
+			device_set_run_wake(&dev->dev, true);
 	}
 
 	/*
@@ -96,56 +70,24 @@ static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
 	 *
 	 * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
 	 */
-	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
+	status = acpi_get_handle(handle, METHOD_NAME__PRT, &handle);
 	if (ACPI_SUCCESS(status)) {
 		if (dev->subordinate)
 			bus = dev->subordinate;
 		else
 			bus = dev->bus;
-		acpi_pci_irq_add_prt(device->handle, bus);
+		acpi_pci_irq_add_prt(handle, bus);
 	}
 
 	return 0;
 }
 
-static int acpi_pci_bind_cb(struct acpi_device *device)
-{
-	int rc = 0;
-	struct pci_dev *dev;
-
-	dev = acpi_get_pci_dev(device->handle);
-	if (dev) {
-		rc = acpi_pci_bind(device, dev);
-		pci_dev_put(dev);
-	}
-
-	return rc;
-}
-
-int acpi_pci_bind_root(struct acpi_device *device)
-{
-	device->ops.bind = acpi_pci_bind_cb;
-	device->ops.unbind = acpi_pci_unbind_cb;
-
-	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) || !acpi_dev)
-		return;
-
-	if (acpi_dev->parent) {
-		if (bind) {
-			if (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));
-		}
+	if (dev_is_pci(dev)) {
+		if (bind)
+			acpi_pci_bind(handle, to_pci_dev(dev));
+		else
+			acpi_pci_unbind(handle, to_pci_dev(dev));
 	}
 }
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 7509034..25d8ad4 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -505,15 +505,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	/* TBD: Locking */
 	list_add_tail(&root->node, &acpi_pci_roots);
 
-	/*
-	 * Attach ACPI-PCI Context
-	 * -----------------------
-	 * Thus binding the ACPI and PCI devices.
-	 */
-	result = acpi_pci_bind_root(device);
-	if (result)
-		goto end;
-
 	printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
 	       acpi_device_name(device), acpi_device_bid(device),
 	       root->segment, &root->secondary);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d1ecca2..f31cb2f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
 	dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
 	device_release_driver(&dev->dev);
 
-	if (!rmdevice)
-		return 0;
-
-	/*
-	 * unbind _ADR-Based Devices when hot removal
-	 */
-	if (dev->flags.bus_address) {
-		if ((dev->parent) && (dev->parent->ops.unbind))
-			dev->parent->ops.unbind(dev);
-	}
-	acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
+	if (rmdevice)
+		acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
 
 	return 0;
 }
@@ -1319,14 +1310,6 @@ static int acpi_add_single_object(struct acpi_device **child,
 
 	result = acpi_device_register(device);
 
-	/*
-	 * Bind _ADR-Based Devices when hot add
-	 */
-	if (device->flags.bus_address) {
-		if (device->parent && device->parent->ops.bind)
-			device->parent->ops.bind(device);
-	}
-
 end:
 	if (!result) {
 		acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index bde976e..ef5babf 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -120,8 +120,6 @@ struct acpi_device;
 typedef int (*acpi_op_add) (struct acpi_device * device);
 typedef int (*acpi_op_remove) (struct acpi_device * device, int type);
 typedef int (*acpi_op_start) (struct acpi_device * device);
-typedef int (*acpi_op_bind) (struct acpi_device * device);
-typedef int (*acpi_op_unbind) (struct acpi_device * device);
 typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
 
 struct acpi_bus_ops {
@@ -133,8 +131,6 @@ struct acpi_device_ops {
 	acpi_op_add add;
 	acpi_op_remove remove;
 	acpi_op_start start;
-	acpi_op_bind bind;
-	acpi_op_unbind unbind;
 	acpi_op_notify notify;
 };
 
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index bb145e4..fb1c0d5 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -100,7 +100,6 @@ void acpi_pci_irq_del_prt(struct pci_bus *bus);
 struct pci_bus;
 
 struct pci_dev *acpi_get_pci_dev(acpi_handle);
-int acpi_pci_bind_root(struct acpi_device *device);
 
 /* Arch-defined function to add a bus to the system */
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 6/9] ACPI/pci_slot: update PCI slot information when PCI hotplug event happens
       [not found] <1347678312-11124-1-git-send-email-jiang.liu@huawei.com>
  2012-09-15  3:05 ` [PATCH v2 4/9] ACPI/pci_bind: correctly update binding relationship for PCI hotplug Jiang Liu
  2012-09-15  3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
@ 2012-09-15  3:05 ` Jiang Liu
  2 siblings, 0 replies; 9+ messages in thread
From: Jiang Liu @ 2012-09-15  3:05 UTC (permalink / raw)
  To: Bjorn Helgaas, Len Brown
  Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
	Jiang Liu, linux-kernel, linux-pci, linux-acpi

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 <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/acpi/pci_slot.c |   86 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 78 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index e50e31a..96dcc19 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,15 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	if (device < 0)
 		return AE_OK;
 
+	/* Avoid duplicated records for the same slot */
+	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) {
+			return AE_OK;
+		}
+	}
+
 	slot = kmalloc(sizeof(*slot), GFP_KERNEL);
 	if (!slot) {
 		err("%s: cannot allocate memory\n", __func__);
@@ -162,9 +167,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	slot->root_handle = parent_context->root_handle;
 	slot->pci_slot = pci_slot;
 	INIT_LIST_HEAD(&slot->list);
-	mutex_lock(&slot_list_lock);
 	list_add(&slot->list, &slot_list);
-	mutex_unlock(&slot_list_lock);
 
 	get_device(&pci_bus->dev);
 
@@ -299,7 +302,9 @@ acpi_pci_slot_add(acpi_handle handle)
 {
 	acpi_status status;
 
+	mutex_lock(&slot_list_lock);
 	status = walk_root_bridge(handle, register_slot);
+	mutex_unlock(&slot_list_lock);
 	if (ACPI_FAILURE(status))
 		err("%s: register_slot failure - %d\n", __func__, status);
 
@@ -354,17 +359,82 @@ 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;
+
+	mutex_lock(&slot_list_lock);
+	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);
+	}
+	mutex_unlock(&slot_list_lock);
+}
+
+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.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
  2012-09-15  3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
@ 2012-09-15 18:53   ` Yinghai Lu
  2012-09-15 23:27   ` Yinghai Lu
  1 sibling, 0 replies; 9+ messages in thread
From: Yinghai Lu @ 2012-09-15 18:53 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
	Yijing Wang, linux-kernel, linux-pci, linux-acpi

On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>  From: Jiang Liu <jiang.liu@huawei.com>
>
> Now ACPI devices are created before/destroyed after corresponding PCI
> devices, and acpi_platform_notify/acpi_platform_notify_remove will
> update PCI<->ACPI binding relationship when creating/destroying PCI
> devices, there's no need to invoke bind/unbind callbacks from ACPI
> device probe/destroy routines anymore. So remove bind/unbind callbacks
> from acpi_device_ops.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  drivers/acpi/pci_bind.c     |  100 +++++++++----------------------------------
>  drivers/acpi/pci_root.c     |    9 ----
>  drivers/acpi/scan.c         |   21 +--------
>  include/acpi/acpi_bus.h     |    4 --
>  include/acpi/acpi_drivers.h |    1 -
>  5 files changed, 23 insertions(+), 112 deletions(-)
>
> diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
> index 320e698..d18316a 100644
> --- a/drivers/acpi/pci_bind.c
> +++ b/drivers/acpi/pci_bind.c
> @@ -35,57 +35,31 @@
>  #define _COMPONENT             ACPI_PCI_COMPONENT
>  ACPI_MODULE_NAME("pci_bind");
>
> -static int acpi_pci_bind_cb(struct acpi_device *device);
> -
> -static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev)
> +static int acpi_pci_unbind(acpi_handle handle, struct pci_dev *dev)
>  {
> -       device_set_run_wake(&dev->dev, false);
> -       pci_acpi_remove_pm_notifier(device);
> +       struct acpi_device *acpi_dev;
>
> -       if (dev->subordinate) {
> -               acpi_pci_irq_del_prt(dev->subordinate);
> -               device->ops.bind = NULL;
> -               device->ops.unbind = NULL;
> +       if (!acpi_bus_get_device(handle, &acpi_dev) && acpi_dev) {
> +               device_set_run_wake(&dev->dev, false);
> +               pci_acpi_remove_pm_notifier(acpi_dev);
>         }
>
> -       return 0;
> -}
> -
> -static int acpi_pci_unbind_cb(struct acpi_device *device)
> -{
> -       int rc = 0;
> -       struct pci_dev *dev;
> -
> -       dev = acpi_get_pci_dev(device->handle);
> -       if (dev) {
> -               rc = acpi_pci_unbind(device, dev);
> -               pci_dev_put(dev);
> -       }
> +       if (dev->subordinate)
> +               acpi_pci_irq_del_prt(dev->subordinate);
>
> -       return rc;
> +       return 0;
>  }
>
> -static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
> +static int acpi_pci_bind(acpi_handle handle, struct pci_dev *dev)
>  {
>         acpi_status status;
> -       acpi_handle handle;
>         struct pci_bus *bus;
> +       struct acpi_device *acpi_dev;
>
> -       pci_acpi_add_pm_notifier(device, dev);
> -       if (device->wakeup.flags.run_wake)
> -               device_set_run_wake(&dev->dev, true);
> -
> -       /*
> -        * Install the 'bind' function to facilitate callbacks for
> -        * children of the P2P bridge.
> -        */
> -       if (dev->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)));
> -               device->ops.bind = acpi_pci_bind_cb;
> -               device->ops.unbind = acpi_pci_unbind_cb;
> +       if (!acpi_bus_get_device(handle, &acpi_dev) && acpi_dev) {
> +               pci_acpi_add_pm_notifier(acpi_dev, dev);
> +               if (acpi_dev->wakeup.flags.run_wake)
> +                       device_set_run_wake(&dev->dev, true);

why not just keep acpi_device *device in function ? so you can avoid
extra acpi_bus_get_device() calling.

>         }
>
>         /*
> @@ -96,56 +70,24 @@ static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
>          *
>          * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
>          */
> -       status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
> +       status = acpi_get_handle(handle, METHOD_NAME__PRT, &handle);
>         if (ACPI_SUCCESS(status)) {
>                 if (dev->subordinate)
>                         bus = dev->subordinate;
>                 else
>                         bus = dev->bus;
> -               acpi_pci_irq_add_prt(device->handle, bus);
> +               acpi_pci_irq_add_prt(handle, bus);
>         }

if not _PRT, handle will become NULL? better to use &handle_tmp etc.



>
>         return 0;
>  }
>
> -static int acpi_pci_bind_cb(struct acpi_device *device)
> -{
> -       int rc = 0;
> -       struct pci_dev *dev;
> -
> -       dev = acpi_get_pci_dev(device->handle);
> -       if (dev) {
> -               rc = acpi_pci_bind(device, dev);
> -               pci_dev_put(dev);
> -       }
> -
> -       return rc;
> -}
> -
> -int acpi_pci_bind_root(struct acpi_device *device)
> -{
> -       device->ops.bind = acpi_pci_bind_cb;
> -       device->ops.unbind = acpi_pci_unbind_cb;
> -
> -       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) || !acpi_dev)
> -               return;
> -
> -       if (acpi_dev->parent) {
> -               if (bind) {
> -                       if (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));
> -               }
> +       if (dev_is_pci(dev)) {
> +               if (bind)
> +                       acpi_pci_bind(handle, to_pci_dev(dev));
> +               else
> +                       acpi_pci_unbind(handle, to_pci_dev(dev));
>         }
>  }
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 7509034..25d8ad4 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -505,15 +505,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>         /* TBD: Locking */
>         list_add_tail(&root->node, &acpi_pci_roots);
>
> -       /*
> -        * Attach ACPI-PCI Context
> -        * -----------------------
> -        * Thus binding the ACPI and PCI devices.
> -        */
> -       result = acpi_pci_bind_root(device);
> -       if (result)
> -               goto end;
> -
>         printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
>                acpi_device_name(device), acpi_device_bid(device),
>                root->segment, &root->secondary);
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..f31cb2f 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
>         dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
>         device_release_driver(&dev->dev);
>
> -       if (!rmdevice)
> -               return 0;
> -
> -       /*
> -        * unbind _ADR-Based Devices when hot removal
> -        */
> -       if (dev->flags.bus_address) {
> -               if ((dev->parent) && (dev->parent->ops.unbind))
> -                       dev->parent->ops.unbind(dev);
> -       }
> -       acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
> +       if (rmdevice)
> +               acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>
>         return 0;
>  }
> @@ -1319,14 +1310,6 @@ static int acpi_add_single_object(struct acpi_device **child,
>
>         result = acpi_device_register(device);
>
> -       /*
> -        * Bind _ADR-Based Devices when hot add
> -        */
> -       if (device->flags.bus_address) {
> -               if (device->parent && device->parent->ops.bind)
> -                       device->parent->ops.bind(device);
> -       }
> -
>  end:
>         if (!result) {
>                 acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index bde976e..ef5babf 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -120,8 +120,6 @@ struct acpi_device;
>  typedef int (*acpi_op_add) (struct acpi_device * device);
>  typedef int (*acpi_op_remove) (struct acpi_device * device, int type);
>  typedef int (*acpi_op_start) (struct acpi_device * device);
> -typedef int (*acpi_op_bind) (struct acpi_device * device);
> -typedef int (*acpi_op_unbind) (struct acpi_device * device);
>  typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
>
>  struct acpi_bus_ops {
> @@ -133,8 +131,6 @@ struct acpi_device_ops {
>         acpi_op_add add;
>         acpi_op_remove remove;
>         acpi_op_start start;
> -       acpi_op_bind bind;
> -       acpi_op_unbind unbind;
>         acpi_op_notify notify;
>  };
>
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index bb145e4..fb1c0d5 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -100,7 +100,6 @@ void acpi_pci_irq_del_prt(struct pci_bus *bus);
>  struct pci_bus;
>
>  struct pci_dev *acpi_get_pci_dev(acpi_handle);
> -int acpi_pci_bind_root(struct acpi_device *device);
>
>  /* Arch-defined function to add a bus to the system */
>
> --
> 1.7.9.5
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
  2012-09-15  3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
  2012-09-15 18:53   ` Yinghai Lu
@ 2012-09-15 23:27   ` Yinghai Lu
  2012-09-17  3:03     ` Yinghai Lu
  2012-09-17 14:31     ` Jiang Liu
  1 sibling, 2 replies; 9+ messages in thread
From: Yinghai Lu @ 2012-09-15 23:27 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
	Yijing Wang, linux-kernel, linux-pci, linux-acpi

On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>  From: Jiang Liu <jiang.liu@huawei.com>
>
> Now ACPI devices are created before/destroyed after corresponding PCI
> devices, and acpi_platform_notify/acpi_platform_notify_remove will
> update PCI<->ACPI binding relationship when creating/destroying PCI
> devices, there's no need to invoke bind/unbind callbacks from ACPI
> device probe/destroy routines anymore. So remove bind/unbind callbacks
> from acpi_device_ops.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  drivers/acpi/pci_bind.c     |  100 +++++++++----------------------------------
>  drivers/acpi/pci_root.c     |    9 ----
>  drivers/acpi/scan.c         |   21 +--------
>  include/acpi/acpi_bus.h     |    4 --
>  include/acpi/acpi_drivers.h |    1 -
>  5 files changed, 23 insertions(+), 112 deletions(-)
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..f31cb2f 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
>         dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
>         device_release_driver(&dev->dev);
>
> -       if (!rmdevice)
> -               return 0;
> -
> -       /*
> -        * unbind _ADR-Based Devices when hot removal
> -        */
> -       if (dev->flags.bus_address) {
> -               if ((dev->parent) && (dev->parent->ops.unbind))
> -                       dev->parent->ops.unbind(dev);
> -       }
> -       acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
> +       if (rmdevice)
> +               acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>
>         return 0;
>  }

for pci root bus, acpi_bus_trim() is used to remove acpi_device.

and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
iommu driver then remove pci devices.

if call back is removed there, then could some functions in
acpi_pci_unbind() will be skipped.

I really do not want to add pci_stop_bus_devices() in
pci_root_hp.c::handle_root_bridge_removal before
calling acpi_bus_trim...

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
  2012-09-15 23:27   ` Yinghai Lu
@ 2012-09-17  3:03     ` Yinghai Lu
  2012-09-17 14:22       ` Jiang Liu
  2012-09-17 14:31     ` Jiang Liu
  1 sibling, 1 reply; 9+ messages in thread
From: Yinghai Lu @ 2012-09-17  3:03 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
	Yijing Wang, linux-kernel, linux-pci, linux-acpi

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

On Sat, Sep 15, 2012 at 4:27 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>>  From: Jiang Liu <jiang.liu@huawei.com>
>>
>> Now ACPI devices are created before/destroyed after corresponding PCI
>> devices, and acpi_platform_notify/acpi_platform_notify_remove will
>> update PCI<->ACPI binding relationship when creating/destroying PCI
>> devices, there's no need to invoke bind/unbind callbacks from ACPI
>> device probe/destroy routines anymore. So remove bind/unbind callbacks
>> from acpi_device_ops.
> for pci root bus, acpi_bus_trim() is used to remove acpi_device.
>
> and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
> iommu driver then remove pci devices.
>
> if call back is removed there, then could some functions in
> acpi_pci_unbind() will be skipped.
>
> I really do not want to add pci_stop_bus_devices() in
> pci_root_hp.c::handle_root_bridge_removal before
> calling acpi_bus_trim...

FYI, I solved the problem.  will call apci_bus_remove() two times. it
will make sure pci devices
get removed at first before acpi devices...

Thanks

Yinghai

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

Subject: [PATCH] PCI, ACPI: Add pci_root_hp hot removal notification support.

Add missing hot_remove support for root device.

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

  echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify
to remove root bus

-v2: separate stop and remove, so could use acpi_pci_bind_notify()...

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org

---
 drivers/acpi/pci_root_hp.c |   67 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c        |    5 ---
 2 files changed, 68 insertions(+), 4 deletions(-)

Index: linux-2.6/drivers/acpi/pci_root_hp.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root_hp.c
+++ linux-2.6/drivers/acpi/pci_root_hp.c
@@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_ha
 	list_add(&bridge->list, &acpi_root_bridge_list);
 }
 
+static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
+{
+	list_del(&bridge->list);
+	kfree(bridge);
+}
+
 struct acpi_root_hp_work {
 	struct work_struct work;
 	acpi_handle handle;
@@ -143,6 +149,61 @@ static void handle_root_bridge_insertion
 		printk(KERN_ERR "cannot start bridge\n");
 }
 
+static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
+{
+	acpi_status status;
+	struct acpi_object_list arg_list;
+	union acpi_object arg;
+
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = val;
+
+	status = acpi_evaluate_object(handle, cmd, &arg_list, NULL);
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_WARNING "%s: %s to %d failed\n",
+				 __func__, cmd, val);
+		return -1;
+	}
+
+	return 0;
+}
+
+static void handle_root_bridge_removal(acpi_handle handle,
+		 struct acpi_root_bridge *bridge)
+{
+	u32 flags = 0;
+	struct acpi_device *device;
+
+	if (bridge) {
+		flags = bridge->flags;
+		remove_acpi_root_bridge(bridge);
+	}
+
+	if (!acpi_bus_get_device(handle, &device)) {
+		int ret_val;
+
+		/* remove pci devices at first */
+		ret_val = acpi_bus_trim(device, 0);
+		printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val);
+
+		/* remove acpi devices */
+		ret_val = acpi_bus_trim(device, 1);
+		printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val);
+	}
+
+	if (flags & ROOT_BRIDGE_HAS_PS3) {
+		acpi_status status;
+
+		status = acpi_evaluate_object(handle, "_PS3", NULL, NULL);
+		if (ACPI_FAILURE(status))
+			printk(KERN_WARNING "%s: _PS3 failed\n", __func__);
+	}
+	if (flags & ROOT_BRIDGE_HAS_EJ0)
+		acpi_root_evaluate_object(handle, "_EJ0", 1);
+}
+
 static void _handle_hotplug_event_root(struct work_struct *work)
 {
 	struct acpi_root_bridge *bridge;
@@ -183,6 +244,12 @@ static void _handle_hotplug_event_root(s
 		}
 		break;
 
+	case ACPI_NOTIFY_EJECT_REQUEST:
+		/* request device eject */
+		printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
+				 objname);
+		handle_root_bridge_removal(handle, bridge);
+		break;
 	default:
 		printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
 				 type, objname);
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -1522,10 +1522,7 @@ int acpi_bus_trim(struct acpi_device *st
 			child = parent;
 			parent = parent->parent;
 
-			if (level == 0)
-				err = acpi_bus_remove(child, rmdevice);
-			else
-				err = acpi_bus_remove(child, 1);
+			err = acpi_bus_remove(child, rmdevice);
 
 			continue;
 		}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
  2012-09-17  3:03     ` Yinghai Lu
@ 2012-09-17 14:22       ` Jiang Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Jiang Liu @ 2012-09-17 14:22 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
	Yijing Wang, linux-kernel, linux-pci, linux-acpi

On 09/17/2012 11:03 AM, Yinghai Lu wrote:
> On Sat, Sep 15, 2012 at 4:27 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>>>  From: Jiang Liu <jiang.liu@huawei.com>
>>>
>>> Now ACPI devices are created before/destroyed after corresponding PCI
>>> devices, and acpi_platform_notify/acpi_platform_notify_remove will
>>> update PCI<->ACPI binding relationship when creating/destroying PCI
>>> devices, there's no need to invoke bind/unbind callbacks from ACPI
>>> device probe/destroy routines anymore. So remove bind/unbind callbacks
>>> from acpi_device_ops.
>> for pci root bus, acpi_bus_trim() is used to remove acpi_device.
>>
>> and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
>> iommu driver then remove pci devices.
>>
>> if call back is removed there, then could some functions in
>> acpi_pci_unbind() will be skipped.
>>
>> I really do not want to add pci_stop_bus_devices() in
>> pci_root_hp.c::handle_root_bridge_removal before
>> calling acpi_bus_trim...
> 
> FYI, I solved the problem.  will call apci_bus_remove() two times. it
> will make sure pci devices
> get removed at first before acpi devices...
Hi Yinghai,
	Great! So we could remove the acpi<->PCI logic now.
	Thanks!
	Gerry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
  2012-09-15 23:27   ` Yinghai Lu
  2012-09-17  3:03     ` Yinghai Lu
@ 2012-09-17 14:31     ` Jiang Liu
  2012-09-17 15:41       ` Yinghai Lu
  1 sibling, 1 reply; 9+ messages in thread
From: Jiang Liu @ 2012-09-17 14:31 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
	Yijing Wang, linux-kernel, linux-pci, linux-acpi

On 09/16/2012 07:27 AM, Yinghai Lu wrote:
> On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>>  From: Jiang Liu <jiang.liu@huawei.com>
>>
>> Now ACPI devices are created before/destroyed after corresponding PCI
>> devices, and acpi_platform_notify/acpi_platform_notify_remove will
>> update PCI<->ACPI binding relationship when creating/destroying PCI
>> devices, there's no need to invoke bind/unbind callbacks from ACPI
>> device probe/destroy routines anymore. So remove bind/unbind callbacks
>> from acpi_device_ops.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> ---
>>  drivers/acpi/pci_bind.c     |  100 +++++++++----------------------------------
>>  drivers/acpi/pci_root.c     |    9 ----
>>  drivers/acpi/scan.c         |   21 +--------
>>  include/acpi/acpi_bus.h     |    4 --
>>  include/acpi/acpi_drivers.h |    1 -
>>  5 files changed, 23 insertions(+), 112 deletions(-)
>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index d1ecca2..f31cb2f 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
>>         dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
>>         device_release_driver(&dev->dev);
>>
>> -       if (!rmdevice)
>> -               return 0;
>> -
>> -       /*
>> -        * unbind _ADR-Based Devices when hot removal
>> -        */
>> -       if (dev->flags.bus_address) {
>> -               if ((dev->parent) && (dev->parent->ops.unbind))
>> -                       dev->parent->ops.unbind(dev);
>> -       }
>> -       acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>> +       if (rmdevice)
>> +               acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>>
>>         return 0;
>>  }
> 
> for pci root bus, acpi_bus_trim() is used to remove acpi_device.
> 
> and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
> iommu driver then remove pci devices.
> 
> if call back is removed there, then could some functions in
> acpi_pci_unbind() will be skipped.
> 
> I really do not want to add pci_stop_bus_devices() in
> pci_root_hp.c::handle_root_bridge_removal before
> calling acpi_bus_trim...
Hi Yinghai,
	We are working on a ACPI based hotplug framework for CPU, memory and IOH.
With our framework, the sequence to remove a IOH is:
1) unbind all PCI device drivers for affected PCI devices.
2) stop and remove all affected PCI devices.
3) stop ACPI ioapic drivers(a new driver like PCI version).
4) destroy all ACPI devices.

--Gerry
> 
> Thanks
> 
> Yinghai
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
  2012-09-17 14:31     ` Jiang Liu
@ 2012-09-17 15:41       ` Yinghai Lu
  0 siblings, 0 replies; 9+ messages in thread
From: Yinghai Lu @ 2012-09-17 15:41 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
	Yijing Wang, linux-kernel, linux-pci, linux-acpi

On Mon, Sep 17, 2012 at 7:31 AM, Jiang Liu <liuj97@gmail.com> wrote:
> On 09/16/2012 07:27 AM, Yinghai Lu wrote:
>>> --- a/drivers/acpi/scan.c
>>> +++ b/drivers/acpi/scan.c
>>> @@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
>>>         dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
>>>         device_release_driver(&dev->dev);
>>>
>>> -       if (!rmdevice)
>>> -               return 0;
>>> -
>>> -       /*
>>> -        * unbind _ADR-Based Devices when hot removal
>>> -        */
>>> -       if (dev->flags.bus_address) {
>>> -               if ((dev->parent) && (dev->parent->ops.unbind))
>>> -                       dev->parent->ops.unbind(dev);
>>> -       }
>>> -       acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>>> +       if (rmdevice)
>>> +               acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>>>
>>>         return 0;
>>>  }
>>
>> for pci root bus, acpi_bus_trim() is used to remove acpi_device.
>>
>> and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
>> iommu driver then remove pci devices.
>>
>> if call back is removed there, then could some functions in
>> acpi_pci_unbind() will be skipped.
>>
>> I really do not want to add pci_stop_bus_devices() in
>> pci_root_hp.c::handle_root_bridge_removal before
>> calling acpi_bus_trim...
> Hi Yinghai,
>         We are working on a ACPI based hotplug framework for CPU, memory and IOH.
> With our framework, the sequence to remove a IOH is:
> 1) unbind all PCI device drivers for affected PCI devices.
> 2) stop and remove all affected PCI devices.
> 3) stop ACPI ioapic drivers(a new driver like PCI version).
> 4) destroy all ACPI devices.

i solved the problem...

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

+       if (!acpi_bus_get_device(handle, &device)) {
+               int ret_val;
+
+               /* remove pci devices at first */
+               ret_val = acpi_bus_trim(device, 0);
+               printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val);
+
+               /* remove acpi devices */
+               ret_val = acpi_bus_trim(device, 1);
+               printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val);
+       }

...

also need to update acpi_bus_trim

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

BTW, memory hot-remove may need some work.

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-09-17 15:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1347678312-11124-1-git-send-email-jiang.liu@huawei.com>
2012-09-15  3:05 ` [PATCH v2 4/9] ACPI/pci_bind: correctly update binding relationship for PCI hotplug Jiang Liu
2012-09-15  3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
2012-09-15 18:53   ` Yinghai Lu
2012-09-15 23:27   ` Yinghai Lu
2012-09-17  3:03     ` Yinghai Lu
2012-09-17 14:22       ` Jiang Liu
2012-09-17 14:31     ` Jiang Liu
2012-09-17 15:41       ` Yinghai Lu
2012-09-15  3:05 ` [PATCH v2 6/9] ACPI/pci_slot: update PCI slot information when PCI hotplug event happens Jiang Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).