[RFC PATCH 0/5] ACPI: Refactor system notify handling

[RFC PATCH 0/5] ACPI: Refactor system notify handling

[Toshi Kani]

This patchset updates the ACPI system-level (ex. hotplug)
notify handling with a new .sys_notify interface.  It provides
the following changes:

- Allow ACPI drivers to register their system (hotplug) notify
  handlers to a new .sys_notify interface in their acpi_driver
  table.  This removes redundant ACPI namespace walks from ACPI
  drivers for faster booting.  The global notify handler
  acpi_bus_notify() is called for all system-level ACPI device
  notification, which then calls an appropriate driver's handler
  if any.  ACPI drivers no longer need to register or unregister
  driver's handlers to each device object in ACPI namespace.
 
- Support dynamic ACPI namespace with LoadTable & Unload opcode
  without any changes in ACPI drivers.  There is no need to 
  register / unregister handlers to ACPI device objects that may
  be loaded to / unloaded from ACPI namespace.

- Allow ACPI drivers to use a common hotplug handler when it is
  implemented.  It removes functional conflict between driver's
  notify handler and the global notify handler acpi_bus_notify().
  acpi_bus_notify() only calls an appropriate notify handler if
  any.

Note that the changes maintain the backward compatibility of ACPI
drivers.  Any ACPI driver registered its hotplug handler through
the existing interfaces, such as acpi_install_notify_handler() and
register_acpi_bus_notifier(), will continue to work as before.

---
Toshi Kani (5):
 ACPI: Add acpi_lookup_driver() function
 ACPI: Support system notify handler via .sys_notify
 ACPI: Update processor_driver to use .sys_notify
 ACPI: Update acpi_memhotplug to use .sys_notify
 ACPI: Update container to use .sys_notify

---
 drivers/acpi/acpi_memhotplug.c  |   93 +-------------------------------------
 drivers/acpi/bus.c              |   48 +++++++++++++++-----
 drivers/acpi/container.c        |   66 +--------------------------
 drivers/acpi/processor_driver.c |   82 +++-------------------------------
 drivers/acpi/scan.c             |   65 +++++++++++++++++++++++++++
 include/acpi/acpi_bus.h         |    4 ++
 6 files changed, 117 insertions(+), 241 deletions(-)

[RFC PATCH 1/5] ACPI: Add acpi_lookup_driver() function

[Toshi Kani]

Added acpi_lookup_driver(), which looks up an associated driver
for the notified ACPI device object by walking through the list
of ACPI drivers.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/scan.c     |   65 +++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |    2 +
 2 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d1ecca2..d0e0d18 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1630,3 +1630,68 @@ int __init acpi_scan_init(void)
 
 	return result;
 }
+
+static int acpi_match_driver(struct device_driver *drv, void *data)
+{
+	struct acpi_driver *driver = to_acpi_driver(drv);
+	struct acpi_device *device = (struct acpi_device *) data;
+	int ret;
+
+	ret = acpi_match_device_ids(device, driver->ids);
+	if (!ret)
+		device->driver = driver;
+
+	return !ret;
+}
+
+/**
+ * acpi_lookup_driver: Look up a driver for the notified ACPI device
+ * @handle: ACPI handle of the notified device object
+ * @event: Notify event 
+ *
+ * Look up an associated driver for the notified ACPI device object
+ * by walking through the list of ACPI drivers.
+ */
+struct acpi_driver *acpi_lookup_driver(acpi_handle handle, u32 event)
+{
+	struct acpi_device *device;
+	struct acpi_driver *driver = NULL;
+	unsigned long long sta;
+	int type;
+	int ret;
+
+	/* allocate a temporary device object */
+	device = kzalloc(sizeof(struct acpi_device), GFP_KERNEL);
+	if (!device) {
+		pr_err(PREFIX "No memory to allocate a tmp device\n");
+		return NULL;
+	}
+
+	ret = acpi_bus_type_and_status(handle, &type, &sta);
+	if (ret) {
+		pr_err(PREFIX "Failed to get type of device\n");
+		goto out;
+	}
+
+	/* setup this temporary device object */
+	INIT_LIST_HEAD(&device->pnp.ids);
+	device->device_type = type;
+	device->handle = handle;
+	device->parent = acpi_bus_get_parent(handle);
+	device->dev.bus = &acpi_bus_type;
+	device->driver = NULL;
+	STRUCT_TO_INT(device->status) = sta;
+	device->status.present = 1;
+
+	/* set HID to this device object */
+	acpi_device_set_id(device);
+
+	/* lookup a matching driver */
+	(void) bus_for_each_drv(device->dev.bus, NULL,
+					device, acpi_match_driver);
+	driver = device->driver;
+
+out:
+	kfree(device);
+	return driver;
+}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index bde976e..a773b46 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -345,6 +345,8 @@ extern int unregister_acpi_notifier(struct notifier_block *);
 
 extern int register_acpi_bus_notifier(struct notifier_block *nb);
 extern void unregister_acpi_bus_notifier(struct notifier_block *nb);
+extern struct acpi_driver *acpi_lookup_driver(acpi_handle handle, u32 event);
+
 /*
  * External Functions
  */
-- 
1.7.7.6

[RFC PATCH 2/5] ACPI: Support system notify handler via .sys_notify

[Toshi Kani]

Added a new .sys_notify interface, which allows ACPI drivers to
register their system-level (ex. hotplug) notify handlers in their
acpi_driver table.  The global notify handler acpi_bus_notify()
is called for all system-level ACPI notification, which is changed
to call an appropriate driver's handler if any.  With .sys_notify,
ACPI drivers no longer need to register or unregister driver's
handler to each ACPI device object.  It also supports dynamic ACPI
namespace with LoadTable & Unload opcode without any modification.

Added a common system notify handler acpi_bus_sys_notify(), which
allows ACPI drivers to set it to .sys_notify when this function is
fully implemented.

Note that the changes maintain the backward compatibility of ACPI
drivers.  Any ACPI driver registered its hotplug handler through
the existing interfaces, such as acpi_install_notify_handler() and
register_acpi_bus_notifier(), will continue to work as before.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/bus.c      |   48 ++++++++++++++++++++++++++++++++++------------
 include/acpi/acpi_bus.h |    2 +
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 9628652..4b941c9 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -760,21 +760,16 @@ void unregister_acpi_bus_notifier(struct notifier_block *nb)
 EXPORT_SYMBOL_GPL(unregister_acpi_bus_notifier);
 
 /**
- * acpi_bus_notify
- * ---------------
- * Callback for all 'system-level' device notifications (values 0x00-0x7F).
+ * acpi_bus_sys_notify: Common system notify handler
+ *
+ * ACPI drivers may specify this common handler to its sys_notify entry.
+ * TBD: This handler is not implemented yet.
  */
-static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
+void acpi_bus_sys_notify(acpi_handle handle, u32 type, void *data)
 {
-	struct acpi_device *device = NULL;
-	struct acpi_driver *driver;
-
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Notification %#02x to handle %p\n",
 			  type, handle));
 
-	blocking_notifier_call_chain(&acpi_bus_notify_list,
-		type, (void *)handle);
-
 	switch (type) {
 
 	case ACPI_NOTIFY_BUS_CHECK:
@@ -823,14 +818,41 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
 				  type));
 		break;
 	}
+}
 
+/**
+ * acpi_bus_notify: The system global notify handler
+ * 
+ * The global notify handler for all 'system-level' device notifications
+ * (values 0x00-0x7F).  This handler calls a driver's notify handler for
+ * the notified ACPI device.
+ */
+static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
+{
+	struct acpi_device *device = NULL;
+	struct acpi_driver *driver;
+
+	/* obtain associated driver object */
 	acpi_bus_get_device(handle, &device);
-	if (device) {
+	if (device && device->driver)
 		driver = device->driver;
-		if (driver && driver->ops.notify &&
-		    (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS))
+	else
+		driver = acpi_lookup_driver(handle, type);
+
+	/* call a driver's handler */
+	if (driver) {
+		if (driver->ops.sys_notify)
+			driver->ops.sys_notify(handle, type, data);
+		else if (device && driver->ops.notify &&
+			 (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS))
 			driver->ops.notify(device, type);
 	}
+
+	/* call handlers in the bus notify list */
+	blocking_notifier_call_chain(&acpi_bus_notify_list,
+		type, (void *)handle);
+
+	return;
 }
 
 /* --------------------------------------------------------------------------
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index a773b46..575fd2f 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -123,6 +123,7 @@ 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);
+typedef void (*acpi_op_sys_notify) (acpi_handle handle, u32 event, void *data);
 
 struct acpi_bus_ops {
 	u32 acpi_op_add:1;
@@ -136,6 +137,7 @@ struct acpi_device_ops {
 	acpi_op_bind bind;
 	acpi_op_unbind unbind;
 	acpi_op_notify notify;
+	acpi_op_sys_notify sys_notify;
 };
 
 #define ACPI_DRIVER_ALL_NOTIFY_EVENTS	0x1	/* system AND device events */
-- 
1.7.7.6

[RFC PATCH 3/5] ACPI: Update processor_driver to use .sys_notify

[Toshi Kani]

Changed the ACPI processor driver to use .sys_notify.  Removed
ACPI namespace walks and their call-back function that register
and unregister the hotplug handler to all processor objects
through acpi_[install|remove]_notify_handler().

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/processor_driver.c |   82 +++-----------------------------------
 1 files changed, 7 insertions(+), 75 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index bfc31cb..a0f6123 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -85,6 +85,10 @@ static void acpi_processor_notify(struct acpi_device *device, u32 event);
 static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr);
 static int acpi_processor_handle_eject(struct acpi_processor *pr);
 static int acpi_processor_start(struct acpi_processor *pr);
+#ifdef CONFIG_ACPI_HOTPLUG_CPU
+static void acpi_processor_hotplug_notify(acpi_handle handle, u32 event,
+		void *data);
+#endif
 
 static const struct acpi_device_id processor_device_ids[] = {
 	{ACPI_PROCESSOR_OBJECT_HID, 0},
@@ -104,6 +108,9 @@ static struct acpi_driver acpi_processor_driver = {
 		.add = acpi_processor_add,
 		.remove = acpi_processor_remove,
 		.notify = acpi_processor_notify,
+#ifdef CONFIG_ACPI_HOTPLUG_CPU
+		.sys_notify = acpi_processor_hotplug_notify,
+#endif
 		},
 	.drv.pm = &acpi_processor_pm,
 };
@@ -755,67 +762,6 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
 	return;
 }
 
