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