Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key
From: Chris Bagwell
Date: Fri Mar 25 2011 - 10:53:25 EST
On Fri, Mar 25, 2011 at 9:05 AM, Corentin Chary
<corentin.chary@xxxxxxxxx> wrote:
> On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
> <seth.forshee@xxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/