-static acpi_status is_processor_device(acpi_handle handle)
-{
-	struct acpi_device_info *info;
-	char *hid;
-	acpi_status status;
-
-	status = acpi_get_object_info(handle, &info);
-	if (ACPI_FAILURE(status))
-		return status;
-
-	if (info->type == ACPI_TYPE_PROCESSOR) {
-		kfree(info);
-		return AE_OK;	/* found a processor object */
-	}
-
-	if (!(info->valid & ACPI_VALID_HID)) {
-		kfree(info);
-		return AE_ERROR;
-	}
-
-	hid = info->hardware_id.string;
-	if ((hid == NULL) || strcmp(hid, ACPI_PROCESSOR_DEVICE_HID)) {
-		kfree(info);
-		return AE_ERROR;
-	}
-
-	kfree(info);
-	return AE_OK;	/* found a processor device object */
-}
-
-static acpi_status
-processor_walk_namespace_cb(acpi_handle handle,
-			    u32 lvl, void *context, void **rv)
-{
-	acpi_status status;
-	int *action = context;
-
-	status = is_processor_device(handle);
-	if (ACPI_FAILURE(status))
-		return AE_OK;	/* not a processor; continue to walk */
-
-	switch (*action) {
-	case INSTALL_NOTIFY_HANDLER:
-		acpi_install_notify_handler(handle,
-					    ACPI_SYSTEM_NOTIFY,
-					    acpi_processor_hotplug_notify,
-					    NULL);
-		break;
-	case UNINSTALL_NOTIFY_HANDLER:
-		acpi_remove_notify_handler(handle,
-					   ACPI_SYSTEM_NOTIFY,
-					   acpi_processor_hotplug_notify);
-		break;
-	default:
-		break;
-	}
-
-	/* found a processor; skip walking underneath */
-	return AE_CTRL_DEPTH;
-}
-
 static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
 {
 	acpi_handle handle = pr->handle;
@@ -869,26 +815,12 @@ static int acpi_processor_handle_eject(struct acpi_processor *pr)
 static
 void acpi_processor_install_hotplug_notify(void)
 {
-#ifdef CONFIG_ACPI_HOTPLUG_CPU
-	int action = INSTALL_NOTIFY_HANDLER;
-	acpi_walk_namespace(ACPI_TYPE_ANY,
-			    ACPI_ROOT_OBJECT,
-			    ACPI_UINT32_MAX,
-			    processor_walk_namespace_cb, NULL, &action, NULL);
-#endif
 	register_hotcpu_notifier(&acpi_cpu_notifier);
 }
 
 static
 void acpi_processor_uninstall_hotplug_notify(void)
 {
-#ifdef CONFIG_ACPI_HOTPLUG_CPU
-	int action = UNINSTALL_NOTIFY_HANDLER;
-	acpi_walk_namespace(ACPI_TYPE_ANY,
-			    ACPI_ROOT_OBJECT,
-			    ACPI_UINT32_MAX,
-			    processor_walk_namespace_cb, NULL, &action, NULL);
-#endif
 	unregister_hotcpu_notifier(&acpi_cpu_notifier);
 }
 
-- 
1.7.7.6

[RFC PATCH 4/5] ACPI: Update acpi_memhotplug to use .sys_notify

[Toshi Kani]

Changed the ACPI memory hotplug driver to use .sys_notify.  Removed
ACPI namespace walks and their call-back functions that register and
unregister the hotplug handler to all memory device objects through
acpi_[install|remove]_notify_handler().

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/acpi_memhotplug.c |   93 +--------------------------------------
 1 files changed, 3 insertions(+), 90 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 24c807f..1f62686 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -54,6 +54,8 @@ MODULE_LICENSE("GPL");
 
 static int acpi_memory_device_add(struct acpi_device *device);
 static int acpi_memory_device_remove(struct acpi_device *device, int type);
+static void acpi_memory_device_notify(acpi_handle handle, u32 event,
+	void *data);
 
 static const struct acpi_device_id memory_device_ids[] = {
 	{ACPI_MEMORY_DEVICE_HID, 0},
@@ -68,6 +70,7 @@ static struct acpi_driver acpi_memory_device_driver = {
 	.ops = {
 		.add = acpi_memory_device_add,
 		.remove = acpi_memory_device_remove,
+		.sys_notify = acpi_memory_device_notify,
 		},
 };
 
@@ -484,111 +487,21 @@ static int acpi_memory_device_remove(struct acpi_device *device, int type)
 	return 0;
 }
 
-/*
- * Helper function to check for memory device
- */
-static acpi_status is_memory_device(acpi_handle handle)
-{
-	char *hardware_id;
-	acpi_status status;
-	struct acpi_device_info *info;
-
-	status = acpi_get_object_info(handle, &info);
-	if (ACPI_FAILURE(status))
-		return status;
-
-	if (!(info->valid & ACPI_VALID_HID)) {
-		kfree(info);
-		return AE_ERROR;
-	}
-
-	hardware_id = info->hardware_id.string;
-	if ((hardware_id == NULL) ||
-	    (strcmp(hardware_id, ACPI_MEMORY_DEVICE_HID)))
-		status = AE_ERROR;
-
-	kfree(info);
-	return status;
-}
-
-static acpi_status
-acpi_memory_register_notify_handler(acpi_handle handle,
-				    u32 level, void *ctxt, void **retv)
-{
-	acpi_status status;
-
-
-	status = is_memory_device(handle);
-	if (ACPI_FAILURE(status))
-		return AE_OK;	/* continue */
-
-	status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-					     acpi_memory_device_notify, NULL);
-	/* continue */
-	return AE_OK;
-}
-
-static acpi_status
-acpi_memory_deregister_notify_handler(acpi_handle handle,
-				      u32 level, void *ctxt, void **retv)
-{
-	acpi_status status;
-
-
-	status = is_memory_device(handle);
-	if (ACPI_FAILURE(status))
-		return AE_OK;	/* continue */
-
-	status = acpi_remove_notify_handler(handle,
-					    ACPI_SYSTEM_NOTIFY,
-					    acpi_memory_device_notify);
-
-	return AE_OK;	/* continue */
-}
-
 static int __init acpi_memory_device_init(void)
 {
 	int result;
-	acpi_status status;
-
 
 	result = acpi_bus_register_driver(&acpi_memory_device_driver);
 
 	if (result < 0)
 		return -ENODEV;
 
-	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-				     ACPI_UINT32_MAX,
-				     acpi_memory_register_notify_handler, NULL,
-				     NULL, NULL);
-
-	if (ACPI_FAILURE(status)) {
-		ACPI_EXCEPTION((AE_INFO, status, "walk_namespace failed"));
-		acpi_bus_unregister_driver(&acpi_memory_device_driver);
-		return -ENODEV;
-	}
-
 	acpi_hotmem_initialized = 1;
 	return 0;
 }
 
 static void __exit acpi_memory_device_exit(void)
 {
-	acpi_status status;
-
-
-	/*
-	 * Adding this to un-install notification handlers for all the device
-	 * handles.
-	 */
-	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-				     ACPI_UINT32_MAX,
-				     acpi_memory_deregister_notify_handler, NULL,
-				     NULL, NULL);
-
-	if (ACPI_FAILURE(status))
-		ACPI_EXCEPTION((AE_INFO, status, "walk_namespace failed"));
-
 	acpi_bus_unregister_driver(&acpi_memory_device_driver);
 
 	return;
