| 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
| ||
|
Date: Fri, 25 Mar 2011 14:05:18 +0000
From: Corentin Chary <corentin.chary@...il.com>
To: Corentin Chary <corentin.chary@...il.com>,
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>
Cc: Seth Forshee <seth.forshee@...onical.com>
Subject: Re: [PATCH 2/2] eeepc-wmi: Add support for T101MT Home/Express Gate key
On Fri, Mar 25, 2011 at 1:53 PM, Seth Forshee
<seth.forshee@...onical.com> wrote:
> On Fri, Mar 25, 2011 at 01:28:30PM +0000, Corentin Chary wrote:
>> > +static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
>> > + int *value, int *autorelease)
>> > +{
>> > + struct eeepc_wmi_driver *eeepc = to_eeepc_wmi_driver(asus_wmi);
>> > + int is_press;
>> > +
>> > + /*
>> > + * The following behavior is used for T101MT "Home" key:
>> > + *
>> > + * On press: No event set
>> > + * On hold: KEY_PROG2 press sent once w/o autorelease
>> > + * On release: If key was held, KEY_PROG2 release sent.
>> > + * Otherwise KEY_HOME press sent w/ autorelease.
>> > + *
>> > + * The simple state machine below implements this behavior.
>> > + */
>> > + switch (*code) {
>> > + case HOME_PRESS:
>> > + eeepc->home_key_state = HOME_PRESS;
>> > + *code = ASUS_WMI_KEY_IGNORE;
>> > + break;
>> > + case HOME_HOLD:
>> > + if (eeepc->home_key_state == HOME_HOLD) {
>> > + *code = ASUS_WMI_KEY_IGNORE;
>> > + } else {
>> > + eeepc->home_key_state = HOME_HOLD;
>> > + *value = 1;
>> > + *autorelease = 0;
>> > + }
>> > + break;
>> > + case HOME_RELEASE:
>> > + if (eeepc->home_key_state == HOME_RELEASE) {
>> > + dev_warn(&asus_wmi->platform_device->dev,
>> > + "Unexpected home key release event\n");
>> > + *code = ASUS_WMI_KEY_IGNORE;
>> > + } else {
>> > + *code = eeepc->home_key_state;
>> > + eeepc->home_key_state = HOME_RELEASE;
>> > + is_press = (*code == HOME_PRESS);
>> > + *value = is_press;
>> > + *autorelease = is_press;
>> > + }
>> > + break;
>> > + }
>> > +}
>> > +
>>
>> Why not something simpler like this ?
>>
>> static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int code,
>> int *value, int *autorelease)
>> {
>> if (code == 0xe4) {
>> *value = 1;
>> *autorelease = 0;
>> } else if (code == 0xe5) {
>> *value = 0;
>> *autorelease = 0;
>> }
>> }
>>
>> with this keymap :
>>
>> { KE_KEY, 0xe4, { KEY_HOME } }, /* Home Key Down */
>> { KE_KEY, 0xe5, { KEY_HOME } }, /* Home Key Up */
>> { KE_KEY, 0xea, { KEY_PROG2 } }, /* Home Key hold more than one second */
>>
>>
>> This sounds simpler and we don't loose information (key down and key
>> up both event reported at the right time).
>> 0xe5 is *always* sent after 0xe4 right ?
>
> I guess it depends on what key events we want on a press-and-hold.
> Remember that you're going to get a scan code sequence like "0xe4 0xea
> 0xea ... 0xea 0xe5", so with my implementation you get
>
> KEY_PROG2 press
> KEY_PROG2 release
>
> With yours
>
> KEY_HOME press
> KEY_PROG2 press
> KEY_PROG2 release
> // KEY_PROG2 press/release repeats every 0.5 secs while button held
> KEY_HOME release
>
> At minimum I'd think we'd want to only send a single PROG2 press/release
> for button hold. My thought was that you'd only want to get the code for
> 0xe4 or 0xea, not both, but I suppose that's debatable.
If you keep a keyboard key pressed, you want multiple events, not one right ?
I think it's important not to loose informations. If someone keep this
key pressed more than 1.5 second, I think it's good idea to send
multiple KEY_PROG2.
About KEY_HOME press / release, and filtering KEY_HOME after
KEY_PROG2, I'm not sure. So if you really want it, and nobody
complains, I'll be happy to accept your patch.
> And back to the question of KEY_HOME -- that's not really what you want,
> is it? As in "move cursor to start of line"?
Ho .. right, that's what mean KEY_HOME :/. So no, I don't want that...
What about:
- KEY_CYCLEWINDOWS
- KEY_COMPUTER
- KEY_HOMEPAGE
- KEY_DASHBOARD
I think KEY_HOMEPAGE is the best choice.
--
Corentin Chary
http://xf.iksaif.net
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists