Skip to contentSkip to conversation

ACPI: support system events for .notify

PATCH In review Bjorn Helgaas 4 participants 11 messages Apr 30, 2009 – May 28, 2009 1 message not in the archive

Subsystems (4)
ACPI x86 platform drivers ACPI component architecture (acpica) ASUS notebooks and eeepc ACPI/WMI extras drivers
lore
Share & tools
lore

Bjorn Helgaas

author
#0 lore raw

cover ACPI: support system events for .notify

This series extends the .notify method I recently added so drivers can request system notification events as well as device-specific events. This simplifies some drivers a bit.

Comments welcome.

---

Bjorn Helgaas (6):
ACPI: eeepc-laptop: use .notify method instead of installing handler directly
ACPI: asus-acpi: use .notify method instead of installing handler directly
ACPI: asus-laptop: use .notify method instead of installing handler directly
ACPI: battery: use .notify method instead of installing handler directly
ACPI: ac: use .notify method instead of installing handler directly
ACPI: allow drivers to request both device and system notify events

 drivers/acpi/ac.c                   |   20 +++++---------------
 drivers/acpi/battery.c              |   22 +++++-----------------
 drivers/acpi/bus.c                  |    6 +++++-
 drivers/platform/x86/asus-laptop.c  |   23 +++++------------------
 drivers/platform/x86/asus_acpi.c    |   30 +++++++++++++-----------------
 drivers/platform/x86/eeepc-laptop.c |   19 ++++++-------------
 include/acpi/acpi_bus.h             |    3 +++
 7 files changed, 42 insertions(+), 81 deletions(-)

------------------------------------------------------------------------------
Register Now & Save for Velocity, the Web Performance & Operations Conference from O'Reilly Media. Velocity features a full day of expert-led, hands-on workshops and two days of sessions from industry leaders in dedicated Performance & Operations tracks. Use code vel09scf and Save an extra 15% before 5/3. http://p.sf.net/sfu/velocityconf

#1 lore raw

PATCH 1/6 ACPI: allow drivers to request both device and system notify events

System notify events (0x00-0x7f) are common across all device types and should be handled in Linux/ACPI, not in drivers. However, some BIOSes use system notify events in device-specific ways that require the driver to be involved.

This patch adds a ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag. When a driver sets this flag and supplies a .notify method, Linux/ACPI calls the .notify method for ALL notify events on the device, not just the device-specific (0x80-0xff) events.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/bus.c      |    6 +++++-
 include/acpi/acpi_bus.h |    3 +++
 2 files changed, 8 insertions(+), 1 deletions(-)
drivers/acpi/bus.c +5 -1
index e8f7b64..fd6a930 100644
@@ -549,6 +549,7 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
 {
 	int result = 0;
 	struct acpi_device *device = NULL;
+	struct acpi_driver *driver;
 
 	blocking_notifier_call_chain(&acpi_bus_notify_list,
 		type, (void *)handle);

@@ -629,7 +630,10 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
 		break;
 	}
 