-- 
1.7.7.6

[RFC PATCH 5/5] ACPI: Update container to use .sys_notify

[Toshi Kani]

Changed the ACPI container driver to use .sys_notify.  Removed
ACPI namespace walks and their call-back function that register
and unregister the hotplug handler to all container objects
through acpi_[install|remove]_notify_handler().

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/container.c |   66 ++-------------------------------------------
 1 files changed, 3 insertions(+), 63 deletions(-)

diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 1f9f7d7..d4e8b71 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -53,6 +53,7 @@ MODULE_LICENSE("GPL");
 
 static int acpi_container_add(struct acpi_device *device);
 static int acpi_container_remove(struct acpi_device *device, int type);
+static void container_notify(acpi_handle handle, u32 type, void *context);
 
 static const struct acpi_device_id container_device_ids[] = {
 	{"ACPI0004", 0},
@@ -69,6 +70,7 @@ static struct acpi_driver acpi_container_driver = {
 	.ops = {
 		.add = acpi_container_add,
 		.remove = acpi_container_remove,
+		.sys_notify = container_notify,
 		},
 };
 
@@ -152,7 +154,7 @@ static int container_device_add(struct acpi_device **device, acpi_handle handle)
 	return result;
 }
 
-static void container_notify_cb(acpi_handle handle, u32 type, void *context)
+static void container_notify(acpi_handle handle, u32 type, void *context)
 {
 	struct acpi_device *device = NULL;
 	int result;
@@ -211,82 +213,20 @@ static void container_notify_cb(acpi_handle handle, u32 type, void *context)
 	return;
 }
 
