Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Mon, Mar 28, 2011 at 02:14:33PM +0000, Corentin Chary wrote:
> But since Dmitry and Chris seems to prefer the autorepeat approach
> with a single key code, if it works then it's ok, we just need to set
> REP_DELAY, report press and release events, and drop 0xea events.

New patches follow. I didn't set REP_DELAY (or REP_PERIOD) because the
input core interprets this to mean that the driver will handle the
autorepeat itself. There's always EVIOCSREP to change the delay and
repeat period from userspace.

Changes since v2:

 - Fixed incorrect types of key_filter() arguments
 - Use a single key code, KEY_CONFIG, for this button
 - Simplify filtering to only emit events for press and release
 - Added a patch to enable autorepeat for hotkey input device

[PATCH v3 1/3] asus-wmi: Add callback for hotkey filtering

[Seth Forshee]

This is required for the T101MT home key, which behaves differently
than other hotkeys.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/platform/x86/asus-wmi.c |   12 +++++++++++-
 drivers/platform/x86/asus-wmi.h |    6 ++++++
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 5b779a9..b14d992 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1058,6 +1058,8 @@ static void asus_wmi_notify(u32 value, void *context)
 	acpi_status status;
 	int code;
 	int orig_code;
+	unsigned int key_value = 1;
+	bool autorelease = 1;
 
 	status = wmi_get_event_data(value, &response);
 	if (status != AE_OK) {
@@ -1073,6 +1075,13 @@ static void asus_wmi_notify(u32 value, void *context)
 	code = obj->integer.value;
 	orig_code = code;
 
+	if (asus->driver->key_filter) {
+		asus->driver->key_filter(asus->driver, &code, &key_value,
+					 &autorelease);
+		if (code == ASUS_WMI_KEY_IGNORE)
+			goto exit;
+	}
+
 	if (code >= NOTIFY_BRNUP_MIN && code <= NOTIFY_BRNUP_MAX)
 		code = NOTIFY_BRNUP_MIN;
 	else if (code >= NOTIFY_BRNDOWN_MIN &&
@@ -1082,7 +1091,8 @@ static void asus_wmi_notify(u32 value, void *context)
 	if (code == NOTIFY_BRNUP_MIN || code == NOTIFY_BRNDOWN_MIN) {
 		if (!acpi_video_backlight_support())
 			asus_wmi_backlight_notify(asus, orig_code);
-	} else if (!sparse_keymap_report_event(asus->inputdev, code, 1, true))
+	} else if (!sparse_keymap_report_event(asus->inputdev, code,
+					       key_value, autorelease))
 		pr_info("Unknown key %x pressed\n", code);
 
 exit:
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index c044522..4da6103 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -29,6 +29,8 @@
 
 #include <linux/platform_device.h>
 
+#define ASUS_WMI_KEY_IGNORE (-1)
+
 struct module;
 struct key_entry;
 struct asus_wmi;
@@ -44,6 +46,10 @@ struct asus_wmi_driver {
 	const struct key_entry	*keymap;
 	const char		*input_name;
 	const char		*input_phys;
+	/* Returns new code, value, and autorelease values in arguments.
+	 * Return ASUS_WMI_KEY_IGNORE in code if event should be ignored. */
+	void (*key_filter) (struct asus_wmi_driver *driver, int *code,
+			    unsigned int *value, bool *autorelease);
 
 	int (*probe) (struct platform_device *device);
 	void (*quirks) (struct asus_wmi_driver *driver);
-- 
1.7.4.1


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and publish 
your website. http://p.sf.net/sfu/ms-webmatrix-sf

[PATCH v3 2/3] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

This key is different than other hotkeys, having seperate scan
codes for press, release, and hold, so it requires some special
filtering. Press and release events are passed on, and hold events
are ignored since sparse-keymap does not support hardware
autorepeat.

Note that "Home" in the context of this button doesn't mean the
same thing as the usual Home key, and it really isn't clear at
all what is meant by "Home". The manufacurer's description of the
button indicates that it should launch some sort of touch screen
settings interface on short press and apply a desktop rotation on
long press.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/platform/x86/eeepc-wmi.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/eeepc-wmi.c b/drivers/platform/x86/eeepc-wmi.c
index 0ddc434..8f3112d5 100644
--- a/drivers/platform/x86/eeepc-wmi.c
+++ b/drivers/platform/x86/eeepc-wmi.c
@@ -56,6 +56,11 @@ MODULE_PARM_DESC(hotplug_wireless,
 		 "If your laptop needs that, please report to "
 		 "acpi4asus-user-5NWGOfrQmneRv+LV9MX5uti2KpX7p8Fi@public.gmane.org");
 
+/* Values for T101MT "Home" key */
+#define HOME_PRESS	0xe4
+#define HOME_HOLD	0xea
+#define HOME_RELEASE	0xe5
+
 static const struct key_entry eeepc_wmi_keymap[] = {
 	/* Sleep already handled via generic ACPI code */
 	{ KE_KEY, 0x30, { KEY_VOLUMEUP } },
@@ -70,6 +75,7 @@ static const struct key_entry eeepc_wmi_keymap[] = {
 	{ KE_KEY, 0xcc, { KEY_SWITCHVIDEOMODE } },
 	{ KE_KEY, 0xe0, { KEY_PROG1 } }, /* Task Manager */
 	{ KE_KEY, 0xe1, { KEY_F14 } }, /* Change Resolution */
+	{ KE_KEY, HOME_PRESS, { KEY_CONFIG } },
 	{ KE_KEY, 0xe9, { KEY_BRIGHTNESS_ZERO } },
 	{ KE_KEY, 0xeb, { KEY_CAMERA_ZOOMOUT } },
 	{ KE_KEY, 0xec, { KEY_CAMERA_UP } },
@@ -79,6 +85,25 @@ static const struct key_entry eeepc_wmi_keymap[] = {
 	{ KE_END, 0},
 };
 
+static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
+				 unsigned int *value, bool *autorelease)
+{
+	switch (*code) {
+	case HOME_PRESS:
+		*value = 1;
+		*autorelease = 0;
+		break;
+	case HOME_HOLD:
+		*code = ASUS_WMI_KEY_IGNORE;
+		break;
+	case HOME_RELEASE:
+		*code = HOME_PRESS;
+		*value = 0;
+		*autorelease = 0;
+		break;
+	}
+}
+
 static acpi_status eeepc_wmi_parse_device(acpi_handle handle, u32 level,
 						 void *context, void **retval)
 {
@@ -149,6 +174,7 @@ static struct asus_wmi_driver asus_wmi_driver = {
 	.keymap = eeepc_wmi_keymap,
 	.input_name = "Eee PC WMI hotkeys",
 	.input_phys = EEEPC_WMI_FILE "/input0",
+	.key_filter = eeepc_wmi_key_filter,
 	.probe = eeepc_wmi_probe,
 	.quirks = eeepc_wmi_quirks,
 };
-- 
1.7.4.1


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and publish 
your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v3 2/3] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Corentin Chary]

On Mon, Mar 28, 2011 at 6:33 PM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> This key is different than other hotkeys, having seperate scan
> codes for press, release, and hold, so it requires some special
> filtering. Press and release events are passed on, and hold events
> are ignored since sparse-keymap does not support hardware
> autorepeat.
>
> Note that "Home" in the context of this button doesn't mean the
> same thing as the usual Home key, and it really isn't clear at
> all what is meant by "Home". The manufacurer's description of the
> button indicates that it should launch some sort of touch screen
> settings interface on short press and apply a desktop rotation on
> long press.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  drivers/platform/x86/eeepc-wmi.c |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/platform/x86/eeepc-wmi.c b/drivers/platform/x86/eeepc-wmi.c
> index 0ddc434..8f3112d5 100644
> --- a/drivers/platform/x86/eeepc-wmi.c
> +++ b/drivers/platform/x86/eeepc-wmi.c
> @@ -56,6 +56,11 @@ MODULE_PARM_DESC(hotplug_wireless,
>                 "If your laptop needs that, please report to "
>                 "acpi4asus-user@lists.sourceforge.net.");
>
> +/* Values for T101MT "Home" key */
> +#define HOME_PRESS     0xe4
> +#define HOME_HOLD      0xea
> +#define HOME_RELEASE   0xe5
> +
>  static const struct key_entry eeepc_wmi_keymap[] = {
>        /* Sleep already handled via generic ACPI code */
>        { KE_KEY, 0x30, { KEY_VOLUMEUP } },
> @@ -70,6 +75,7 @@ static const struct key_entry eeepc_wmi_keymap[] = {
>        { KE_KEY, 0xcc, { KEY_SWITCHVIDEOMODE } },
>        { KE_KEY, 0xe0, { KEY_PROG1 } }, /* Task Manager */
>        { KE_KEY, 0xe1, { KEY_F14 } }, /* Change Resolution */
> +       { KE_KEY, HOME_PRESS, { KEY_CONFIG } },

What X11 key is mapped to KEY_CONFIG ?

>        { KE_KEY, 0xe9, { KEY_BRIGHTNESS_ZERO } },
>        { KE_KEY, 0xeb, { KEY_CAMERA_ZOOMOUT } },
>        { KE_KEY, 0xec, { KEY_CAMERA_UP } },
> @@ -79,6 +85,25 @@ static const struct key_entry eeepc_wmi_keymap[] = {
>        { KE_END, 0},
>  };
>
> +static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
> +                                unsigned int *value, bool *autorelease)
> +{
> +       switch (*code) {
> +       case HOME_PRESS:
> +               *value = 1;
> +               *autorelease = 0;
> +               break;
> +       case HOME_HOLD:
> +               *code = ASUS_WMI_KEY_IGNORE;
> +               break;
> +       case HOME_RELEASE:
> +               *code = HOME_PRESS;
> +               *value = 0;
> +               *autorelease = 0;
> +               break;
> +       }
> +}
> +
>  static acpi_status eeepc_wmi_parse_device(acpi_handle handle, u32 level,
>                                                 void *context, void **retval)
>  {
> @@ -149,6 +174,7 @@ static struct asus_wmi_driver asus_wmi_driver = {
>        .keymap = eeepc_wmi_keymap,
>        .input_name = "Eee PC WMI hotkeys",
>        .input_phys = EEEPC_WMI_FILE "/input0",
> +       .key_filter = eeepc_wmi_key_filter,
>        .probe = eeepc_wmi_probe,
>        .quirks = eeepc_wmi_quirks,
>  };
> --
> 1.7.4.1
>
>

I think this version is ok,

Matthew, do you think this should be delayed for 2.6.40 (merge window
will close soon, so now it's probably time for fix only) ?

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH v3 2/3] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Tue, Mar 29, 2011 at 12:29:11PM +0000, Corentin Chary wrote:
> > +       { KE_KEY, HOME_PRESS, { KEY_CONFIG } },
> 
> What X11 key is mapped to KEY_CONFIG ?

XF86Tools.

I'm still not 100% confident in that choice though. It's one of the two
suggested by Dmitry. The other (KEY_DASHBOARD) only seems to be used for
the key on Mac keyboards that displays the OS X dashboard, which is some
kind of widget overlay on top of the desktop.

The other option I considered was KEY_CYCLEWINDOWS, which maps to
XF86RotateWindows. No one seemed to know for sure what its intended
purpose is, but in some cases at least it appears to be used to rotate
the desktop.

Seth

Re: [PATCH v3 2/3] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Matthew Garrett]

On Tue, Mar 29, 2011 at 12:29:11PM +0000, Corentin Chary wrote:

> I think this version is ok,
> 
> Matthew, do you think this should be delayed for 2.6.40 (merge window
> will close soon, so now it's probably time for fix only) ?

Yeah, I'd lean towards this being .40 material.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3 2/3] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Corentin Chary]

>> I think this version is ok,
>>
>> Matthew, do you think this should be delayed for 2.6.40 (merge window
>> will close soon, so now it's probably time for fix only) ?
>
> Yeah, I'd lean towards this being .40 material.

Seth, I've added your (rebased) patch to my tree, I'll re-send them
with others for .40.
Thanks !


-- 
Corentin Chary
http://xf.iksaif.net

[PATCH v3 3/3] asus-wmi: Enable autorepeat for hotkey input device

[Seth Forshee]

The T101MT Home/Express Gate key autorepeats in hardware, but
sparse-keymap does not support hardware autorepeat. Enable the
input core's software autorepeat to emulate the hardware behavior.
Normal hotkeys are autoreleased, so the behavior of these keys
will not be affected.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/platform/x86/asus-wmi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index b14d992..0cb0fa7 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -205,6 +205,7 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
 	asus->inputdev->phys = asus->driver->input_name;
 	asus->inputdev->id.bustype = BUS_HOST;
 	asus->inputdev->dev.parent = &asus->platform_device->dev;
+	__set_bit(EV_REP, asus->inputdev->evbit);
 
 	err = sparse_keymap_setup(asus->inputdev, asus->driver->keymap, NULL);
 	if (err)
-- 
1.7.4.1


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and publish 
your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Dmitry Torokhov]

On Mon, Mar 28, 2011 at 01:33:41PM -0500, Seth Forshee wrote:
> On Mon, Mar 28, 2011 at 02:14:33PM +0000, Corentin Chary wrote:
> > But since Dmitry and Chris seems to prefer the autorepeat approach
> > with a single key code, if it works then it's ok, we just need to set
> > REP_DELAY, report press and release events, and drop 0xea events.
> 
> New patches follow. I didn't set REP_DELAY (or REP_PERIOD) because the
> input core interprets this to mean that the driver will handle the
> autorepeat itself. There's always EVIOCSREP to change the delay and
> repeat period from userspace.

Just an FYI: it is perfectly fine to set custom REP_DELAY and REP_PERIOD
after registering an input device.

Thanks.

-- 
Dmitry

------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar

Re: [PATCH] eeepc-wmi: Add support for T101MT "Express Gate" key

[Seth Forshee]

On Thu, Mar 24, 2011 at 09:00:14AM -0500, Chris Bagwell wrote:
> On Thu, Mar 24, 2011 at 8:40 AM, Corentin Chary
> <corentin.chary@gmail.com> wrote:
> >> I would suggest sending BTN_HOME upon button release 0xe5 (not press
> >> 0xe4 and do the autorelease because of it) and BTN_PROG2 on
> >> press-and-hold 0xea.
> >>
> >> If you press-and-hold, does it still send the 0xe5 upon release?  That
> >> could complicate things so that both button events do not get sent.
> >>
> >> I'm not sure what BTN_HOME is commonly used for.  This button does
> >> sound like a HOME button in iPad sense but not in web browser sense.
> >
> > Except the fact that I don't find BTN_HOME in inpout.h, that's seems a
> > better idea, forget what I've said earlier.
> 
> Ha... Sorry, I had buttons on my brain.  I meant KEY_HOME as you suggested.

Really? Isn't KEY_HOME the home key on a standard keyboard?

> > 0xe4 -> KEY_IGNORE
> > 0xe5 -> KEY_HOME
> > 0xea -> KEY_PROG2, (or BTN_TASK to copy android/iOS ?)
> 
> From a quick google, I couldn't finding any specific meaning of
> BTN_TASK but it looks like xf86-input-evdev turns it into a button
> press so we may not want that one.
> 
> I've no idea of best values but those are pretty close.

I'm still not sure what key codes are best either. I've implemented this
approach, and I went with BTN_TASK and KEY_PROG2, but I'm getting the
idea that BTN_TASK was intended for a mouse button so I'm not really
sure if it's a good choice or not. I'll follow up with the patches.

Thanks for the feedback so far.

Seth

[PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

This key is different than other hotkeys, having seperate scan
codes for press, release, and hold, so it requires some special
handling.

Note that "Home" in the context of this button doesn't mean the
same thing as the usual Home key, and it really isn't clear at
all what is meant by "Home". The manufacurer's description of the
button indicates that it should launch some sort of touch screen
settings interface on short press and apply a desktop rotation on
long press.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/platform/x86/eeepc-wmi.c |   87 +++++++++++++++++++++++++++++++++-----
 1 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/eeepc-wmi.c b/drivers/platform/x86/eeepc-wmi.c
index 0ddc434..f9b0ee4 100644
--- a/drivers/platform/x86/eeepc-wmi.c
+++ b/drivers/platform/x86/eeepc-wmi.c
@@ -56,6 +56,19 @@ MODULE_PARM_DESC(hotplug_wireless,
 		 "If your laptop needs that, please report to "
 		 "acpi4asus-user-5NWGOfrQmneRv+LV9MX5uti2KpX7p8Fi@public.gmane.org");
 
+struct eeepc_wmi_driver {
+	struct asus_wmi_driver	asus_wmi_driver;
+	int			home_key_state;
+};
+
+#define to_eeepc_wmi_driver(asus_drv) \
+	container_of((asus_drv), struct eeepc_wmi_driver, asus_wmi_driver)
+
+/* Values for T101MT "Home" key */
+#define HOME_PRESS	0xe4
+#define HOME_HOLD	0xea
+#define HOME_RELEASE	0xe5
+
 static const struct key_entry eeepc_wmi_keymap[] = {
 	/* Sleep already handled via generic ACPI code */
 	{ KE_KEY, 0x30, { KEY_VOLUMEUP } },
@@ -70,7 +83,9 @@ static const struct key_entry eeepc_wmi_keymap[] = {
 	{ KE_KEY, 0xcc, { KEY_SWITCHVIDEOMODE } },
 	{ KE_KEY, 0xe0, { KEY_PROG1 } }, /* Task Manager */
 	{ KE_KEY, 0xe1, { KEY_F14 } }, /* Change Resolution */
+	{ KE_KEY, HOME_PRESS, { BTN_TASK } },
 	{ KE_KEY, 0xe9, { KEY_BRIGHTNESS_ZERO } },
+	{ KE_KEY, HOME_HOLD, { KEY_PROG2 } },
 	{ KE_KEY, 0xeb, { KEY_CAMERA_ZOOMOUT } },
 	{ KE_KEY, 0xec, { KEY_CAMERA_UP } },
 	{ KE_KEY, 0xed, { KEY_CAMERA_DOWN } },
@@ -79,6 +94,52 @@ static const struct key_entry eeepc_wmi_keymap[] = {
 	{ KE_END, 0},
 };
 
+static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
+				 int *value, int *autorelease)
+{
+	struct eeepc_wmi_driver *eeepc = to_eeepc_wmi_driver(asus_wmi);
+	int is_press;
+
+	/*
+	 * The following behavior is used for T101MT "Home" key:
+	 * 
+	 *   On press:   No event set
+	 *   On hold:    KEY_PROG2 press sent once w/o autorelease
+	 *   On release: If key was held, KEY_PROG2 release sent.
+	 *               Otherwise KEY_HOME press sent w/ autorelease.
+	 *
+	 * The simple state machine below implements this behavior.
+	 */
+	switch (*code) {
+	case HOME_PRESS:
+		eeepc->home_key_state = HOME_PRESS;
+		*code = ASUS_WMI_KEY_IGNORE;
+		break;
+	case HOME_HOLD:
+		if (eeepc->home_key_state == HOME_HOLD) {
+			*code = ASUS_WMI_KEY_IGNORE;
+		} else {
+			eeepc->home_key_state = HOME_HOLD;
+			*value = 1;
+			*autorelease = 0;
+		}
+		break;
+	case HOME_RELEASE:
+		if (eeepc->home_key_state == HOME_RELEASE) {
+			dev_warn(&asus_wmi->platform_device->dev,
+				 "Unexpected home key release event\n");
+			*code = ASUS_WMI_KEY_IGNORE;
+		} else {
+			*code = eeepc->home_key_state;
+			eeepc->home_key_state = HOME_RELEASE;
+			is_press = (*code == HOME_PRESS);
+			*value = is_press;
+			*autorelease = is_press;
+		}
+		break;
+	}
+}
+
 static acpi_status eeepc_wmi_parse_device(acpi_handle handle, u32 level,
 						 void *context, void **retval)
 {
@@ -142,26 +203,30 @@ static void eeepc_wmi_quirks(struct asus_wmi_driver *driver)
 	eeepc_dmi_check(driver);
 }
 
-static struct asus_wmi_driver asus_wmi_driver = {
-	.name = EEEPC_WMI_FILE,
-	.owner = THIS_MODULE,
-	.event_guid = EEEPC_WMI_EVENT_GUID,
-	.keymap = eeepc_wmi_keymap,
-	.input_name = "Eee PC WMI hotkeys",
-	.input_phys = EEEPC_WMI_FILE "/input0",
-	.probe = eeepc_wmi_probe,
-	.quirks = eeepc_wmi_quirks,
+static struct eeepc_wmi_driver eeepc_wmi_driver = {
+	.asus_wmi_driver = {
+		.name = EEEPC_WMI_FILE,
+		.owner = THIS_MODULE,
+		.event_guid = EEEPC_WMI_EVENT_GUID,
+		.keymap = eeepc_wmi_keymap,
+		.input_name = "Eee PC WMI hotkeys",
+		.input_phys = EEEPC_WMI_FILE "/input0",
+		.key_filter = eeepc_wmi_key_filter,
+		.probe = eeepc_wmi_probe,
+		.quirks = eeepc_wmi_quirks,
+	},
+	.home_key_state = HOME_RELEASE,
 };
 
 
 static int __init eeepc_wmi_init(void)
 {
-	return asus_wmi_register_driver(&asus_wmi_driver);
+	return asus_wmi_register_driver(&eeepc_wmi_driver.asus_wmi_driver);
 }
 
 static void __exit eeepc_wmi_exit(void)
 {
-	asus_wmi_unregister_driver(&asus_wmi_driver);
+	asus_wmi_unregister_driver(&eeepc_wmi_driver.asus_wmi_driver);
 }
 
 module_init(eeepc_wmi_init);
-- 
1.7.4.1


------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Thu, Mar 24, 2011 at 03:03:55PM -0500, Seth Forshee wrote:
> @@ -79,6 +94,52 @@ static const struct key_entry eeepc_wmi_keymap[] = {
>  	{ KE_END, 0},
>  };
>  
> +static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
> +				 int *value, int *autorelease)
> +{
> +	struct eeepc_wmi_driver *eeepc = to_eeepc_wmi_driver(asus_wmi);
> +	int is_press;
> +
> +	/*
> +	 * The following behavior is used for T101MT "Home" key:
> +	 * 
> +	 *   On press:   No event set
> +	 *   On hold:    KEY_PROG2 press sent once w/o autorelease
> +	 *   On release: If key was held, KEY_PROG2 release sent.
> +	 *               Otherwise KEY_HOME press sent w/ autorelease.

Doh, just noticed that this comment is incorrect. I'll fix it, but I'll
wait for comments before sending new patches.

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Corentin Chary]

> +static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
> +                                int *value, int *autorelease)
> +{
> +       struct eeepc_wmi_driver *eeepc = to_eeepc_wmi_driver(asus_wmi);
> +       int is_press;
> +
> +       /*
> +        * The following behavior is used for T101MT "Home" key:
> +        *
> +        *   On press:   No event set
> +        *   On hold:    KEY_PROG2 press sent once w/o autorelease
> +        *   On release: If key was held, KEY_PROG2 release sent.
> +        *               Otherwise KEY_HOME press sent w/ autorelease.
> +        *
> +        * The simple state machine below implements this behavior.
> +        */
> +       switch (*code) {
> +       case HOME_PRESS:
> +               eeepc->home_key_state = HOME_PRESS;
> +               *code = ASUS_WMI_KEY_IGNORE;
> +               break;
> +       case HOME_HOLD:
> +               if (eeepc->home_key_state == HOME_HOLD) {
> +                       *code = ASUS_WMI_KEY_IGNORE;
> +               } else {
> +                       eeepc->home_key_state = HOME_HOLD;
> +                       *value = 1;
> +                       *autorelease = 0;
> +               }
> +               break;
> +       case HOME_RELEASE:
> +               if (eeepc->home_key_state == HOME_RELEASE) {
> +                       dev_warn(&asus_wmi->platform_device->dev,
> +                                "Unexpected home key release event\n");
> +                       *code = ASUS_WMI_KEY_IGNORE;
> +               } else {
> +                       *code = eeepc->home_key_state;
> +                       eeepc->home_key_state = HOME_RELEASE;
> +                       is_press = (*code == HOME_PRESS);
> +                       *value = is_press;
> +                       *autorelease = is_press;
> +               }
> +               break;
> +       }
> +}
> +

Why not something simpler like this ?

static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int code,
                                                 int *value, int *autorelease)
{
        if (code == 0xe4) {
                *value = 1;
                *autorelease = 0;
        } else if (code == 0xe5) {
                *value = 0;
                *autorelease = 0;
       }
}

with this keymap :

       { KE_KEY, 0xe4, { KEY_HOME } }, /* Home Key Down */
       { KE_KEY, 0xe5, { KEY_HOME } }, /* Home Key Up */
       { KE_KEY, 0xea, { KEY_PROG2 } }, /* Home Key hold more than one second */


This sounds simpler and we don't loose information (key down and key
up both event reported at the right time).
0xe5 is *always* sent after 0xe4 right ?

Also, for the callback, "value" should be an unsigned int, and
"autorelease" a bool.

Thanks,
-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Fri, Mar 25, 2011 at 01:28:30PM +0000, Corentin Chary wrote:
> > +static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
> > +                                int *value, int *autorelease)
> > +{
> > +       struct eeepc_wmi_driver *eeepc = to_eeepc_wmi_driver(asus_wmi);
> > +       int is_press;
> > +
> > +       /*
> > +        * The following behavior is used for T101MT "Home" key:
> > +        *
> > +        *   On press:   No event set
> > +        *   On hold:    KEY_PROG2 press sent once w/o autorelease
> > +        *   On release: If key was held, KEY_PROG2 release sent.
> > +        *               Otherwise KEY_HOME press sent w/ autorelease.
> > +        *
> > +        * The simple state machine below implements this behavior.
> > +        */
> > +       switch (*code) {
> > +       case HOME_PRESS:
> > +               eeepc->home_key_state = HOME_PRESS;
> > +               *code = ASUS_WMI_KEY_IGNORE;
> > +               break;
> > +       case HOME_HOLD:
> > +               if (eeepc->home_key_state == HOME_HOLD) {
> > +                       *code = ASUS_WMI_KEY_IGNORE;
> > +               } else {
> > +                       eeepc->home_key_state = HOME_HOLD;
> > +                       *value = 1;
> > +                       *autorelease = 0;
> > +               }
> > +               break;
> > +       case HOME_RELEASE:
> > +               if (eeepc->home_key_state == HOME_RELEASE) {
> > +                       dev_warn(&asus_wmi->platform_device->dev,
> > +                                "Unexpected home key release event\n");
> > +                       *code = ASUS_WMI_KEY_IGNORE;
> > +               } else {
> > +                       *code = eeepc->home_key_state;
> > +                       eeepc->home_key_state = HOME_RELEASE;
> > +                       is_press = (*code == HOME_PRESS);
> > +                       *value = is_press;
> > +                       *autorelease = is_press;
> > +               }
> > +               break;
> > +       }
> > +}
> > +
> 
> Why not something simpler like this ?
> 
> static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int code,
>                                                  int *value, int *autorelease)
> {
>         if (code == 0xe4) {
>                 *value = 1;
>                 *autorelease = 0;
>         } else if (code == 0xe5) {
>                 *value = 0;
>                 *autorelease = 0;
>        }
> }
> 
> with this keymap :
> 
>        { KE_KEY, 0xe4, { KEY_HOME } }, /* Home Key Down */
>        { KE_KEY, 0xe5, { KEY_HOME } }, /* Home Key Up */
>        { KE_KEY, 0xea, { KEY_PROG2 } }, /* Home Key hold more than one second */
> 
> 
> This sounds simpler and we don't loose information (key down and key
> up both event reported at the right time).
> 0xe5 is *always* sent after 0xe4 right ?

I guess it depends on what key events we want on a press-and-hold.
Remember that you're going to get a scan code sequence like "0xe4 0xea
0xea ... 0xea 0xe5", so with my implementation you get

  KEY_PROG2 press
  KEY_PROG2 release

With yours

  KEY_HOME press
  KEY_PROG2 press
  KEY_PROG2 release
  // KEY_PROG2 press/release repeats every 0.5 secs while button held
  KEY_HOME release

At minimum I'd think we'd want to only send a single PROG2 press/release
for button hold. My thought was that you'd only want to get the code for
0xe4 or 0xea, not both, but I suppose that's debatable.

And back to the question of KEY_HOME -- that's not really what you want,
is it? As in "move cursor to start of line"?

> Also, for the callback, "value" should be an unsigned int, and
> "autorelease" a bool.

Right, silly mistake. Thanks for catching it.


------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar
_______________________________________________
Acpi4asus-user mailing list
Acpi4asus-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/acpi4asus-user

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Corentin Chary]