-	return;
+	driver = device->driver;
+	if (driver && driver->ops.notify &&
+	    (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS))
+		driver->ops.notify(device, type);
 }
 
 /* --------------------------------------------------------------------------
include/acpi/acpi_bus.h +3 -0
index c34b110..84e35d5 100644
@@ -114,10 +114,13 @@ struct acpi_device_ops {
 	acpi_op_notify notify;
 };
 
+#define ACPI_DRIVER_ALL_NOTIFY_EVENTS	0x1	/* system AND device events */
+
 struct acpi_driver {
 	char name[80];
 	char class[80];
 	const struct acpi_device_id *ids; /* Supported Hardware IDs */
+	unsigned int flags;
 	struct acpi_device_ops ops;
 	struct device_driver drv;
 	struct module *owner;
reviewer
#2 lore raw

Re: [PATCH 1/6] ACPI: allow drivers to request both device and system notify events

Quoted 9 lines · Bjorn Helgaas

System notify events (0x00-0x7f) are common across all device types and should be handled in Linux/ACPI, not in drivers. However, some BIOSes use system notify events in device-specific ways that require the driver to be involved.

This patch adds a ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag. When a driver sets this flag and supplies a .notify method, Linux/ACPI calls the .notify method for ALL notify events on the device, not just the device-specific (0x80-0xff) events.

What bad things would happen if we did this by default rather than requiring a driver to ask for it?

thanks,
-Len Brown, Intel Open Source Technology Center

Quoted 49 lines
Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/bus.c      |    6 +++++-
 include/acpi/acpi_bus.h |    3 +++
 2 files changed, 8 insertions(+), 1 deletions(-)
drivers/acpi/bus.c +5 -1
index e8f7b64..fd6a930 100644
@@ -549,6 +549,7 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
 {
 	int result = 0;
 	struct acpi_device *device = NULL;
+	struct acpi_driver *driver;
 
 	blocking_notifier_call_chain(&acpi_bus_notify_list,
 		type, (void *)handle);

@@ -629,7 +630,10 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
 		break;
 	}
 
-	return;
+	driver = device->driver;
+	if (driver && driver->ops.notify &&
+	    (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS))
+		driver->ops.notify(device, type);
 }
 
 /* --------------------------------------------------------------------------
include/acpi/acpi_bus.h +3 -0
index c34b110..84e35d5 100644
@@ -114,10 +114,13 @@ struct acpi_device_ops {
 	acpi_op_notify notify;
 };
 
+#define ACPI_DRIVER_ALL_NOTIFY_EVENTS	0x1	/* system AND device events */
+
 struct acpi_driver {
 	char name[80];
 	char class[80];
 	const struct acpi_device_id *ids; /* Supported Hardware IDs */
+	unsigned int flags;
 	struct acpi_device_ops ops;
 	struct device_driver drv;
 	struct module *owner;
#3 lore raw

Re: [PATCH 1/6] ACPI: allow drivers to request both device and system notify events

Quoted 7 lines · Len Brown
Quoted 5 lines · Bjorn Helgaas

System notify events (0x00-0x7f) are common across all device types and should be handled in Linux/ACPI, not in drivers. However, some BIOSes use system notify events in device-specific ways that require the driver to be involved.

Quoted 7 lines
Quoted 4 lines

This patch adds a ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag. When a driver sets this flag and supplies a .notify method, Linux/ACPI calls the .notify method for ALL notify events on the device, not just the device-specific (0x80-0xff) events.

What bad things would happen if we did this by default rather than requiring a driver to ask for it?

If the Linux/ACPI code were doing the right thing, I don't think drivers would have to know about the system events.

I think some of the drivers that use ACPI_ALL_NOTIFY probably do it to deal with BIOSes that use system events when they should be using device events. And some are probably using it unnecessarily.

If we sent all events to drivers by default, I guess we'd have to tweak a few driver .notify methods to ignore the system ones. Maybe that's the right thing.

Bjorn

Quoted 50 lines
Quoted 49 lines
Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/bus.c      |    6 +++++-
 include/acpi/acpi_bus.h |    3 +++
 2 files changed, 8 insertions(+), 1 deletions(-)
drivers/acpi/bus.c +5 -1
index e8f7b64..fd6a930 100644
@@ -549,6 +549,7 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
 {
 	int result = 0;
 	struct acpi_device *device = NULL;
+	struct acpi_driver *driver;
 
 	blocking_notifier_call_chain(&acpi_bus_notify_list,
 		type, (void *)handle);

@@ -629,7 +630,10 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
 		break;
 	}
 
-	return;
+	driver = device->driver;
+	if (driver && driver->ops.notify &&
+	    (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS))
+		driver->ops.notify(device, type);
 }
 
 /* --------------------------------------------------------------------------
include/acpi/acpi_bus.h +3 -0
index c34b110..84e35d5 100644
@@ -114,10 +114,13 @@ struct acpi_device_ops {
 	acpi_op_notify notify;
 };
 
+#define ACPI_DRIVER_ALL_NOTIFY_EVENTS	0x1	/* system AND device events */
+
 struct acpi_driver {
 	char name[80];
 	char class[80];
 	const struct acpi_device_id *ids; /* Supported Hardware IDs */
+	unsigned int flags;
 	struct acpi_device_ops ops;
 	struct device_driver drv;
 	struct module *owner;

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#5 lore raw

PATCH 2/6 ACPI: ac: use .notify method instead of installing handler directly

This patch adds a .notify() method. The presence of .notify() causes Linux/ACPI to manage event handlers and notify handlers on our behalf, so we don't have to install and remove them ourselves.

This driver apparently relies on seeing ALL notify events, not just device-specific ones (because it used ACPI_ALL_NOTIFY). We use the ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag to request all events.

Signed-off-by Bjorn Helgaas <[email protected]>
CC Alexey Starikovskiy <alexey.y.starikovskiy-VuQAYsv1563Yd54FQh9/[email protected]>
---
 drivers/acpi/ac.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)