-static acpi_status
-container_walk_namespace_cb(acpi_handle handle,
-			    u32 lvl, void *context, void **rv)
-{
-	char *hid = NULL;
-	struct acpi_device_info *info;
-	acpi_status status;
-	int *action = context;
-
-	status = acpi_get_object_info(handle, &info);
-	if (ACPI_FAILURE(status)) {
-		return AE_OK;
-	}
-
-	if (info->valid & ACPI_VALID_HID)
-		hid = info->hardware_id.string;
-
-	if (hid == NULL) {
-		goto end;
-	}
-
-	if (strcmp(hid, "ACPI0004") && strcmp(hid, "PNP0A05") &&
-	    strcmp(hid, "PNP0A06")) {
-		goto end;
-	}
-
-	switch (*action) {
-	case INSTALL_NOTIFY_HANDLER:
-		acpi_install_notify_handler(handle,
-					    ACPI_SYSTEM_NOTIFY,
-					    container_notify_cb, NULL);
-		break;
-	case UNINSTALL_NOTIFY_HANDLER:
-		acpi_remove_notify_handler(handle,
-					   ACPI_SYSTEM_NOTIFY,
-					   container_notify_cb);
-		break;
-	default:
-		break;
-	}
-
-      end:
-	kfree(info);
-
-	return AE_OK;
-}
-
 static int __init acpi_container_init(void)
 {
 	int result = 0;
-	int action = INSTALL_NOTIFY_HANDLER;
 
 	result = acpi_bus_register_driver(&acpi_container_driver);
 	if (result < 0) {
 		return (result);
 	}
 
-	/* register notify handler to every container device */
-	acpi_walk_namespace(ACPI_TYPE_DEVICE,
-			    ACPI_ROOT_OBJECT,
-			    ACPI_UINT32_MAX,
-			    container_walk_namespace_cb, NULL, &action, NULL);
-
 	return (0);
 }
 
 static void __exit acpi_container_exit(void)
 {
-	int action = UNINSTALL_NOTIFY_HANDLER;
-
-
-	acpi_walk_namespace(ACPI_TYPE_DEVICE,
-			    ACPI_ROOT_OBJECT,
-			    ACPI_UINT32_MAX,
-			    container_walk_namespace_cb, NULL, &action, NULL);
-
 	acpi_bus_unregister_driver(&acpi_container_driver);
 
 	return;
-- 
1.7.7.6

Re: [RFC PATCH 1/5] ACPI: Add acpi_lookup_driver() function

[Bjorn Helgaas]

On Thu, Aug 30, 2012 at 1:16 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> Added acpi_lookup_driver(), which looks up an associated driver
> for the notified ACPI device object by walking through the list
> of ACPI drivers.
>
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> ---
>  drivers/acpi/scan.c     |   65 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h |    2 +
>  2 files changed, 67 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..d0e0d18 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1630,3 +1630,68 @@ int __init acpi_scan_init(void)
>
>         return result;
>  }
> +
> +static int acpi_match_driver(struct device_driver *drv, void *data)
> +{
> +       struct acpi_driver *driver = to_acpi_driver(drv);
> +       struct acpi_device *device = (struct acpi_device *) data;
> +       int ret;
> +
> +       ret = acpi_match_device_ids(device, driver->ids);
> +       if (!ret)
> +               device->driver = driver;
> +
> +       return !ret;
> +}
> +
> +/**
> + * acpi_lookup_driver: Look up a driver for the notified ACPI device
> + * @handle: ACPI handle of the notified device object
> + * @event: Notify event
> + *
> + * Look up an associated driver for the notified ACPI device object
> + * by walking through the list of ACPI drivers.
> + */
> +struct acpi_driver *acpi_lookup_driver(acpi_handle handle, u32 event)
> +{
> +       struct acpi_device *device;
> +       struct acpi_driver *driver = NULL;
> +       unsigned long long sta;
> +       int type;
> +       int ret;
> +
> +       /* allocate a temporary device object */
> +       device = kzalloc(sizeof(struct acpi_device), GFP_KERNEL);
> +       if (!device) {
> +               pr_err(PREFIX "No memory to allocate a tmp device\n");
> +               return NULL;
> +       }
> +
> +       ret = acpi_bus_type_and_status(handle, &type, &sta);
> +       if (ret) {
> +               pr_err(PREFIX "Failed to get type of device\n");
> +               goto out;
> +       }
> +
> +       /* setup this temporary device object */
> +       INIT_LIST_HEAD(&device->pnp.ids);
> +       device->device_type = type;
> +       device->handle = handle;
> +       device->parent = acpi_bus_get_parent(handle);
> +       device->dev.bus = &acpi_bus_type;
> +       device->driver = NULL;
> +       STRUCT_TO_INT(device->status) = sta;
> +       device->status.present = 1;
> +
> +       /* set HID to this device object */
> +       acpi_device_set_id(device);
> +
> +       /* lookup a matching driver */
> +       (void) bus_for_each_drv(device->dev.bus, NULL,
> +                                       device, acpi_match_driver);
> +       driver = device->driver;

This path is used when we receive a Notify to a device and a matching
driver has been registered, but the driver is not bound to the device.
 For example, it may be a newly-added device where we haven't bound a
driver to it yet.

Is there anything that prevents us from unloading the driver between
here (the point where we capture the "struct acpi_driver *") and the
point where we call "driver->ops.sys_notify"?

> +
> +out:
> +       kfree(device);
> +       return driver;
> +}
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index bde976e..a773b46 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -345,6 +345,8 @@ extern int unregister_acpi_notifier(struct notifier_block *);
>
>  extern int register_acpi_bus_notifier(struct notifier_block *nb);
>  extern void unregister_acpi_bus_notifier(struct notifier_block *nb);
> +extern struct acpi_driver *acpi_lookup_driver(acpi_handle handle, u32 event);
> +
>  /*
>   * External Functions
>   */
> --
> 1.7.7.6
>