On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Fri, Mar 25, 2011 at 01:28:30PM +0000, Corentin Chary wrote:
>> > +static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
>> > +                                int *value, int *autorelease)
>> > +{
>> > +       struct eeepc_wmi_driver *eeepc = to_eeepc_wmi_driver(asus_wmi);
>> > +       int is_press;
>> > +
>> > +       /*
>> > +        * The following behavior is used for T101MT "Home" key:
>> > +        *
>> > +        *   On press:   No event set
>> > +        *   On hold:    KEY_PROG2 press sent once w/o autorelease
>> > +        *   On release: If key was held, KEY_PROG2 release sent.
>> > +        *               Otherwise KEY_HOME press sent w/ autorelease.
>> > +        *
>> > +        * The simple state machine below implements this behavior.
>> > +        */
>> > +       switch (*code) {
>> > +       case HOME_PRESS:
>> > +               eeepc->home_key_state = HOME_PRESS;
>> > +               *code = ASUS_WMI_KEY_IGNORE;
>> > +               break;
>> > +       case HOME_HOLD:
>> > +               if (eeepc->home_key_state == HOME_HOLD) {
>> > +                       *code = ASUS_WMI_KEY_IGNORE;
>> > +               } else {
>> > +                       eeepc->home_key_state = HOME_HOLD;
>> > +                       *value = 1;
>> > +                       *autorelease = 0;
>> > +               }
>> > +               break;
>> > +       case HOME_RELEASE:
>> > +               if (eeepc->home_key_state == HOME_RELEASE) {
>> > +                       dev_warn(&asus_wmi->platform_device->dev,
>> > +                                "Unexpected home key release event\n");
>> > +                       *code = ASUS_WMI_KEY_IGNORE;
>> > +               } else {
>> > +                       *code = eeepc->home_key_state;
>> > +                       eeepc->home_key_state = HOME_RELEASE;
>> > +                       is_press = (*code == HOME_PRESS);
>> > +                       *value = is_press;
>> > +                       *autorelease = is_press;
>> > +               }
>> > +               break;
>> > +       }
>> > +}
>> > +
>>
>> Why not something simpler like this ?
>>
>> static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int code,
>>                                                  int *value, int *autorelease)
>> {
>>         if (code == 0xe4) {
>>                 *value = 1;
>>                 *autorelease = 0;
>>         } else if (code == 0xe5) {
>>                 *value = 0;
>>                 *autorelease = 0;
>>        }
>> }
>>
>> with this keymap :
>>
>>        { KE_KEY, 0xe4, { KEY_HOME } }, /* Home Key Down */
>>        { KE_KEY, 0xe5, { KEY_HOME } }, /* Home Key Up */
>>        { KE_KEY, 0xea, { KEY_PROG2 } }, /* Home Key hold more than one second */
>>
>>
>> This sounds simpler and we don't loose information (key down and key
>> up both event reported at the right time).
>> 0xe5 is *always* sent after 0xe4 right ?
>
> I guess it depends on what key events we want on a press-and-hold.
> Remember that you're going to get a scan code sequence like "0xe4 0xea
> 0xea ... 0xea 0xe5", so with my implementation you get
>
>  KEY_PROG2 press
>  KEY_PROG2 release
>
> With yours
>
>  KEY_HOME press
>  KEY_PROG2 press
>  KEY_PROG2 release
>  // KEY_PROG2 press/release repeats every 0.5 secs while button held
>  KEY_HOME release
>
> At minimum I'd think we'd want to only send a single PROG2 press/release
> for button hold. My thought was that you'd only want to get the code for
> 0xe4 or 0xea, not both, but I suppose that's debatable.

If you keep a keyboard key pressed, you want multiple events, not one right ?
I think it's important not to loose informations. If someone keep this
key pressed more than 1.5 second, I think it's good idea to send
multiple KEY_PROG2.

About KEY_HOME press / release, and filtering KEY_HOME after
KEY_PROG2, I'm not sure. So if you really want it, and nobody
complains, I'll be happy to accept your patch.

> And back to the question of KEY_HOME -- that's not really what you want,
> is it? As in "move cursor to start of line"?

Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
What about:
- KEY_CYCLEWINDOWS
- KEY_COMPUTER
- KEY_HOMEPAGE
- KEY_DASHBOARD

I think KEY_HOMEPAGE is the best choice.

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Chris Bagwell]

On Fri, Mar 25, 2011 at 9:05 AM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
>> On Fri, Mar 25, 2011 at 01:28:30PM +0000, Corentin Chary wrote:
>>> > +static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
>>> > +                                int *value, int *autorelease)
>>> > +{
>>> > +       struct eeepc_wmi_driver *eeepc = to_eeepc_wmi_driver(asus_wmi);
>>> > +       int is_press;
>>> > +
>>> > +       /*
>>> > +        * The following behavior is used for T101MT "Home" key:
>>> > +        *
>>> > +        *   On press:   No event set
>>> > +        *   On hold:    KEY_PROG2 press sent once w/o autorelease
>>> > +        *   On release: If key was held, KEY_PROG2 release sent.
>>> > +        *               Otherwise KEY_HOME press sent w/ autorelease.
>>> > +        *
>>> > +        * The simple state machine below implements this behavior.
>>> > +        */
>>> > +       switch (*code) {
>>> > +       case HOME_PRESS:
>>> > +               eeepc->home_key_state = HOME_PRESS;
>>> > +               *code = ASUS_WMI_KEY_IGNORE;
>>> > +               break;
>>> > +       case HOME_HOLD:
>>> > +               if (eeepc->home_key_state == HOME_HOLD) {
>>> > +                       *code = ASUS_WMI_KEY_IGNORE;
>>> > +               } else {
>>> > +                       eeepc->home_key_state = HOME_HOLD;
>>> > +                       *value = 1;
>>> > +                       *autorelease = 0;
>>> > +               }
>>> > +               break;
>>> > +       case HOME_RELEASE:
>>> > +               if (eeepc->home_key_state == HOME_RELEASE) {
>>> > +                       dev_warn(&asus_wmi->platform_device->dev,
>>> > +                                "Unexpected home key release event\n");
>>> > +                       *code = ASUS_WMI_KEY_IGNORE;
>>> > +               } else {
>>> > +                       *code = eeepc->home_key_state;
>>> > +                       eeepc->home_key_state = HOME_RELEASE;
>>> > +                       is_press = (*code == HOME_PRESS);
>>> > +                       *value = is_press;
>>> > +                       *autorelease = is_press;
>>> > +               }
>>> > +               break;
>>> > +       }
>>> > +}
>>> > +
>>>
>>> Why not something simpler like this ?
>>>
>>> static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int code,
>>>                                                  int *value, int *autorelease)
>>> {
>>>         if (code == 0xe4) {
>>>                 *value = 1;
>>>                 *autorelease = 0;
>>>         } else if (code == 0xe5) {
>>>                 *value = 0;
>>>                 *autorelease = 0;
>>>        }
>>> }
>>>
>>> with this keymap :
>>>
>>>        { KE_KEY, 0xe4, { KEY_HOME } }, /* Home Key Down */
>>>        { KE_KEY, 0xe5, { KEY_HOME } }, /* Home Key Up */
>>>        { KE_KEY, 0xea, { KEY_PROG2 } }, /* Home Key hold more than one second */
>>>
>>>
>>> This sounds simpler and we don't loose information (key down and key
>>> up both event reported at the right time).
>>> 0xe5 is *always* sent after 0xe4 right ?
>>
>> I guess it depends on what key events we want on a press-and-hold.
>> Remember that you're going to get a scan code sequence like "0xe4 0xea
>> 0xea ... 0xea 0xe5", so with my implementation you get
>>
>>  KEY_PROG2 press
>>  KEY_PROG2 release
>>
>> With yours
>>
>>  KEY_HOME press
>>  KEY_PROG2 press
>>  KEY_PROG2 release
>>  // KEY_PROG2 press/release repeats every 0.5 secs while button held
>>  KEY_HOME release
>>
>> At minimum I'd think we'd want to only send a single PROG2 press/release
>> for button hold. My thought was that you'd only want to get the code for
>> 0xe4 or 0xea, not both, but I suppose that's debatable.
>
> If you keep a keyboard key pressed, you want multiple events, not one right ?
> I think it's important not to loose informations. If someone keep this
> key pressed more than 1.5 second, I think it's good idea to send
> multiple KEY_PROG2.
>
> About KEY_HOME press / release, and filtering KEY_HOME after
> KEY_PROG2, I'm not sure. So if you really want it, and nobody
> complains, I'll be happy to accept your patch.

