Skip to contentSkip to conversation

Make ACPI button driver an input device

RFC PATCH In review Matthew Garrett 6 participants 13 messages Apr 19, 2006 – Apr 24, 2006

Subsystems (2)
ACPI Input drivers
lore
Share & tools
lore

Matthew Garrett

author
#0 lore raw

RFC PATCH 1/1 Make ACPI button driver an input device

Sleep and power buttons are logically part of the keyboard, and it makes for them to be exposed via an input device rather than an odd file in /proc. This patch adds a keycode for lid switches (are we running out of keycodes?) and allows the button driver to register an input device.

Signed-off-by Matthew Garrett <[email protected]>
drivers/acpi/button.c renamed from acpi.bak/button.c +58 -0
diff -ur drivers/acpi.bak/button.c drivers/acpi/button.c
@@ -26,6 +26,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/input.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>

@@ -102,6 +103,8 @@
 	.release = single_release,
 };
 
+static struct input_dev *input_dev;
+
 /* --------------------------------------------------------------------------
                               FS Interface (/proc)
    -------------------------------------------------------------------------- */

@@ -260,6 +263,37 @@
                                 Driver Interface
    -------------------------------------------------------------------------- */
 
+static void acpi_button_report (struct acpi_button *button)
+{
+	int keycode = 0;
+
+	ACPI_FUNCTION_TRACE("acpi_button_report");
+
+	switch(button->type) {
+	case ACPI_BUTTON_TYPE_POWER:
+	case ACPI_BUTTON_TYPE_POWERF:
+		keycode = KEY_POWER;
+		break;
+	case ACPI_BUTTON_TYPE_SLEEP:
+	case ACPI_BUTTON_TYPE_SLEEPF:
+		keycode = KEY_SLEEP;
+		break;
+	case ACPI_BUTTON_TYPE_LID:
+		keycode = KEY_LID;
+		break;
+
+	}
+
+	if (keycode) {
+		input_report_key(input_dev, keycode, 1);
+		input_sync(input_dev);
+		input_report_key(input_dev, keycode, 0);
+		input_sync(input_dev);
+	}
+
+	return_VOID;
+}
+
 static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 {
 	struct acpi_button *button = (struct acpi_button *)data;

@@ -273,6 +307,7 @@
 	case ACPI_BUTTON_NOTIFY_STATUS:
 		acpi_bus_generate_event(button->device, event,
 					++button->pushed);
+		acpi_button_report(button);		
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,

@@ -467,6 +502,26 @@
 		return_VALUE(-ENODEV);
 	}
 
+	input_dev = input_allocate_device();
+
+	if (input_dev) {
+		int error;
+
+		input_dev->name = "ACPI button driver";
+		input_dev->phys = "acpi/input0";
+		input_dev->id.bustype = BUS_HOST;
+		input_dev->evbit[LONG(EV_KEY)] = BIT(EV_KEY);
+		set_bit(KEY_SLEEP, input_dev->keybit);
+		set_bit(KEY_POWER, input_dev->keybit);
+		set_bit(KEY_LID, input_dev->keybit);
+
+		error = input_register_device(input_dev);
+		if (error) {
+			printk(KERN_ERR "Unable to register ACPI input device\n");
+			input_free_device(input_dev);
+		}
+	}
+
 	return_VALUE(0);
 }
 

@@ -484,6 +539,9 @@
 		remove_proc_entry(ACPI_BUTTON_SUBCLASS_LID, acpi_button_dir);
 	remove_proc_entry(ACPI_BUTTON_CLASS, acpi_root_dir);
 
+	if (input_dev)
+		input_unregister_device(input_dev);
+
 	return_VOID;
 }
include/linux/input.h renamed from linux/input.h.bak +1 -0
@@ -344,6 +344,7 @@
 #define KEY_FORWARDMAIL		233
 #define KEY_SAVE		234
 #define KEY_DOCUMENTS		235
+#define KEY_LID			237
 
 #define KEY_UNKNOWN		240
Signature · 1 line
Matthew Garrett | [email protected]
#1 lore raw

include/linux/input.h line 347

Quoted 6 lines · Matthew Garrett

+++ a/include/linux/input.h 2006-04-19 20:49:18 +0100

include/linux/input.h renamed from linux/input.h.bak +1 -0
@@ -344,6 +344,7 @@
 #define KEY_FORWARDMAIL		233
 #define KEY_SAVE		234
 #define KEY_DOCUMENTS		235
+#define KEY_LID			237

What about 236?

Dominik

author
#2 lore raw
Quoted 9 lines · Dominik Brodowski

include/linux/input.h line 347

Quoted 6 lines · Matthew Garrett

+++ a/include/linux/input.h 2006-04-19 20:49:18 +0100

include/linux/input.h renamed from linux/input.h.bak +1 -0
@@ -344,6 +344,7 @@
 #define KEY_FORWARDMAIL		233
 #define KEY_SAVE		234
 #define KEY_DOCUMENTS		235
+#define KEY_LID			237

What about 236?

I sent a patch last month that uses 236 for KEY_BATTERY, so decided not to conflict with it.

Signature · 1 line
Matthew Garrett | [email protected]
maintainer
#3 lore raw
Quoted 11 lines · Matthew Garrett
Quoted 9 lines · Dominik Brodowski

include/linux/input.h line 347

Quoted 6 lines · Matthew Garrett

+++ a/include/linux/input.h 2006-04-19 20:49:18 +0100

include/linux/input.h renamed from linux/input.h.bak +1 -0
@@ -344,6 +344,7 @@
 #define KEY_FORWARDMAIL            233
 #define KEY_SAVE           234
 #define KEY_DOCUMENTS              235
+#define KEY_LID                    237

What about 236?

Quoted 3 lines

I sent a patch last month that uses 236 for KEY_BATTERY, so decided not to conflict with it.

Yes, I still need to apply it.

Matthew, I would recommend not adding KEY_LID but using one of the switch codes (SW_0?) for the lid.

Richard, on your handhelds what switch would be most similar to notebook's lid? Should we alias one of the switches to SW_LID?

--
Dmitry

author
#4 lore raw
Quoted 2 lines · Dmitry Torokhov

Matthew, I would recommend not adding KEY_LID but using one of the switch codes (SW_0?) for the lid.

Ok, sounds good. I'll wait to see which one seems most preferable.

Signature · 1 line
Matthew Garrett | [email protected]
#5 lore raw
In reply to Dmitry Torokhov
Quoted 7 lines · Dmitry Torokhov

Yes, I still need to apply it.

Matthew, I would recommend not adding KEY_LID but using one of the switch codes (SW_0?) for the lid.

Richard, on your handhelds what switch would be most similar to notebook's lid? Should we alias one of the switches to SW_LID?

This gets tricky as the handhelds have two "lid" switches. Pictures of how it can fold are at http://www.dynamism.com/sl-c3000/main.shtml . To summarise, it can be in three positions:

* Screen folded facing the keys (shut like a laptop)
* Screen open above the keyboard (like an open laptop)
* Screen folded over the keys but with the screen visible (becomes a tablet like handheld with no keyboard)

Shut is when both switches (SW_0 and SW_1) are pressed, open is when neither are pressed and the "no keyboard" mode has only one switch pressed.

The final switch (SW_2) is mapped to headphone detection.

If we are going to have a KEY_LID switch, we should probably decide now whether the switch is pressed (1) when the lid is either open or shut otherwise things are going to get confused.

I've often wondered whether the input system could/should be used to pass more generic events. I know my handheld has lots of other switch like events such as MMC/SD card insertion, CF card insertion, a battery lock switch, AC power insertion switch and USB cable detection "switch". Most of these are currently acted on by the kernel so userspace doesn't need to see them but some like the USB cable detection would be useful. It could then load the user's chosen USB gadget kernel module for example. In that case its a user choice as there is no "right" gadget module to autoload. I'm not sure if it would be right for these to go via the input system and last time I looked, udev wasn't able to handle generic events like this.

In an ideal world, given the system nature of the events, perhaps they'd be better suited to a more generalised or specialised piece of code based on the input system. A more general events system would mean we could have:

EVENT_LID_SHUT
EVENT_LID_OPEN
EVENT_LID_OPEN_NOKEYBOARD

or similar which would avoid the issues associated with a single SW_LID switch. I suspect there are no easy answers though.

Whilst sort of on the subject (AC power switches and AC power events) I'd like to see some standard way of exporting power/battery information to userspace. Currently, the ARM handhelds use kernel emulation of an APM bios and export the battery info as part of that. Making ARM emulate ACPI interfaces doesn't appeal. The answer could be a battery sysfs class and the above system events interface but I'm open to other suggestions.

