Signed-off-by: Seth Forshee <[email protected]>
---
This button is unusual. On press it emits scanocde 0xe4, on release
0xe5, and when held for a few seconds it begins emitting 0xea about
twice per second. I chose to handle only the press event in order to
make it behave like the other hotkeys.
Probably the closest thing to the intended function of this button
would be to alternate the display orientation between portrait and
landscape, but since there doesn't appear to be a keycode defined for
that function I went with KEY_PROG2. I'm open to other suggestions
however.
drivers/platform/x86/eeepc-wmi.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/platform/x86/eeepc-wmi.c b/drivers/platform/x86/eeepc-wmi.c
index 0ddc434..d761f60 100644
--- a/drivers/platform/x86/eeepc-wmi.c
+++ b/drivers/platform/x86/eeepc-wmi.c
@@ -70,6 +70,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, 0xe4, { KEY_PROG2 } }, /* Rotate Display */
{ KE_KEY, 0xe9, { KEY_BRIGHTNESS_ZERO } },
{ KE_KEY, 0xeb, { KEY_CAMERA_ZOOMOUT } },
{ KE_KEY, 0xec, { KEY_CAMERA_UP } },
--
1.7.4.1
On Wed, Mar 23, 2011 at 8:39 PM, Seth Forshee
<[email protected]> wrote:
> Signed-off-by: Seth Forshee <[email protected]>
> ---
>
> This button is unusual. On press it emits scanocde 0xe4, on release
> 0xe5, and when held for a few seconds it begins emitting 0xea about
> twice per second. I chose to handle only the press event in order to
> make it behave like the other hotkeys.
>
> Probably the closest thing to the intended function of this button
> would be to alternate the display orientation between portrait and
> landscape, but since there doesn't appear to be a keycode defined for
> that function I went with KEY_PROG2. I'm open to other suggestions
> however.
Hi,
The manual says that under Windows, this is the "Home Key", so
KEY_HOME would be more appropriate.
Also, it would be great to preserve key up and key down informations
(sparse keymap have an "autorelease" parameter to control that).
Thanks,
--
Corentin Chary
http://xf.iksaif.net
On Thu, Mar 24, 2011 at 08:33:33AM +0100, Corentin Chary wrote:
> On Wed, Mar 23, 2011 at 8:39 PM, Seth Forshee
> <[email protected]> wrote:
> > Signed-off-by: Seth Forshee <[email protected]>
> > ---
> >
> > This button is unusual. On press it emits scanocde 0xe4, on release
> > 0xe5, and when held for a few seconds it begins emitting 0xea about
> > twice per second. I chose to handle only the press event in order to
> > make it behave like the other hotkeys.
> >
> > Probably the closest thing to the intended function of this button
> > would be to alternate the display orientation between portrait and
> > landscape, but since there doesn't appear to be a keycode defined for
> > that function I went with KEY_PROG2. I'm open to other suggestions
> > however.
>
> Hi,
> The manual says that under Windows, this is the "Home Key", so
> KEY_HOME would be more appropriate.
The manual does call it the Home Key (I missed previously that the
Express Gate description referred only to the quick boot mode), but the
description of the key operation reads:
Shortly press this key to launch the Touch Gate touch screen
interface. Press and hold this key to preview desktop rotation, and
then release the key to finish desktop rotation.
Which doesn't really sound like a home key to me, and the icon above the
button also suggests display rotation. But then we don't seem to have a
code for that function, so KEY_PROG2 may be no better than KEY_HOME...
> Also, it would be great to preserve key up and key down informations
> (sparse keymap have an "autorelease" parameter to control that).
Okay, I'll add some special casing for this key to support that. Do you
suggest ignoring the 0xea scan codes?
>> Also, it would be great to preserve key up and key down informations
>> (sparse keymap have an "autorelease" parameter to control that).
>
> Okay, I'll add some special casing for this key to support that.
You may have to add an optional callback for that in asus-wmi's driver
model because it's used by both asus-nb-wmi and eeepc-wmi, and we only
want this behavior for eeepc-wmi.
> Do you suggest ignoring the 0xea scan codes?
Yep, 0xea means "The key is still pressed", but it's useless if we
already repport key down and key up events correctly.
You'll have to ignore this scan code explicitly to remove traces in
dmesg (use KEY_IGNORE).
--
Corentin Chary
http://xf.iksaif.net
On Thu, Mar 24, 2011 at 8:14 AM, Seth Forshee
<[email protected]> wrote:
> On Thu, Mar 24, 2011 at 08:33:33AM +0100, Corentin Chary wrote:
>> On Wed, Mar 23, 2011 at 8:39 PM, Seth Forshee
>> <[email protected]> wrote:
>> > Signed-off-by: Seth Forshee <[email protected]>
>> > ---
>> >
>> > This button is unusual. On press it emits scanocde 0xe4, on release
>> > 0xe5, and when held for a few seconds it begins emitting 0xea about
>> > twice per second. I chose to handle only the press event in order to
>> > make it behave like the other hotkeys.
>> >
>> > Probably the closest thing to the intended function of this button
>> > would be to alternate the display orientation between portrait and
>> > landscape, but since there doesn't appear to be a keycode defined for
>> > that function I went with KEY_PROG2. I'm open to other suggestions
>> > however.
>>
>> Hi,
>> The manual says that under Windows, this is the "Home Key", so
>> KEY_HOME would be more appropriate.
>
> The manual does call it the Home Key (I missed previously that the
> Express Gate description referred only to the quick boot mode), but the
> description of the key operation reads:
>
> ?Shortly press this key to launch the Touch Gate touch screen
> ?interface. Press and hold this key to preview desktop rotation, and
> ?then release the key to finish desktop rotation.
>
> Which doesn't really sound like a home key to me, and the icon above the
> button also suggests display rotation. But then we don't seem to have a
> code for that function, so KEY_PROG2 may be no better than KEY_HOME...
>
>> Also, it would be great to preserve key up and key down informations
>> (sparse keymap have an "autorelease" parameter to control that).
>
> Okay, I'll add some special casing for this key to support that. Do you
> suggest ignoring the 0xea scan codes?
>
I have one of these in the mail so I'm interested in outcome.
I suggest implementing as manual describes and can be handled similar
to brightness control logic.
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.
Chris
On Thu, Mar 24, 2011 at 8:27 AM, Chris Bagwell <[email protected]> wrote:
> On Thu, Mar 24, 2011 at 8:14 AM, Seth Forshee
> <[email protected]> wrote:
>> On Thu, Mar 24, 2011 at 08:33:33AM +0100, Corentin Chary wrote:
>>> On Wed, Mar 23, 2011 at 8:39 PM, Seth Forshee
>>> <[email protected]> wrote:
>>> > Signed-off-by: Seth Forshee <[email protected]>
>>> > ---
>>> >
>>> > This button is unusual. On press it emits scanocde 0xe4, on release
>>> > 0xe5, and when held for a few seconds it begins emitting 0xea about
>>> > twice per second. I chose to handle only the press event in order to
>>> > make it behave like the other hotkeys.
>>> >
>>> > Probably the closest thing to the intended function of this button
>>> > would be to alternate the display orientation between portrait and
>>> > landscape, but since there doesn't appear to be a keycode defined for
>>> > that function I went with KEY_PROG2. I'm open to other suggestions
>>> > however.
>>>
>>> Hi,
>>> The manual says that under Windows, this is the "Home Key", so
>>> KEY_HOME would be more appropriate.
>>
>> The manual does call it the Home Key (I missed previously that the
>> Express Gate description referred only to the quick boot mode), but the
>> description of the key operation reads:
>>
>> ?Shortly press this key to launch the Touch Gate touch screen
>> ?interface. Press and hold this key to preview desktop rotation, and
>> ?then release the key to finish desktop rotation.
>>
>> Which doesn't really sound like a home key to me, and the icon above the
>> button also suggests display rotation. But then we don't seem to have a
>> code for that function, so KEY_PROG2 may be no better than KEY_HOME...
>>
>>> Also, it would be great to preserve key up and key down informations
>>> (sparse keymap have an "autorelease" parameter to control that).
>>
>> Okay, I'll add some special casing for this key to support that. Do you
>> suggest ignoring the 0xea scan codes?
>>
>
> I have one of these in the mail so I'm interested in outcome.
>
> I suggest implementing as manual describes and can be handled similar
> to brightness control logic.
>
> 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.
>
> Chris
>
When I mentioned similar to brightness control logic, I meant the
logic in eeepc-laptop and not eeepc-wmi. It appears to use sparse
keys logic but with some event filtering.
Chris
On Thu, Mar 24, 2011 at 08:27:41AM -0500, Chris Bagwell wrote:
> On Thu, Mar 24, 2011 at 8:14 AM, Seth Forshee
> <[email protected]> wrote:
> > On Thu, Mar 24, 2011 at 08:33:33AM +0100, Corentin Chary wrote:
> >> On Wed, Mar 23, 2011 at 8:39 PM, Seth Forshee
> >> <[email protected]> wrote:
> >> > Signed-off-by: Seth Forshee <[email protected]>
> >> > ---
> >> >
> >> > This button is unusual. On press it emits scanocde 0xe4, on release
> >> > 0xe5, and when held for a few seconds it begins emitting 0xea about
> >> > twice per second. I chose to handle only the press event in order to
> >> > make it behave like the other hotkeys.
> >> >
> >> > Probably the closest thing to the intended function of this button
> >> > would be to alternate the display orientation between portrait and
> >> > landscape, but since there doesn't appear to be a keycode defined for
> >> > that function I went with KEY_PROG2. I'm open to other suggestions
> >> > however.
> >>
> >> Hi,
> >> The manual says that under Windows, this is the "Home Key", so
> >> KEY_HOME would be more appropriate.
> >
> > The manual does call it the Home Key (I missed previously that the
> > Express Gate description referred only to the quick boot mode), but the
> > description of the key operation reads:
> >
> > Shortly press this key to launch the Touch Gate touch screen
> > interface. Press and hold this key to preview desktop rotation, and
> > then release the key to finish desktop rotation.
> >
> > Which doesn't really sound like a home key to me, and the icon above the
> > button also suggests display rotation. But then we don't seem to have a
> > code for that function, so KEY_PROG2 may be no better than KEY_HOME...
> >
> >> Also, it would be great to preserve key up and key down informations
> >> (sparse keymap have an "autorelease" parameter to control that).
> >
> > Okay, I'll add some special casing for this key to support that. Do you
> > suggest ignoring the 0xea scan codes?
> >
>
> I have one of these in the mail so I'm interested in outcome.
>
> I suggest implementing as manual describes and can be handled similar
> to brightness control logic.
>
> 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.
Yes, on press-and-hold you get a sequence like
e4 ea ea ... ea e5
So there'd have to be some simple state machine there to be sure the
release event was sent for KEY_PROG2.
> 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.
I haven't used an iPad, but I was assuming on that the home button took
you, well, to the home screen, which is what it does on Android devices.
In my mind that would equate to something like "show desktop" on a
desktop OS, but I guess that's for the desktop designers to decide. In
Windows this one seems to take you to some set up screen, unless you
hold it, in which case it rotates the screen orientation. And I'm
assuming on the press-and-hold you don't see the setup screen before the
rotation, but I never got a chance to experiment with the button under
Windows so I can't say for sure.
> 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.
0xe4 -> KEY_IGNORE
0xe5 -> KEY_HOME
0xea -> KEY_PROG2, (or BTN_TASK to copy android/iOS ?)
--
Corentin Chary
http://xf.iksaif.net
On Thu, Mar 24, 2011 at 8:40 AM, Corentin Chary
<[email protected]> 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.
>
> 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.
Chris
On Thu, Mar 24, 2011 at 09:00:14AM -0500, Chris Bagwell wrote:
> On Thu, Mar 24, 2011 at 8:40 AM, Corentin Chary
> <[email protected]> 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
This is required for the T101MT home key, which behaves differently
than other hotkeys.
Signed-off-by: Seth Forshee <[email protected]>
---
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..c98b787 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;
+ int key_value = 1;
+ int 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..58dd019 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,
+ int *value, int *autorelease);
int (*probe) (struct platform_device *device);
void (*quirks) (struct asus_wmi_driver *driver);
--
1.7.4.1
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 <[email protected]>
---
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 "
"[email protected].");
+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
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.
> +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
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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.
On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
<[email protected]> 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
On Fri, Mar 25, 2011 at 9:05 AM, Corentin Chary
<[email protected]> wrote:
> On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
> <[email protected]> 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
On Fri, Mar 25, 2011 at 09:53:20AM -0500, Chris Bagwell wrote:
> On Fri, Mar 25, 2011 at 9:05 AM, Corentin Chary
> <[email protected]> wrote:
> > On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
> > <[email protected]> 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
>
> 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
On Thu, Mar 24, 2011 at 08:27:41AM -0500, Chris Bagwell wrote:
> On Thu, Mar 24, 2011 at 8:14 AM, Seth Forshee
> <[email protected]> wrote:
> > On Thu, Mar 24, 2011 at 08:33:33AM +0100, Corentin Chary wrote:
> >> On Wed, Mar 23, 2011 at 8:39 PM, Seth Forshee
> >> <[email protected]> wrote:
> >> > Signed-off-by: Seth Forshee <[email protected]>
> >> > ---
> >> >
> >> > This button is unusual. On press it emits scanocde 0xe4, on release
> >> > 0xe5, and when held for a few seconds it begins emitting 0xea about
> >> > twice per second. I chose to handle only the press event in order to
> >> > make it behave like the other hotkeys.
> >> >
> >> > Probably the closest thing to the intended function of this button
> >> > would be to alternate the display orientation between portrait and
> >> > landscape, but since there doesn't appear to be a keycode defined for
> >> > that function I went with KEY_PROG2. I'm open to other suggestions
> >> > however.
> >>
> >> Hi,
> >> The manual says that under Windows, this is the "Home Key", so
> >> KEY_HOME would be more appropriate.
> >
> > The manual does call it the Home Key (I missed previously that the
> > Express Gate description referred only to the quick boot mode), but the
> > description of the key operation reads:
> >
> > ?Shortly press this key to launch the Touch Gate touch screen
> > ?interface. Press and hold this key to preview desktop rotation, and
> > ?then release the key to finish desktop rotation.
> >
> > Which doesn't really sound like a home key to me, and the icon above the
> > button also suggests display rotation. But then we don't seem to have a
> > code for that function, so KEY_PROG2 may be no better than KEY_HOME...
> >
> >> Also, it would be great to preserve key up and key down informations
> >> (sparse keymap have an "autorelease" parameter to control that).
> >
> > Okay, I'll add some special casing for this key to support that. Do you
> > suggest ignoring the 0xea scan codes?
> >
>
> I have one of these in the mail so I'm interested in outcome.
>
> I suggest implementing as manual describes and can be handled similar
> to brightness control logic.
>
> 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.
Not in the kernel. It is a single button and should generate single
event. Have userspace decipher that pressing it once causes application
launcher to appear, holding it longer causes screen rotation, and if you
still keep it pressed for 10 minutes fireworks will start.
>
> 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.
>
It is 'Home' key on your full AT keyboard, normally making cursor to
move to the very first column on the line.
Thanks.
--
Dmitry
On Thu, Mar 24, 2011 at 09:00:14AM -0500, Chris Bagwell wrote:
> On Thu, Mar 24, 2011 at 8:40 AM, Corentin Chary
> <[email protected]> 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.
>
> >
> > 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.
>
It was one of additional keys on Logitech mice IIRC.
--
Dmitry
On Fri, Mar 25, 2011 at 4:07 PM, Dmitry Torokhov
<[email protected]> wrote:
> On Thu, Mar 24, 2011 at 09:00:14AM -0500, Chris Bagwell wrote:
>> On Thu, Mar 24, 2011 at 8:40 AM, Corentin Chary
>> <[email protected]> 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.
>>
>> >
>> > 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.
>>
>
> It was one of additional keys on Logitech mice IIRC.
I thinks it's the same as KEY_CYCLEWINDOWS (there are little windows
on the button).
--
Corentin Chary
http://xf.iksaif.net
On Fri, Mar 25, 2011 at 02:05:18PM +0000, Corentin Chary wrote:
> On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
> <[email protected]> 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
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
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
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
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
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
On Sun, Mar 27, 2011 at 12:13 PM, Dmitry Torokhov
<[email protected]> 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
>
On Sun, Mar 27, 2011 at 01:32:45PM -0500, Chris Bagwell wrote:
> On Sun, Mar 27, 2011 at 12:13 PM, Dmitry Torokhov
> <[email protected]> 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
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
On Mon, Mar 28, 2011 at 1:46 PM, Seth Forshee
<[email protected]> 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
This is required for the T101MT home key, which behaves differently
than other hotkeys.
Signed-off-by: Seth Forshee <[email protected]>
---
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
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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 "
"[email protected].");
+/* 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
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
On Mon, Mar 28, 2011 at 6:33 PM, Seth Forshee
<[email protected]> 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 <[email protected]>
> ---
> 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 "
> "[email protected].");
>
> +/* 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
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
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 | [email protected]
>> 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