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.
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