Cheers,

Richard

maintainer
#6 lore raw
Quoted 13 lines · Richard Purdie
Quoted 7 lines · Dmitry Torokhov

Yes, I still need to apply it.

Matthew, I would recommend not adding KEY_LID but using one of the switch codes (SW_0?) for the lid.

Richard, on your handhelds what switch would be most similar to notebook's lid? Should we alias one of the switches to SW_LID?

This gets tricky as the handhelds have two "lid" switches. Pictures of how it can fold are at http://www.dynamism.com/sl-c3000/main.shtml . To summarise, it can be in three positions:

Quoted 9 lines

* Screen folded facing the keys (shut like a laptop)
* Screen open above the keyboard (like an open laptop)
* Screen folded over the keys but with the screen visible (becomes a tablet like handheld with no keyboard)

Shut is when both switches (SW_0 and SW_1) are pressed, open is when neither are pressed and the "no keyboard" mode has only one switch pressed.

Ugh... I wonder if we could change SW_0 to report "comletely shut"/"open" and SW_1 to report orientation. Then we could alias SW_0 <-> SW_LID, SW_1 <-> SW_TABLET_MODE.

Quoted 2 lines

The final switch (SW_2) is mapped to headphone detection.

We need to start naming switches so drivers will use same constants for similar switches.

Quoted 4 lines

If we are going to have a KEY_LID switch, we should probably decide now whether the switch is pressed (1) when the lid is either open or shut otherwise things are going to get confused.

SW_LID please.

Quoted 12 lines

I've often wondered whether the input system could/should be used to pass more generic events. I know my handheld has lots of other switch like events such as MMC/SD card insertion, CF card insertion, a battery lock switch, AC power insertion switch and USB cable detection "switch". Most of these are currently acted on by the kernel so userspace doesn't need to see them but some like the USB cable detection would be useful. It could then load the user's chosen USB gadget kernel module for example. In that case its a user choice as there is no "right" gadget module to autoload. I'm not sure if it would be right for these to go via the input system and last time I looked, udev wasn't able to handle generic events like this.

I don't think these belong to input system.

Quoted 11 lines

In an ideal world, given the system nature of the events, perhaps they'd be better suited to a more generalised or specialised piece of code based on the input system. A more general events system would mean we could have:

EVENT_LID_SHUT
EVENT_LID_OPEN
EVENT_LID_OPEN_NOKEYBOARD

or similar which would avoid the issues associated with a single SW_LID switch. I suspect there are no easy answers though.

Bring dbus into the kernel ;) No, I think kernel should not have "cover everythingunder the sun" interface but rather set of specialized ones.

Quoted 9 lines

Whilst sort of on the subject (AC power switches and AC power events) I'd like to see some standard way of exporting power/battery information to userspace. Currently, the ARM handhelds use kernel emulation of an APM bios and export the battery info as part of that. Making ARM emulate ACPI interfaces doesn't appeal. The answer could be a battery sysfs class and the above system events interface but I'm open to other suggestions.

Having generic battery interface would be nice.

--
Dmitry

#7 lore raw
Quoted 10 lines · Dmitry Torokhov
Quoted 9 lines · Richard Purdie

This gets tricky as the handhelds have two "lid" switches. Pictures of how it can fold are at http://www.dynamism.com/sl-c3000/main.shtml . To summarise, it can be in three positions:

* Screen folded facing the keys (shut like a laptop)
* Screen open above the keyboard (like an open laptop)
* Screen folded over the keys but with the screen visible (becomes a tablet like handheld with no keyboard)

Quoted 7 lines
Quoted 3 lines

Shut is when both switches (SW_0 and SW_1) are pressed, open is when neither are pressed and the "no keyboard" mode has only one switch pressed.

Ugh... I wonder if we could change SW_0 to report "comletely shut"/"open" and SW_1 to report orientation. Then we could alias SW_0 <-> SW_LID, SW_1 <-> SW_TABLET_MODE.

I've checked which switch is which and SW_0 is equivalent to SW_LID. We can just rename them and retain compatibility if:

SW_0/SW_LID is pressed to indicate shut
SW_1/SW_TABLET_MODE is pressed to indicate tablet mode SW_2/SW_HEADPHONE_DETECT is pressed to indicate headphone jack insertion