drivers/acpi/ac.c +5 -15
index 88e42ab..0df8fcb 100644
@@ -61,6 +61,7 @@ static int acpi_ac_open_fs(struct inode *inode, struct file *file);
 static int acpi_ac_add(struct acpi_device *device);
 static int acpi_ac_remove(struct acpi_device *device, int type);
 static int acpi_ac_resume(struct acpi_device *device);
+static void acpi_ac_notify(struct acpi_device *device, u32 event);
 
 static const struct acpi_device_id ac_device_ids[] = {
 	{"ACPI0003", 0},

@@ -72,10 +73,12 @@ static struct acpi_driver acpi_ac_driver = {
 	.name = "ac",
 	.class = ACPI_AC_CLASS,
 	.ids = ac_device_ids,
+	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = acpi_ac_add,
 		.remove = acpi_ac_remove,
 		.resume = acpi_ac_resume,
+		.notify = acpi_ac_notify,
 		},
 };
 

@@ -220,16 +223,14 @@ static int acpi_ac_remove_fs(struct acpi_device *device)
                                    Driver Model
    -------------------------------------------------------------------------- */
 
-static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_ac_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_ac *ac = data;
-	struct acpi_device *device = NULL;
+	struct acpi_ac *ac = acpi_driver_data(device);
 
 
 	if (!ac)
 		return;
 