Re: [RFC PATCH 1/5] ACPI: Add acpi_lookup_driver() function

[Toshi Kani]

On Thu, 2012-08-30 at 22:42 -0700, Bjorn Helgaas wrote:
> On Thu, Aug 30, 2012 at 1:16 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > Added acpi_lookup_driver(), which looks up an associated driver
> > for the notified ACPI device object by walking through the list
> > of ACPI drivers.
> >
> > Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> > ---
> >  drivers/acpi/scan.c     |   65 +++++++++++++++++++++++++++++++++++++++++++++++
> >  include/acpi/acpi_bus.h |    2 +
> >  2 files changed, 67 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index d1ecca2..d0e0d18 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -1630,3 +1630,68 @@ int __init acpi_scan_init(void)
> >
> >         return result;
> >  }
> > +
> > +static int acpi_match_driver(struct device_driver *drv, void *data)
> > +{
> > +       struct acpi_driver *driver = to_acpi_driver(drv);
> > +       struct acpi_device *device = (struct acpi_device *) data;
> > +       int ret;
> > +
> > +       ret = acpi_match_device_ids(device, driver->ids);
> > +       if (!ret)
> > +               device->driver = driver;
> > +
> > +       return !ret;
> > +}
> > +
> > +/**
> > + * acpi_lookup_driver: Look up a driver for the notified ACPI device
> > + * @handle: ACPI handle of the notified device object
> > + * @event: Notify event
> > + *
> > + * Look up an associated driver for the notified ACPI device object
> > + * by walking through the list of ACPI drivers.
> > + */
> > +struct acpi_driver *acpi_lookup_driver(acpi_handle handle, u32 event)
> > +{
> > +       struct acpi_device *device;
> > +       struct acpi_driver *driver = NULL;
> > +       unsigned long long sta;
> > +       int type;
> > +       int ret;
> > +
> > +       /* allocate a temporary device object */
> > +       device = kzalloc(sizeof(struct acpi_device), GFP_KERNEL);
> > +       if (!device) {
> > +               pr_err(PREFIX "No memory to allocate a tmp device\n");
> > +               return NULL;
> > +       }
> > +
> > +       ret = acpi_bus_type_and_status(handle, &type, &sta);
> > +       if (ret) {
> > +               pr_err(PREFIX "Failed to get type of device\n");
> > +               goto out;
> > +       }
> > +
> > +       /* setup this temporary device object */
> > +       INIT_LIST_HEAD(&device->pnp.ids);
> > +       device->device_type = type;
> > +       device->handle = handle;
> > +       device->parent = acpi_bus_get_parent(handle);
> > +       device->dev.bus = &acpi_bus_type;
> > +       device->driver = NULL;
> > +       STRUCT_TO_INT(device->status) = sta;
> > +       device->status.present = 1;
> > +
> > +       /* set HID to this device object */
> > +       acpi_device_set_id(device);
> > +
> > +       /* lookup a matching driver */
> > +       (void) bus_for_each_drv(device->dev.bus, NULL,
> > +                                       device, acpi_match_driver);
> > +       driver = device->driver;
> 
> This path is used when we receive a Notify to a device and a matching
> driver has been registered, but the driver is not bound to the device.
>  For example, it may be a newly-added device where we haven't bound a
> driver to it yet.
> 
> Is there anything that prevents us from unloading the driver between
> here (the point where we capture the "struct acpi_driver *") and the
> point where we call "driver->ops.sys_notify"?

Hi Bjorn,

That's a very good question.  I will look into how we protect a driver
from unloading when we try to attach & probe a driver in the
acpi_add_single_object() path, and see if we can use a similar way to
protect it here as well.

Thanks,
-Toshi


> > +
> > +out:
> > +       kfree(device);
> > +       return driver;
> > +}
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index bde976e..a773b46 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -345,6 +345,8 @@ extern int unregister_acpi_notifier(struct notifier_block *);
> >
> >  extern int register_acpi_bus_notifier(struct notifier_block *nb);
> >  extern void unregister_acpi_bus_notifier(struct notifier_block *nb);
> > +extern struct acpi_driver *acpi_lookup_driver(acpi_handle handle, u32 event);
> > +
> >  /*
> >   * External Functions
> >   */
> > --
> > 1.7.7.6
> >

Re: [RFC PATCH 1/5] ACPI: Add acpi_lookup_driver() function

[Toshi Kani]

(snip)
 :
> > > +
> > > +       /* set HID to this device object */
> > > +       acpi_device_set_id(device);
> > > +
> > > +       /* lookup a matching driver */
> > > +       (void) bus_for_each_drv(device->dev.bus, NULL,
> > > +                                       device, acpi_match_driver);
> > > +       driver = device->driver;
> > 
> > This path is used when we receive a Notify to a device and a matching
> > driver has been registered, but the driver is not bound to the device.
> >  For example, it may be a newly-added device where we haven't bound a
> > driver to it yet.
> > 
> > Is there anything that prevents us from unloading the driver between
> > here (the point where we capture the "struct acpi_driver *") and the
> > point where we call "driver->ops.sys_notify"?
> 
> Hi Bjorn,
> 
> That's a very good question.  I will look into how we protect a driver
> from unloading when we try to attach & probe a driver in the
> acpi_add_single_object() path, and see if we can use a similar way to
> protect it here as well.

I have protected this path in version 2, which I just sent out.  It
calls .sys_notify() directly from bus_for_each_drv(), which protects the
driver entry by increasing its reference count while the callback
function is being called.

Thanks,
-Toshi