I'm happy to submit a patch to make that change (and remove the as yet unused SW_[3-7] if you like?

I realise there are arguments for having the headphone jack control monitoring in the sound driver but this was the nicest way I've found of passing the event to user space to deal with. I didn't want to deal with it in the sound driver as in this case there is no right answer as to what the sound driver should do (we can't detect if it was headphones, a speaker + mic headset etc. or just a mic that was inserted yet we can support all of them).

Quoted 5 lines
Quoted 3 lines

If we are going to have a KEY_LID switch, we should probably decide now whether the switch is pressed (1) when the lid is either open or shut otherwise things are going to get confused.

SW_LID please.

Sorry, I did mean SW_LID.

Quoted 13 lines
Quoted 12 lines

I've often wondered whether the input system could/should be used to pass more generic events. I know my handheld has lots of other switch like events such as MMC/SD card insertion, CF card insertion, a battery lock switch, AC power insertion switch and USB cable detection "switch". Most of these are currently acted on by the kernel so userspace doesn't need to see them but some like the USB cable detection would be useful. It could then load the user's chosen USB gadget kernel module for example. In that case its a user choice as there is no "right" gadget module to autoload. I'm not sure if it would be right for these to go via the input system and last time I looked, udev wasn't able to handle generic events like this.

Quoted 1 line

I don't think these belong to input system.

Yet they're arguably switches and have all the right semantics to be reported as such. I agree they shouldn't be in the input system but where should they be? Currently we don't need most of them but some would be useful.

Quoted 10 lines
Quoted 8 lines

Whilst sort of on the subject (AC power switches and AC power events) I'd like to see some standard way of exporting power/battery information to userspace. Currently, the ARM handhelds use kernel emulation of an APM bios and export the battery info as part of that. Making ARM emulate ACPI interfaces doesn't appeal. The answer could be a battery sysfs class and the above system events interface but I'm open to other suggestions.

Having generic battery interface would be nice.

It gets tricky. AC presence isn't a property of a battery for example and is in fact more like a switch (my handheld has a mechanical switch to detect when its plugged in) ;). The battery class would export some information but not all of it and I don't know where the leftover information should go. If I knew that, I'd write the class.

Cheers,

Richard

#8 lore raw

Hi!

Quoted 14 lines
Quoted 3 lines

Ugh... I wonder if we could change SW_0 to report "comletely shut"/"open" and SW_1 to report orientation. Then we could alias SW_0 <-> SW_LID, SW_1 <-> SW_TABLET_MODE.

I've checked which switch is which and SW_0 is equivalent to SW_LID. We can just rename them and retain compatibility if:

SW_0/SW_LID is pressed to indicate shut
SW_1/SW_TABLET_MODE is pressed to indicate tablet mode SW_2/SW_HEADPHONE_DETECT is pressed to indicate headphone jack insertion

I'm happy to submit a patch to make that change (and remove the as yet unused SW_[3-7] if you like?

Quoted 7 lines

I realise there are arguments for having the headphone jack control monitoring in the sound driver but this was the nicest way I've found of passing the event to user space to deal with. I didn't want to deal with it in the sound driver as in this case there is no right answer as to what the sound driver should do (we can't detect if it was headphones, a speaker + mic headset etc. or just a mic that was inserted yet we can support all of them).

Well, it "headset inserted" is a kind of user button...

What is on the jack, BTW? Left headphone, right headphone, mic in? Can all of them be used independently?

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

Whilst sort of on the subject (AC power switches and AC power events) I'd like to see some standard way of exporting power/battery information to userspace. Currently, the ARM handhelds use kernel emulation of an APM bios and export the battery info as part of that. Making ARM emulate ACPI interfaces doesn't appeal. The answer could be a battery sysfs class and the above system events interface but I'm open to other suggestions.

Quoted 5 lines
Quoted 1 line

Having generic battery interface would be nice.

It gets tricky. AC presence isn't a property of a battery for example and is in fact more like a switch (my handheld has a mechanical switch

Oops, really? Mechanical switch to sense ac in? (What happens when you plug in charger but that is not plugged to AC?)

I think that AC presence should be handled independently from battery. There can be >1 battery in the system.

(Another interesting question is: is AC status 0/1 or is it number of milivolts?)

Quoted 3 lines

to detect when its plugged in) ;). The battery class would export some information but not all of it and I don't know where the leftover information should go. If I knew that, I'd write the class.