-	device = ac->device;
 	switch (event) {
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,

@@ -253,7 +254,6 @@ static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
 static int acpi_ac_add(struct acpi_device *device)
 {
 	int result = 0;
-	acpi_status status = AE_OK;
 	struct acpi_ac *ac = NULL;
 
 

@@ -286,13 +286,6 @@ static int acpi_ac_add(struct acpi_device *device)
 	ac->charger.get_property = get_ac_property;
 	power_supply_register(&ac->device->dev, &ac->charger);
 #endif
-	status = acpi_install_notify_handler(device->handle,
-					     ACPI_ALL_NOTIFY, acpi_ac_notify,
-					     ac);
-	if (ACPI_FAILURE(status)) {
-		result = -ENODEV;
-		goto end;
-	}
 
 	printk(KERN_INFO PREFIX "%s [%s] (%s)\n",
 	       acpi_device_name(device), acpi_device_bid(device),

@@ -328,7 +321,6 @@ static int acpi_ac_resume(struct acpi_device *device)
 
 static int acpi_ac_remove(struct acpi_device *device, int type)
 {
-	acpi_status status = AE_OK;
 	struct acpi_ac *ac = NULL;
 
 

@@ -337,8 +329,6 @@ static int acpi_ac_remove(struct acpi_device *device, int type)
 
 	ac = acpi_driver_data(device);
 
-	status = acpi_remove_notify_handler(device->handle,
-					    ACPI_ALL_NOTIFY, acpi_ac_notify);
 #ifdef CONFIG_ACPI_SYSFS_POWER
 	if (ac->charger.dev)
 		power_supply_unregister(&ac->charger);

------------------------------------------------------------------------------
Register Now & Save for Velocity, the Web Performance & Operations Conference from O'Reilly Media. Velocity features a full day of expert-led, hands-on workshops and two days of sessions from industry leaders in dedicated Performance & Operations tracks. Use code vel09scf and Save an extra 15% before 5/3. http://p.sf.net/sfu/velocityconf

author
#6 lore raw

PATCH 4/6 ACPI: asus-laptop: use .notify method instead of installing handler directly

In reply to a message not in the archive

This patch adds a .notify() method. The presence of .notify() causes Linux/ACPI to manage event handlers and notify handlers on our behalf, so we don't have to install and remove them ourselves.

This driver apparently relies on seeing ALL notify events, not just device-specific ones (because it used ACPI_ALL_NOTIFY). We use the ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag to request all events.

Signed-off-by Bjorn Helgaas <[email protected]>
CC Corentin Chary <[email protected]>
CC [email protected]
---
 drivers/platform/x86/asus-laptop.c |   23 +++++------------------
 1 files changed, 5 insertions(+), 18 deletions(-)
drivers/platform/x86/asus-laptop.c +5 -18
index eeafc6c..22a38fe 100644
@@ -207,13 +207,17 @@ MODULE_DEVICE_TABLE(acpi, asus_device_ids);
 
 static int asus_hotk_add(struct acpi_device *device);
 static int asus_hotk_remove(struct acpi_device *device, int type);
+static void asus_hotk_notify(struct acpi_device *device, u32 event);
+
 static struct acpi_driver asus_hotk_driver = {
 	.name = ASUS_HOTK_NAME,
 	.class = ASUS_HOTK_CLASS,
 	.ids = asus_device_ids,
+	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = asus_hotk_add,
 		.remove = asus_hotk_remove,
+		.notify = asus_hotk_notify,
 		},
 };
 

@@ -812,7 +816,7 @@ static int asus_setkeycode(struct input_dev *dev, int scancode, int keycode)
 	return -EINVAL;
 }
 
-static void asus_hotk_notify(acpi_handle handle, u32 event, void *data)
+static void asus_hotk_notify(struct acpi_device *device, u32 event)
 {
 	static struct key_entry *key;
 	u16 count;

@@ -1124,7 +1128,6 @@ static int asus_hotk_found;
 
 static int asus_hotk_add(struct acpi_device *device)
 {
-	acpi_status status = AE_OK;
 	int result;
 
 	if (!device)

@@ -1149,15 +1152,6 @@ static int asus_hotk_add(struct acpi_device *device)
 
 	asus_hotk_add_fs();
 
-	/*
-	 * We install the handler, it will receive the hotk in parameter, so, we
-	 * could add other data to the hotk struct
-	 */
-	status = acpi_install_notify_handler(hotk->handle, ACPI_ALL_NOTIFY,
-					     asus_hotk_notify, hotk);
-	if (ACPI_FAILURE(status))
-		printk(ASUS_ERR "Error installing notify handler\n");
-
 	asus_hotk_found = 1;
 
 	/* WLED and BLED are on by default */

@@ -1198,16 +1192,9 @@ end:
 
 static int asus_hotk_remove(struct acpi_device *device, int type)
 {
-	acpi_status status = 0;
-
 	if (!device || !acpi_driver_data(device))
 		return -EINVAL;
 
-	status = acpi_remove_notify_handler(hotk->handle, ACPI_ALL_NOTIFY,
-					    asus_hotk_notify);
-	if (ACPI_FAILURE(status))
-		printk(ASUS_ERR "Error removing notify handler\n");
-
 	kfree(hotk->name);
 	kfree(hotk);

------------------------------------------------------------------------------
Register Now & Save for Velocity, the Web Performance & Operations Conference from O'Reilly Media. Velocity features a full day of expert-led, hands-on workshops and two days of sessions from industry leaders in dedicated Performance & Operations tracks. Use code vel09scf and Save an extra 15% before 5/3. http://p.sf.net/sfu/velocityconf

author
#7 lore raw

PATCH 5/6 ACPI: asus-acpi: use .notify method instead of installing handler directly

In reply to a message not in the archive

This patch adds a .notify() method. The presence of .notify() causes Linux/ACPI to manage event handlers and notify handlers on our behalf, so we don't have to install and remove them ourselves.

This driver relies on seeing system notify events, not device-specific ones (because it used ACPI_SYSTEM_NOTIFY). We use the ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag to request all events, then just ignore any device events we get.

Signed-off-by Bjorn Helgaas <[email protected]>
CC Corentin Chary <[email protected]>
CC Karol Kozimor <[email protected]>
CC [email protected]
---
 drivers/platform/x86/asus_acpi.c |   30 +++++++++++++-----------------
 1 files changed, 13 insertions(+), 17 deletions(-)
drivers/platform/x86/asus_acpi.c +13 -17
index ba1f749..ddf5240 100644
@@ -455,6 +455,8 @@ static struct asus_hotk *hotk;
  */
 static int asus_hotk_add(struct acpi_device *device);
 static int asus_hotk_remove(struct acpi_device *device, int type);
+static void asus_hotk_notify(struct acpi_device *device, u32 event);
+
 static const struct acpi_device_id asus_device_ids[] = {
 	{"ATK0100", 0},
 	{"", 0},

@@ -465,9 +467,11 @@ static struct acpi_driver asus_hotk_driver = {
 	.name = "asus_acpi",
 	.class = ACPI_HOTK_CLASS,
 	.ids = asus_device_ids,
+	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = asus_hotk_add,
 		.remove = asus_hotk_remove,
+		.notify = asus_hotk_notify,
 		},
 };
 

@@ -1101,12 +1105,20 @@ static int asus_hotk_remove_fs(struct acpi_device *device)
 	return 0;
 }
 
-static void asus_hotk_notify(acpi_handle handle, u32 event, void *data)
+static void asus_hotk_notify(struct acpi_device *device, u32 event)
 {
 	/* TODO Find a better way to handle events count. */
 	if (!hotk)
 		return;
 
+	/*
+	 * The BIOS *should* be sending us device events, but apparently
+	 * Asus uses system events instead, so just ignore any device
+	 * events we get.
+	 */
+	if (event > ACPI_MAX_SYS_NOTIFY)
+		return;
+
 	if ((event & ~((u32) BR_UP)) < 16)
 		hotk->brightness = (event & ~((u32) BR_UP));
 	else if ((event & ~((u32) BR_DOWN)) < 16)

@@ -1346,15 +1358,6 @@ static int asus_hotk_add(struct acpi_device *device)
 	if (result)
 		goto end;
 
-	/*
-	 * We install the handler, it will receive the hotk in parameter, so, we
-	 * could add other data to the hotk struct
-	 */
-	status = acpi_install_notify_handler(hotk->handle, ACPI_SYSTEM_NOTIFY,
-					     asus_hotk_notify, hotk);
-	if (ACPI_FAILURE(status))
-		printk(KERN_ERR "  Error installing notify handler\n");
-
 	/* For laptops without GPLV: init the hotk->brightness value */
 	if ((!hotk->methods->brightness_get)
 	    && (!hotk->methods->brightness_status)

@@ -1389,16 +1392,9 @@ end:
 
 static int asus_hotk_remove(struct acpi_device *device, int type)
 {
-	acpi_status status = 0;
-
 	if (!device || !acpi_driver_data(device))
 		return -EINVAL;
 
-	status = acpi_remove_notify_handler(hotk->handle, ACPI_SYSTEM_NOTIFY,
-					    asus_hotk_notify);
-	if (ACPI_FAILURE(status))
-		printk(KERN_ERR "Asus ACPI: Error removing notify handler\n");
-
 	asus_hotk_remove_fs(device);
 
 	kfree(hotk);

------------------------------------------------------------------------------
Register Now & Save for Velocity, the Web Performance & Operations Conference from O'Reilly Media. Velocity features a full day of expert-led, hands-on workshops and two days of sessions from industry leaders in dedicated Performance & Operations tracks. Use code vel09scf and Save an extra 15% before 5/3. http://p.sf.net/sfu/velocityconf

author
#8 lore raw

PATCH 6/6 ACPI: eeepc-laptop: use .notify method instead of installing handler directly

In reply to a message not in the archive

This patch adds a .notify() method. The presence of .notify() causes Linux/ACPI to manage event handlers and notify handlers on our behalf, so we don't have to install and remove them ourselves.

This driver relies on seeing system notify events, not device-specific ones (because it used ACPI_SYSTEM_NOTIFY). We use the ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag to request all events, then just ignore any device events we get.

Signed-off-by Bjorn Helgaas <[email protected]>
CC Corentin Chary <[email protected]>
CC [email protected]
CC Matthew Garrett <[email protected]>
---
 drivers/platform/x86/eeepc-laptop.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)
drivers/platform/x86/eeepc-laptop.c +6 -13
index 6f54fd1..d9424fd 100644
@@ -177,6 +177,7 @@ static struct key_entry eeepc_keymap[] = {
  */
 static int eeepc_hotk_add(struct acpi_device *device);
 static int eeepc_hotk_remove(struct acpi_device *device, int type);
+static void eeepc_hotk_notify(struct acpi_device *device, u32 event);
 
 static const struct acpi_device_id eeepc_device_ids[] = {
 	{EEEPC_HOTK_HID, 0},

@@ -188,9 +189,11 @@ static struct acpi_driver eeepc_hotk_driver = {
 	.name = EEEPC_HOTK_NAME,
 	.class = EEEPC_HOTK_CLASS,
 	.ids = eeepc_device_ids,
+	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = eeepc_hotk_add,
 		.remove = eeepc_hotk_remove,
+		.notify = eeepc_hotk_notify,
 	},
 };
 

@@ -554,13 +557,15 @@ static void eeepc_rfkill_notify(acpi_handle handle, u32 event, void *data)
 	}
 }
 
-static void eeepc_hotk_notify(acpi_handle handle, u32 event, void *data)
+static void eeepc_hotk_notify(struct acpi_device *device, u32 event)
 {
 	static struct key_entry *key;
 	u16 count;
 
 	if (!ehotk)
 		return;
+	if (event > ACPI_MAX_SYS_NOTIFY)
+		return;
 	if (event >= NOTIFY_BRN_MIN && event <= NOTIFY_BRN_MAX)
 		notify_brn();
 	count = ehotk->event_count[event % 128]++;

@@ -626,7 +631,6 @@ static void eeepc_unregister_rfkill_notifier(char *node)
 
 static int eeepc_hotk_add(struct acpi_device *device)
 {
-	acpi_status status = AE_OK;
 	int result;
 
 	if (!device)

@@ -644,11 +648,6 @@ static int eeepc_hotk_add(struct acpi_device *device)
 	result = eeepc_hotk_check();
 	if (result)
 		goto ehotk_fail;
-	status = acpi_install_notify_handler(ehotk->handle, ACPI_SYSTEM_NOTIFY,
-					     eeepc_hotk_notify, ehotk);
-	if (ACPI_FAILURE(status))
-		printk(EEEPC_ERR "Error installing notify handler\n");
-
 	if (get_acpi(CM_ASL_WLAN) != -1) {
 		ehotk->eeepc_wlan_rfkill = rfkill_allocate(&device->dev,
 							   RFKILL_TYPE_WLAN);

@@ -726,14 +725,8 @@ static int eeepc_hotk_add(struct acpi_device *device)
 
 static int eeepc_hotk_remove(struct acpi_device *device, int type)
 {
-	acpi_status status = 0;
-
 	if (!device || !acpi_driver_data(device))
 		 return -EINVAL;
-	status = acpi_remove_notify_handler(ehotk->handle, ACPI_SYSTEM_NOTIFY,
-					    eeepc_hotk_notify);
-	if (ACPI_FAILURE(status))
-		printk(EEEPC_ERR "Error removing notify handler\n");
 
 	eeepc_unregister_rfkill_notifier("\\_SB.PCI0.P0P6");
 	eeepc_unregister_rfkill_notifier("\\_SB.PCI0.P0P7");

------------------------------------------------------------------------------
Register Now & Save for Velocity, the Web Performance & Operations Conference from O'Reilly Media. Velocity features a full day of expert-led, hands-on workshops and two days of sessions from industry leaders in dedicated Performance & Operations tracks. Use code vel09scf and Save an extra 15% before 5/3. http://p.sf.net/sfu/velocityconf

reviewer
#9 lore raw

Re: [PATCH 6/6] ACPI: eeepc-laptop: use .notify method instead of installing handler directly

This one conflicts with recent addition of
eeepc_register_rfkill_notifier()

thanks,
Len Brown, Intel Open Source Technology Center

#10 lore raw
In reply to a message not in the archive

applied to acpi-test

thanks,
Len Brown, Intel Open Source Technology Center

Quoted 17 lines · Bjorn Helgaas

This series extends the .notify method I recently added so drivers can request system notification events as well as device-specific events. This simplifies some drivers a bit.

Comments welcome.

---

Bjorn Helgaas (6):
ACPI: eeepc-laptop: use .notify method instead of installing handler directly
ACPI: asus-acpi: use .notify method instead of installing handler directly
ACPI: asus-laptop: use .notify method instead of installing handler directly
ACPI: battery: use .notify method instead of installing handler directly
ACPI: ac: use .notify method instead of installing handler directly
ACPI: allow drivers to request both device and system notify events

Quoted 9 lines
 drivers/acpi/ac.c                   |   20 +++++---------------
 drivers/acpi/battery.c              |   22 +++++-----------------
 drivers/acpi/bus.c                  |    6 +++++-
 drivers/platform/x86/asus-laptop.c  |   23 +++++------------------
 drivers/platform/x86/asus_acpi.c    |   30 +++++++++++++-----------------
 drivers/platform/x86/eeepc-laptop.c |   19 ++++++-------------
 include/acpi/acpi_bus.h             |    3 +++
 7 files changed, 42 insertions(+), 81 deletions(-)

------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT is a gathering of tech-side developers & brand creativity professionals. Meet the minds behind Google Creative Lab, Visual Complexity, Processing, & iPhoneDevCamp as they present alongside digital heavyweights like Barbarian Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com

#11 lore raw

PATCH 3/6 ACPI: battery: use .notify method instead of installing handler directly

In reply to Bjorn Helgaas

This patch adds a .notify() method. The presence of .notify() causes Linux/ACPI to manage event handlers and notify handlers on our behalf, so we don't have to install and remove them ourselves.

This driver apparently relies on seeing ALL notify events, not just device-specific ones (because it used ACPI_ALL_NOTIFY). We use the ACPI_DRIVER_ALL_NOTIFY_EVENTS driver flag to request all events.

Signed-off-by Bjorn Helgaas <[email protected]>
CC Alexey Starikovskiy <[email protected]>
---
 drivers/acpi/battery.c |   22 +++++-----------------
 1 files changed, 5 insertions(+), 17 deletions(-)
drivers/acpi/battery.c +5 -17
index b0de631..eb00c4e 100644
@@ -796,13 +796,12 @@ static void acpi_battery_remove_fs(struct acpi_device *device)
                                  Driver Interface
    -------------------------------------------------------------------------- */
 
-static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_battery_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_battery *battery = data;
-	struct acpi_device *device;
+	struct acpi_battery *battery = acpi_driver_data(device);
+
 	if (!battery)
 		return;
-	device = battery->device;
 	acpi_battery_update(battery);
 	acpi_bus_generate_proc_event(device, event,
 				     acpi_battery_present(battery));

@@ -819,7 +818,6 @@ static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
 static int acpi_battery_add(struct acpi_device *device)
 {
 	int result = 0;
-	acpi_status status = 0;
 	struct acpi_battery *battery = NULL;
 	if (!device)
 		return -EINVAL;

@@ -837,14 +835,6 @@ static int acpi_battery_add(struct acpi_device *device)
 	if (result)
 		goto end;
 #endif
-	status = acpi_install_notify_handler(device->handle,
-					     ACPI_ALL_NOTIFY,
-					     acpi_battery_notify, battery);
-	if (ACPI_FAILURE(status)) {
-		ACPI_EXCEPTION((AE_INFO, status, "Installing notify handler"));
-		result = -ENODEV;
-		goto end;
-	}
 	printk(KERN_INFO PREFIX "%s Slot [%s] (battery %s)\n",
 	       ACPI_BATTERY_DEVICE_NAME, acpi_device_bid(device),
 	       device->status.battery_present ? "present" : "absent");

@@ -860,15 +850,11 @@ static int acpi_battery_add(struct acpi_device *device)
 
 static int acpi_battery_remove(struct acpi_device *device, int type)
 {
-	acpi_status status = 0;
 	struct acpi_battery *battery = NULL;
 
 	if (!device || !acpi_driver_data(device))
 		return -EINVAL;
 	battery = acpi_driver_data(device);
-	status = acpi_remove_notify_handler(device->handle,
-					    ACPI_ALL_NOTIFY,
-					    acpi_battery_notify);
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_battery_remove_fs(device);
 #endif

@@ -896,10 +882,12 @@ static struct acpi_driver acpi_battery_driver = {
 	.name = "battery",
 	.class = ACPI_BATTERY_CLASS,
 	.ids = battery_device_ids,
+	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = acpi_battery_add,
 		.resume = acpi_battery_resume,
 		.remove = acpi_battery_remove,
+		.notify = acpi_battery_notify,
 		},
 };