Here is how I envision using these keys.  I wanted to map quick press
to GNOME3/KDE4/Unity's Activities menu and map press-and-hold to
script that rotates screen.  I like the repeating of rotates but I
didn't really want a rotate to also force a pop up of the activities
menu.

>
>> And back to the question of KEY_HOME -- that's not really what you want,
>> is it? As in "move cursor to start of line"?
>
> Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
> What about:
> - KEY_CYCLEWINDOWS
> - KEY_COMPUTER
> - KEY_HOMEPAGE
> - KEY_DASHBOARD
>
> I think KEY_HOMEPAGE is the best choice.

I've only had limited time to look more.  So far, I found under udev a
toshiba tablet that maps what is probably a rotate button to
KEY_DIRECTION so thats one option for it instead of KEY_PROG2.  I
couldn't find anybody using that though.

I see in /usr/share/X11/symbols/inet:

    key <I162>   {      [ XF86RotateWindows     ]       };

and in /usr/share/X11/xkb/symbols/evdev:

xkb/keycodes/evdev:     <I162> = 162;   // #define KEY_CYCLEWINDOWS        154

Looks like KEY_CYCLEWINDOWS is already hooked up to
gnome-settings-daemon to auto-rotate screen?

I ran into total dead end for finding a pre-existing example of home
button usage.  I'm really surprised we do not yet have a KEY_LAUNCHER
or similar because so many tablet PCs/smartphones/pads do have a
button with this specific concept of "bring up your icon based
application launcher".

