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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTimF2UR8BVQW-3YJaGFEz39q8ydESAiMOjmdM1X7@mail.gmail.com>
Date:	Fri, 25 Mar 2011 13:28:30 +0000
From:	Corentin Chary <corentin.chary@...il.com>
To:	Seth Forshee <seth.forshee@...onical.com>
Cc:	Chris Bagwell <chris@...bagwell.com>,
	Matthew Garrett <mjg@...hat.com>,
	acpi4asus-user@...ts.sourceforge.net,
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key

> +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