Leftover information?

I think we should create directory in sysfs, and populate it according to battery's capabilities.

Zaurus' battery would have voltage and maybe percent fields.

ACPI battery would have all the usual fields.

Another important question is a way for user applications to avoid polling... but I guess that should be solveable by enabling select on one of those files.
Pavel

Signature · 1 line
Thanks for all the (sleeping) penguins.
#9 lore raw
Quoted 9 lines · Pavel Machek

What is on the jack, BTW? Left headphone, right headphone, mic in? Can all of them be used independently?

Quoted 3 lines

It gets tricky. AC presence isn't a property of a battery for example and is in fact more like a switch (my handheld has a mechanical switch

Oops, really? Mechanical switch to sense ac in? (What happens when you plug in charger but that is not plugged to AC?)

Most of the Zaurus devices can measure the AC voltage and make sanity checks on it in the SharpSL Battery/PM code.

Quoted 2 lines

I think that AC presence should be handled independently from battery. There can be >1 battery in the system.

Agreed. This is some of the leftover information I'm referring to below.

Quoted 2 lines

(Another interesting question is: is AC status 0/1 or is it number of milivolts?)

I'd say millivolts except for the problem of what you do on systems that don't support voltage readings. Use a very high value I guess. For a lot of devices millivolts also means in kernel conversion tables. Do such things belong in kernel or user space?

Quoted 5 lines
Quoted 3 lines

to detect when its plugged in) ;). The battery class would export some information but not all of it and I don't know where the leftover information should go. If I knew that, I'd write the class.

Leftover information?

Where to put AC status and AC voltage readings amongst other things. Another sysfs class? Also, how do you control suspend/resume notifications to userspace if not using APM/ACPI?

Quoted 10 lines

I think we should create directory in sysfs, and populate it according to battery's capabilities.

Zaurus' battery would have voltage and maybe percent fields.

ACPI battery would have all the usual fields.

Another important question is a way for user applications to avoid polling... but I guess that should be solveable by enabling select on one of those files.

Agreed, this is something that would be needed.

Richard

#10 lore raw

Hi!

Quoted 7 lines
Quoted 2 lines

(Another interesting question is: is AC status 0/1 or is it number of milivolts?)

I'd say millivolts except for the problem of what you do on systems that don't support voltage readings. Use a very high value I guess. For a lot of devices millivolts also means in kernel conversion tables. Do such things belong in kernel or user space?

We should probably return 0/-1 in such case (-1 = on, but voltage unknown).

Quoted 9 lines
Quoted 5 lines
Quoted 3 lines

to detect when its plugged in) ;). The battery class would export some information but not all of it and I don't know where the leftover information should go. If I knew that, I'd write the class.

Leftover information?

Where to put AC status and AC voltage readings amongst other things. Another sysfs class? Also, how do you control suspend/resume notifications to userspace if not using APM/ACPI?

I'd say that AC status should go to separate class, I'm afraid. Potentially AC has more parameters (like current, current frequency, difference between phase of current and voltage, ...?) and UPSs even measure that.

Does userspace need to be notified of suspend/resume? [I keep insisting that they do not, and I'm probably wrong, but... :-)]

Pavel

Signature · 1 line
Thanks for all the (sleeping) penguins.
#11 lore raw
In reply to Richard Purdie

Hi!

Quoted 7 lines

Whilst sort of on the subject (AC power switches and AC power events) I'd like to see some standard way of exporting power/battery information to userspace. Currently, the ARM handhelds use kernel emulation of an APM bios and export the battery info as part of that. Making ARM emulate ACPI interfaces doesn't appeal. The answer could be a battery sysfs class and the above system events interface but I'm open to other suggestions.

Battery sysfs class would be _very_ welcome. Both existing interfaces (APM and ACPI) are crap :-(.
Pavel

Signature · 1 line
Thanks for all the (sleeping) penguins.
#12 lore raw
In reply to Matthew Garrett

Hi!

Quoted 4 lines

Sleep and power buttons are logically part of the keyboard, and it makes for them to be exposed via an input device rather than an odd file in /proc. This patch adds a keycode for lid switches (are we running out of keycodes?) and allows the button driver to register an input device.

Yes, please. This is actually long overdue.

Pavel

Signature · 1 line
Thanks, Sharp!