To add to your list, I'll also throw out:

- KEY_MENU
- KEY_EXIT (smartphones sometime mean this)
- KEY_PROG3 (basically all that Windows is doing)
- KEY_LAUNCHER (why not be the first to finally create it!)

I vote for either KEY_PROG3 or KEY_HOMEPAGE for at least short term.

Chris

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Fri, Mar 25, 2011 at 09:53:20AM -0500, Chris Bagwell wrote:
> On Fri, Mar 25, 2011 at 9:05 AM, Corentin Chary
> <corentin.chary@gmail.com> wrote:
> > On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
> > <seth.forshee@canonical.com> wrote:
> >> On Fri, Mar 25, 2011 at 01:28:30PM +0000, Corentin Chary wrote:
> >>> > +static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
> >>> > +                                int *value, int *autorelease)
> >>> > +{
> >>> > +       struct eeepc_wmi_driver *eeepc = to_eeepc_wmi_driver(asus_wmi);
> >>> > +       int is_press;
> >>> > +
> >>> > +       /*
> >>> > +        * The following behavior is used for T101MT "Home" key:
> >>> > +        *
> >>> > +        *   On press:   No event set
> >>> > +        *   On hold:    KEY_PROG2 press sent once w/o autorelease
> >>> > +        *   On release: If key was held, KEY_PROG2 release sent.
> >>> > +        *               Otherwise KEY_HOME press sent w/ autorelease.
> >>> > +        *
> >>> > +        * The simple state machine below implements this behavior.
> >>> > +        */
> >>> > +       switch (*code) {
> >>> > +       case HOME_PRESS:
> >>> > +               eeepc->home_key_state = HOME_PRESS;
> >>> > +               *code = ASUS_WMI_KEY_IGNORE;
> >>> > +               break;
> >>> > +       case HOME_HOLD:
> >>> > +               if (eeepc->home_key_state == HOME_HOLD) {
> >>> > +                       *code = ASUS_WMI_KEY_IGNORE;
> >>> > +               } else {
> >>> > +                       eeepc->home_key_state = HOME_HOLD;
> >>> > +                       *value = 1;
> >>> > +                       *autorelease = 0;
> >>> > +               }
> >>> > +               break;
> >>> > +       case HOME_RELEASE:
> >>> > +               if (eeepc->home_key_state == HOME_RELEASE) {
> >>> > +                       dev_warn(&asus_wmi->platform_device->dev,
> >>> > +                                "Unexpected home key release event\n");
> >>> > +                       *code = ASUS_WMI_KEY_IGNORE;
> >>> > +               } else {
> >>> > +                       *code = eeepc->home_key_state;
> >>> > +                       eeepc->home_key_state = HOME_RELEASE;
> >>> > +                       is_press = (*code == HOME_PRESS);
> >>> > +                       *value = is_press;
> >>> > +                       *autorelease = is_press;
> >>> > +               }
> >>> > +               break;
> >>> > +       }
> >>> > +}
> >>> > +
> >>>
> >>> Why not something simpler like this ?
> >>>
> >>> static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int code,
> >>>                                                  int *value, int *autorelease)
> >>> {
> >>>         if (code == 0xe4) {
> >>>                 *value = 1;
> >>>                 *autorelease = 0;
> >>>         } else if (code == 0xe5) {
> >>>                 *value = 0;
> >>>                 *autorelease = 0;
> >>>        }
> >>> }
> >>>
> >>> with this keymap :
> >>>
> >>>        { KE_KEY, 0xe4, { KEY_HOME } }, /* Home Key Down */
> >>>        { KE_KEY, 0xe5, { KEY_HOME } }, /* Home Key Up */
> >>>        { KE_KEY, 0xea, { KEY_PROG2 } }, /* Home Key hold more than one second */
> >>>
> >>>
> >>> This sounds simpler and we don't loose information (key down and key
> >>> up both event reported at the right time).
> >>> 0xe5 is *always* sent after 0xe4 right ?
> >>
> >> I guess it depends on what key events we want on a press-and-hold.
> >> Remember that you're going to get a scan code sequence like "0xe4 0xea
> >> 0xea ... 0xea 0xe5", so with my implementation you get
> >>
> >>  KEY_PROG2 press
> >>  KEY_PROG2 release
> >>
> >> With yours
> >>
> >>  KEY_HOME press
> >>  KEY_PROG2 press
> >>  KEY_PROG2 release
> >>  // KEY_PROG2 press/release repeats every 0.5 secs while button held
> >>  KEY_HOME release
> >>
> >> At minimum I'd think we'd want to only send a single PROG2 press/release
> >> for button hold. My thought was that you'd only want to get the code for
> >> 0xe4 or 0xea, not both, but I suppose that's debatable.
> >
> > If you keep a keyboard key pressed, you want multiple events, not one right ?
> > I think it's important not to loose informations. If someone keep this
> > key pressed more than 1.5 second, I think it's good idea to send
> > multiple KEY_PROG2.
> >
> > About KEY_HOME press / release, and filtering KEY_HOME after
> > KEY_PROG2, I'm not sure. So if you really want it, and nobody
> > complains, I'll be happy to accept your patch.
> 
> Here is how I envision using these keys.  I wanted to map quick press
> to GNOME3/KDE4/Unity's Activities menu and map press-and-hold to
> script that rotates screen.  I like the repeating of rotates but I
> didn't really want a rotate to also force a pop up of the activities
> menu.

I've been experimenting with different keyboards a bit, and I basically
see two different behaviours.

  1. Standard keys and some hotkeys - These tend to send a press event
     followed by a short delay, followed by rapidly-repeating press
     events. I.e. what you normally see when you are typing. For hotkeys
     userspace seems to only act upon the release event.

  2. Some hotkeys only seem to send a single scan code when the key is
     released (this seems to be the case with most of the T101MT
     hotkeys). In these cases the drivers send a press followed
     immediately by a release.

So I'm starting to think that the most natural way to handle this is to
treat it as a single key with a couple of special flags (bit 3 = repeat
event and bit 0 = release event). I.e. we only send on keycode and the
filter code just clears the flag bits, clears autorelease, and decides
whether to send a press or release, if (code & ~0x09) == 0xe4. Then
userspace could map seperate events on press and hold if desired.

In any case I don't think multiple repeat events while the button is
held is the right thing to do.

> >> And back to the question of KEY_HOME -- that's not really what you want,
> >> is it? As in "move cursor to start of line"?
> >
> > Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
> > What about:
> > - KEY_CYCLEWINDOWS
> > - KEY_COMPUTER
> > - KEY_HOMEPAGE
> > - KEY_DASHBOARD
> >
> > I think KEY_HOMEPAGE is the best choice.
> 
> I've only had limited time to look more.  So far, I found under udev a
> toshiba tablet that maps what is probably a rotate button to
> KEY_DIRECTION so thats one option for it instead of KEY_PROG2.  I
> couldn't find anybody using that though.
> 
> I see in /usr/share/X11/symbols/inet:
> 
>     key <I162>   {      [ XF86RotateWindows     ]       };
> 
> and in /usr/share/X11/xkb/symbols/evdev:
> 
> xkb/keycodes/evdev:     <I162> = 162;   // #define KEY_CYCLEWINDOWS        154
> 
> Looks like KEY_CYCLEWINDOWS is already hooked up to
> gnome-settings-daemon to auto-rotate screen?
> 
> I ran into total dead end for finding a pre-existing example of home
> button usage.  I'm really surprised we do not yet have a KEY_LAUNCHER
> or similar because so many tablet PCs/smartphones/pads do have a
> button with this specific concept of "bring up your icon based
> application launcher".

That seems to map well onto the idea of the Windows button, which iirc
maps to KEY_LEFTMETA or KEY_RIGHMETA.

> 
> To add to your list, I'll also throw out:
> 
> - KEY_MENU
> - KEY_EXIT (smartphones sometime mean this)
> - KEY_PROG3 (basically all that Windows is doing)
> - KEY_LAUNCHER (why not be the first to finally create it!)
> 
> I vote for either KEY_PROG3 or KEY_HOMEPAGE for at least short term.
> 
> Chris
> 

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Corentin Chary]

> Here is how I envision using these keys.  I wanted to map quick press
> to GNOME3/KDE4/Unity's Activities menu and map press-and-hold to
> script that rotates screen.  I like the repeating of rotates but I
> didn't really want a rotate to also force a pop up of the activities
> menu.

Makes sense.

>>
>>> And back to the question of KEY_HOME -- that's not really what you want,
>>> is it? As in "move cursor to start of line"?
>>
>> Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
>> What about:
>> - KEY_CYCLEWINDOWS
>> - KEY_COMPUTER
>> - KEY_HOMEPAGE
>> - KEY_DASHBOARD
>>
>> I think KEY_HOMEPAGE is the best choice.
>
> I've only had limited time to look more.  So far, I found under udev a
> toshiba tablet that maps what is probably a rotate button to
> KEY_DIRECTION so thats one option for it instead of KEY_PROG2.  I
> couldn't find anybody using that though.
>
> I see in /usr/share/X11/symbols/inet:
>
>    key <I162>   {      [ XF86RotateWindows     ]       };
>
> and in /usr/share/X11/xkb/symbols/evdev:
>
> xkb/keycodes/evdev:     <I162> = 162;   // #define KEY_CYCLEWINDOWS        154
>
> Looks like KEY_CYCLEWINDOWS is already hooked up to
> gnome-settings-daemon to auto-rotate screen?

XF86RotateWindows is probably more "alt-tab" than "rotate screen".

> I ran into total dead end for finding a pre-existing example of home
> button usage.  I'm really surprised we do not yet have a KEY_LAUNCHER
> or similar because so many tablet PCs/smartphones/pads do have a
> button with this specific concept of "bring up your icon based
> application launcher".

KEY_LAUNCHER sounds like "Alt-F2" (that means krunner on KDE).
The advantage over KEY_HOMEPAGE is that it would work even when a
browser have the focus.

> - KEY_MENU
> - KEY_EXIT (smartphones sometime mean this)
> - KEY_PROG3 (basically all that Windows is doing)
> - KEY_LAUNCHER (why not be the first to finally create it!)
>
> I vote for either KEY_PROG3 or KEY_HOMEPAGE for at least short term.

So now, I vote (in order):
- KEY_LAUNCHER
- KEY_DASHBOARD
- KEY_PROG3

With this behavior (I'll use KEY_LAUCHER and KEY_PROG2)

- On press, do nothing
- On release, emit KEY_LAUNCHER if KEY_PROG2 wasn't emited
- On press, emit only one  KEY_PROG2 (I'd like to emit repeated
KEY_PROG2, but I understand that it makes things harder for
userspace).

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Fri, Mar 25, 2011 at 03:43:11PM +0000, Corentin Chary wrote:
> > Here is how I envision using these keys.  I wanted to map quick press
> > to GNOME3/KDE4/Unity's Activities menu and map press-and-hold to
> > script that rotates screen.  I like the repeating of rotates but I
> > didn't really want a rotate to also force a pop up of the activities
> > menu.
> 
> Makes sense.
> 
> >>
> >>> And back to the question of KEY_HOME -- that's not really what you want,
> >>> is it? As in "move cursor to start of line"?
> >>
> >> Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
> >> What about:
> >> - KEY_CYCLEWINDOWS
> >> - KEY_COMPUTER
> >> - KEY_HOMEPAGE
> >> - KEY_DASHBOARD
> >>
> >> I think KEY_HOMEPAGE is the best choice.
> >
> > I've only had limited time to look more.  So far, I found under udev a
> > toshiba tablet that maps what is probably a rotate button to
> > KEY_DIRECTION so thats one option for it instead of KEY_PROG2.  I
> > couldn't find anybody using that though.
> >
> > I see in /usr/share/X11/symbols/inet:
> >
> >    key <I162>   {      [ XF86RotateWindows     ]       };
> >
> > and in /usr/share/X11/xkb/symbols/evdev:
> >
> > xkb/keycodes/evdev:     <I162> = 162;   // #define KEY_CYCLEWINDOWS        154
> >
> > Looks like KEY_CYCLEWINDOWS is already hooked up to
> > gnome-settings-daemon to auto-rotate screen?
> 
> XF86RotateWindows is probably more "alt-tab" than "rotate screen".

Gnome at least seems to handle it by doing a screen rotation.

> > I ran into total dead end for finding a pre-existing example of home
> > button usage.  I'm really surprised we do not yet have a KEY_LAUNCHER
> > or similar because so many tablet PCs/smartphones/pads do have a
> > button with this specific concept of "bring up your icon based
> > application launcher".
> 
> KEY_LAUNCHER sounds like "Alt-F2" (that means krunner on KDE).
> The advantage over KEY_HOMEPAGE is that it would work even when a
> browser have the focus.
> 
> > - KEY_MENU
> > - KEY_EXIT (smartphones sometime mean this)
> > - KEY_PROG3 (basically all that Windows is doing)
> > - KEY_LAUNCHER (why not be the first to finally create it!)
> >
> > I vote for either KEY_PROG3 or KEY_HOMEPAGE for at least short term.
> 
> So now, I vote (in order):
> - KEY_LAUNCHER
> - KEY_DASHBOARD
> - KEY_PROG3
> 
> With this behavior (I'll use KEY_LAUCHER and KEY_PROG2)
> 
> - On press, do nothing
> - On release, emit KEY_LAUNCHER if KEY_PROG2 wasn't emited
> - On press, emit only one  KEY_PROG2 (I'd like to emit repeated

Do you mean on hold?

> KEY_PROG2, but I understand that it makes things harder for
> userspace).

I have a keyboard that emit repeated press events for hotkeys, so I
think userspace can handle this. It just waits for the release before
doing anything, so as long as it's multiple press events and a single
release I think we're okay.

Since it seems KEY_CYCLEWINDOWS might actually accomplish what a hold is
intended to do, I think I'll proceed with the following slightly amended
version of your plan.

 - On press, do nothing
 - On hold, emit KEY_CYCLEWINDOWS repeatedly
 - On release, emit KEY_LAUNCHER w/ autorelease if KEY_CYCLEWINDOWS
   wasn't released, else emit KEY_CYCLEWINDOWS release

Assuming of course that Dmitry agrees that adding KEY_LAUNCHER makes
sense.

Seth


------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar
_______________________________________________
Acpi4asus-user mailing list
Acpi4asus-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/acpi4asus-user

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Dmitry Torokhov]

On Fri, Mar 25, 2011 at 02:05:18PM +0000, Corentin Chary wrote:
> On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> >
> > I guess it depends on what key events we want on a press-and-hold.
> > Remember that you're going to get a scan code sequence like "0xe4 0xea
> > 0xea ... 0xea 0xe5", so with my implementation you get
> >
> >  KEY_PROG2 press
> >  KEY_PROG2 release
> >
> > With yours
> >
> >  KEY_HOME press
> >  KEY_PROG2 press
> >  KEY_PROG2 release
> >  // KEY_PROG2 press/release repeats every 0.5 secs while button held
> >  KEY_HOME release
> >
> > At minimum I'd think we'd want to only send a single PROG2 press/release
> > for button hold. My thought was that you'd only want to get the code for
> > 0xe4 or 0xea, not both, but I suppose that's debatable.
> 
> If you keep a keyboard key pressed, you want multiple events, not one right ?
> I think it's important not to loose informations. If someone keep this
> key pressed more than 1.5 second, I think it's good idea to send
> multiple KEY_PROG2.

You can have input autorepeat facilities do that for you. If you want to
do this yourself then the proper value for repeated events is _2_, not
1, or they will be dropped as duplicates.

> 
> About KEY_HOME press / release, and filtering KEY_HOME after
> KEY_PROG2, I'm not sure. So if you really want it, and nobody
> complains, I'll be happy to accept your patch.
> 
> > And back to the question of KEY_HOME -- that's not really what you want,
> > is it? As in "move cursor to start of line"?
> 
> Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
> What about:
> - KEY_CYCLEWINDOWS
> - KEY_COMPUTER
> - KEY_HOMEPAGE
> - KEY_DASHBOARD
> 
> I think KEY_HOMEPAGE is the best choice.

No, KEY_HOMEPAGE should cause your browser to go to your home page. What
you want is either:

#define KEY_CONFIG         171     /* AL Consumer Control Configuration */
#define KEY_DASHBOARD      204     /* AL Dashboard */

("AL" stands for "Application Launcher", "AC" for 'Application Control",
matched loosely to HUT tables).

Thanks.

-- 
Dmitry

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Fri, Mar 25, 2011 at 09:14:05AM -0700, Dmitry Torokhov wrote:
> > If you keep a keyboard key pressed, you want multiple events, not one right ?
> > I think it's important not to loose informations. If someone keep this
> > key pressed more than 1.5 second, I think it's good idea to send
> > multiple KEY_PROG2.
> 
> You can have input autorepeat facilities do that for you. If you want to
> do this yourself then the proper value for repeated events is _2_, not
> 1, or they will be dropped as duplicates.
> 
> > 
> > About KEY_HOME press / release, and filtering KEY_HOME after
> > KEY_PROG2, I'm not sure. So if you really want it, and nobody
> > complains, I'll be happy to accept your patch.
> > 
> > > And back to the question of KEY_HOME -- that's not really what you want,
> > > is it? As in "move cursor to start of line"?
> > 
> > Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
> > What about:
> > - KEY_CYCLEWINDOWS
> > - KEY_COMPUTER
> > - KEY_HOMEPAGE
> > - KEY_DASHBOARD
> > 
> > I think KEY_HOMEPAGE is the best choice.
> 
> No, KEY_HOMEPAGE should cause your browser to go to your home page. What
> you want is either:
> 
> #define KEY_CONFIG         171     /* AL Consumer Control Configuration */
> #define KEY_DASHBOARD      204     /* AL Dashboard */
> 
> ("AL" stands for "Application Launcher", "AC" for 'Application Control",
> matched loosely to HUT tables).

Dmitry,

I didn't see you responses before my last response, so you can ignore
it. Let me make sure I understand correctly. For reference, the button
in question sends an 0xe4 scancode on press, repeated 0xea scancodes on
hold, and 0xe5 on release.

So, would the following make sense (KEY_TBD is may abbreviation for "key
to be determinded"):

 - On press (0xe4), send KEY_TBD, value = 1
 - On hold (0xea), send KEY_TBD, value = 2
 - On release (0xe5), send KEY_TBD, value = 0

Also, is KEY_CYCLEWINDOWS intended to imply screen rotation, or
something else?

Thanks,
Seth


------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Dmitry Torokhov]

On Fri, Mar 25, 2011 at 11:28:50AM -0500, Seth Forshee wrote:
> On Fri, Mar 25, 2011 at 09:14:05AM -0700, Dmitry Torokhov wrote:
> > > If you keep a keyboard key pressed, you want multiple events, not one right ?
> > > I think it's important not to loose informations. If someone keep this
> > > key pressed more than 1.5 second, I think it's good idea to send
> > > multiple KEY_PROG2.
> > 
> > You can have input autorepeat facilities do that for you. If you want to
> > do this yourself then the proper value for repeated events is _2_, not
> > 1, or they will be dropped as duplicates.
> > 
> > > 
> > > About KEY_HOME press / release, and filtering KEY_HOME after
> > > KEY_PROG2, I'm not sure. So if you really want it, and nobody
> > > complains, I'll be happy to accept your patch.
> > > 
> > > > And back to the question of KEY_HOME -- that's not really what you want,
> > > > is it? As in "move cursor to start of line"?
> > > 
> > > Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
> > > What about:
> > > - KEY_CYCLEWINDOWS
> > > - KEY_COMPUTER
> > > - KEY_HOMEPAGE
> > > - KEY_DASHBOARD
> > > 
> > > I think KEY_HOMEPAGE is the best choice.
> > 
> > No, KEY_HOMEPAGE should cause your browser to go to your home page. What
> > you want is either:
> > 
> > #define KEY_CONFIG         171     /* AL Consumer Control Configuration */
> > #define KEY_DASHBOARD      204     /* AL Dashboard */
> > 
> > ("AL" stands for "Application Launcher", "AC" for 'Application Control",
> > matched loosely to HUT tables).
> 
> Dmitry,
> 
> I didn't see you responses before my last response, so you can ignore
> it. Let me make sure I understand correctly. For reference, the button
> in question sends an 0xe4 scancode on press, repeated 0xea scancodes on
> hold, and 0xe5 on release.
> 
> So, would the following make sense (KEY_TBD is may abbreviation for "key
> to be determinded"):
> 
>  - On press (0xe4), send KEY_TBD, value = 1
>  - On hold (0xea), send KEY_TBD, value = 2
>  - On release (0xe5), send KEY_TBD, value = 0

Yes.

> 
> Also, is KEY_CYCLEWINDOWS intended to imply screen rotation, or
> something else?

Hmm, it was added way before my time. I'd expect it to have an action
similar to Alt-Tab. Vojtech, do you still remember what it was supposed
to be?

-- 
Dmitry

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Fri, Mar 25, 2011 at 10:03:33AM -0700, Dmitry Torokhov wrote:
> > So, would the following make sense (KEY_TBD is may abbreviation for "key
> > to be determinded"):
> > 
> >  - On press (0xe4), send KEY_TBD, value = 1
> >  - On hold (0xea), send KEY_TBD, value = 2
> >  - On release (0xe5), send KEY_TBD, value = 0
> 
> Yes.

There's a little problem with this. The driver uses sparse-keymap, which
calls input_report_key(), which is doing this:

	input_event(dev, EV_KEY, code, !!value);

I don't see the hold events in userspace, so they must be getting
dropped as duplicates as you suggested.

I'm not sure who is at fault here. Should input_report_key() not be
forcing value to 0 or 1? Or should sparse-keymap be calling
input_event() directly? Or is sparse-keymap the wrong tool for handling
keys this way?

Thanks,
Seth


------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Dmitry Torokhov]

On Fri, Mar 25, 2011 at 01:58:24PM -0500, Seth Forshee wrote:
> On Fri, Mar 25, 2011 at 10:03:33AM -0700, Dmitry Torokhov wrote:
> > > So, would the following make sense (KEY_TBD is may abbreviation for "key
> > > to be determinded"):
> > > 
> > >  - On press (0xe4), send KEY_TBD, value = 1
> > >  - On hold (0xea), send KEY_TBD, value = 2
> > >  - On release (0xe5), send KEY_TBD, value = 0
> > 
> > Yes.
> 
> There's a little problem with this. The driver uses sparse-keymap, which
> calls input_report_key(), which is doing this:
> 
> 	input_event(dev, EV_KEY, code, !!value);

Ah, right, it really did not have in mind passing on auto-repeating keys
as is when it was written.

> 
> I don't see the hold events in userspace, so they must be getting
> dropped as duplicates as you suggested.
> 
> I'm not sure who is at fault here. Should input_report_key() not be
> forcing value to 0 or 1? Or should sparse-keymap be calling
> input_event() directly? Or is sparse-keymap the wrong tool for handling
> keys this way?

I do not thik we shoudl burden sparse-keymap with handling repeats. Just
mark the device as auto-repeating (by setting EV_REP bit) and let input
core autorepeat logic do it for you.

Thanks.

-- 
Dmitry

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Chris Bagwell]

On Sun, Mar 27, 2011 at 12:13 PM, Dmitry Torokhov
<dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Mar 25, 2011 at 01:58:24PM -0500, Seth Forshee wrote:
>> On Fri, Mar 25, 2011 at 10:03:33AM -0700, Dmitry Torokhov wrote:
>> > > So, would the following make sense (KEY_TBD is may abbreviation for "key
>> > > to be determinded"):
>> > >
>> > >  - On press (0xe4), send KEY_TBD, value = 1
>> > >  - On hold (0xea), send KEY_TBD, value = 2
>> > >  - On release (0xe5), send KEY_TBD, value = 0
>> >
>> > Yes.
>>
>> There's a little problem with this. The driver uses sparse-keymap, which
>> calls input_report_key(), which is doing this:
>>
>>       input_event(dev, EV_KEY, code, !!value);
>
> Ah, right, it really did not have in mind passing on auto-repeating keys
> as is when it was written.
>
>>
>> I don't see the hold events in userspace, so they must be getting
>> dropped as duplicates as you suggested.
>>
>> I'm not sure who is at fault here. Should input_report_key() not be
>> forcing value to 0 or 1? Or should sparse-keymap be calling
>> input_event() directly? Or is sparse-keymap the wrong tool for handling
>> keys this way?
>
> I do not thik we shoudl burden sparse-keymap with handling repeats. Just
> mark the device as auto-repeating (by setting EV_REP bit) and let input
> core autorepeat logic do it for you.

It would be nice to have this key sent roughly every 2 seconds to
re-execute a rotate script using generic key-to-script support in most
X environments.  So to use kernel auto-repeat we'd need to bump up
REP_DELAY quite a bit or do filtering in userspace.  If userspace is
filtering then its not much more work to do their own auto-repeat as
well.

If we do set up auto-repeat and increase REP_DELAY, I'm guessing this
would enable auto-repeat for all other keys defined in driver?  That
needs to have some thought on if could have negative impact (any other
keys not using auto-release?).

Chris

>
> Thanks.
>
> --
> Dmitry
>

------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Dmitry Torokhov]

On Sun, Mar 27, 2011 at 01:32:45PM -0500, Chris Bagwell wrote:
> On Sun, Mar 27, 2011 at 12:13 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Mar 25, 2011 at 01:58:24PM -0500, Seth Forshee wrote:
> >> On Fri, Mar 25, 2011 at 10:03:33AM -0700, Dmitry Torokhov wrote:
> >> > > So, would the following make sense (KEY_TBD is may abbreviation for "key
> >> > > to be determinded"):
> >> > >
> >> > >  - On press (0xe4), send KEY_TBD, value = 1
> >> > >  - On hold (0xea), send KEY_TBD, value = 2
> >> > >  - On release (0xe5), send KEY_TBD, value = 0
> >> >
> >> > Yes.
> >>
> >> There's a little problem with this. The driver uses sparse-keymap, which
> >> calls input_report_key(), which is doing this:
> >>
> >>       input_event(dev, EV_KEY, code, !!value);
> >
> > Ah, right, it really did not have in mind passing on auto-repeating keys
> > as is when it was written.
> >
> >>
> >> I don't see the hold events in userspace, so they must be getting
> >> dropped as duplicates as you suggested.
> >>
> >> I'm not sure who is at fault here. Should input_report_key() not be
> >> forcing value to 0 or 1? Or should sparse-keymap be calling
> >> input_event() directly? Or is sparse-keymap the wrong tool for handling
> >> keys this way?
> >
> > I do not thik we shoudl burden sparse-keymap with handling repeats. Just
> > mark the device as auto-repeating (by setting EV_REP bit) and let input
> > core autorepeat logic do it for you.
> 
> It would be nice to have this key sent roughly every 2 seconds to
> re-execute a rotate script using generic key-to-script support in most
> X environments.  So to use kernel auto-repeat we'd need to bump up
> REP_DELAY quite a bit or do filtering in userspace.  If userspace is
> filtering then its not much more work to do their own auto-repeat as
> well.

That is true as well.

> 
> If we do set up auto-repeat and increase REP_DELAY, I'm guessing this
> would enable auto-repeat for all other keys defined in driver?  That
> needs to have some thought on if could have negative impact (any other
> keys not using auto-release?).

Right. Right now there are 4 autoreprat options (in general):

- hardware autorepeat (if hardware supports it);
- input core software autorepeat (one delay and rate per input device);
- driver-implemented software autorepeat - in cases when different
  repeat rate is needed;
- userspace autorepeat (like X does nowadays);

Well, 4th option is not mutually exclusive with the other 3...

-- 
Dmitry

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Seth Forshee]

On Sun, Mar 27, 2011 at 12:11:16PM -0700, Dmitry Torokhov wrote:
> > If we do set up auto-repeat and increase REP_DELAY, I'm guessing this
> > would enable auto-repeat for all other keys defined in driver?  That
> > needs to have some thought on if could have negative impact (any other
> > keys not using auto-release?).
> 
> Right. Right now there are 4 autoreprat options (in general):
> 
> - hardware autorepeat (if hardware supports it);
> - input core software autorepeat (one delay and rate per input device);
> - driver-implemented software autorepeat - in cases when different
>   repeat rate is needed;
> - userspace autorepeat (like X does nowadays);
> 
> Well, 4th option is not mutually exclusive with the other 3...

Currently all the other keys are using autorelease, so the autorepeat
setting shouldn't be affecting them at all. So the concern is whether
enabling autorepeat could become a problem the next time some oddball
key shows up on a machine.

Corentin, you were concerned about the loss of information to userspace,
but it seems the only way to maintain the hardware events is to drop the
use of sparse-keymap altogether. Do you have an opinion on how to
proceed?

I'm leaning towards using the input core autorepeat, since it seems to
get the closest to typical key behavior.

Seth


------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar

Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

[Corentin Chary]

On Mon, Mar 28, 2011 at 1:46 PM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Sun, Mar 27, 2011 at 12:11:16PM -0700, Dmitry Torokhov wrote:
>> > If we do set up auto-repeat and increase REP_DELAY, I'm guessing this
>> > would enable auto-repeat for all other keys defined in driver?  That
>> > needs to have some thought on if could have negative impact (any other
>> > keys not using auto-release?).
>>
>> Right. Right now there are 4 autoreprat options (in general):
>>
>> - hardware autorepeat (if hardware supports it);
>> - input core software autorepeat (one delay and rate per input device);
>> - driver-implemented software autorepeat - in cases when different
>>   repeat rate is needed;
>> - userspace autorepeat (like X does nowadays);
>>
>> Well, 4th option is not mutually exclusive with the other 3...
>
> Currently all the other keys are using autorelease, so the autorepeat
> setting shouldn't be affecting them at all. So the concern is whether
> enabling autorepeat could become a problem the next time some oddball
> key shows up on a machine.

As far as I know, the only key that could be like that (not yet
mapped, but I've seen it somewhere) is a "Camera Autofocus Key". For
this key we don't really care about "repeating", we just need to send
"press" and "release" events correctly, and userspace will keep
adjusting the autofocus while the key is pressed.

> Corentin, you were concerned about the loss of information to userspace,
> but it seems the only way to maintain the hardware events is to drop the
> use of sparse-keymap altogether. Do you have an opinion on how to
> proceed?

I really don't think dropping sparse-keymap for a single key on a
single model is a good idea.

On my side I'd have used another keycode for 0xea events, and reported
press and release events as they come, since it's more a "two-in-one"
key than a key with hardware autorepeat.

But since Dmitry and Chris seems to prefer the autorepeat approach
with a single key code, if it works then it's ok, we just need to set
REP_DELAY, report press and release events, and drop 0xea events.

> I'm leaning towards using the input core autorepeat, since it seems to
> get the closest to typical key behavior.

-- 
Corentin Chary
http://xf.iksaif.net