| Message ID | 20190903144632.26299-1-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | aec256d0ecd561036f188dbc8fa7924c47a9edfd |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [v2] HID: apple: Fix stuck function keys when using FN | expand |
Hi Benjamin, On Tue, 3 Sep 2019 at 16:46, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > From: Joao Moreno <mail@joaomoreno.com> > > This fixes an issue in which key down events for function keys would be > repeatedly emitted even after the user has raised the physical key. For > example, the driver fails to emit the F5 key up event when going through > the following steps: > - fnmode=1: hold FN, hold F5, release FN, release F5 > - fnmode=2: hold F5, hold FN, release F5, release FN > > The repeated F5 key down events can be easily verified using xev. > > Signed-off-by: Joao Moreno <mail@joaomoreno.com> > Co-developed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > > Hi Joao, > > last chance to pull back :) > > If you are still happy, I'll push this version > > Cheers, > Benjamin > Looks great. Thanks a bunch for your help! Cheers, João > drivers/hid/hid-apple.c | 49 +++++++++++++++++++++++------------------ > 1 file changed, 28 insertions(+), 21 deletions(-) > > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c > index 81df62f48c4c..6ac8becc2372 100644 > --- a/drivers/hid/hid-apple.c > +++ b/drivers/hid/hid-apple.c > @@ -54,7 +54,6 @@ MODULE_PARM_DESC(swap_opt_cmd, "Swap the Option (\"Alt\") and Command (\"Flag\") > struct apple_sc { > unsigned long quirks; > unsigned int fn_on; > - DECLARE_BITMAP(pressed_fn, KEY_CNT); > DECLARE_BITMAP(pressed_numlock, KEY_CNT); > }; > > @@ -181,6 +180,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, > { > struct apple_sc *asc = hid_get_drvdata(hid); > const struct apple_key_translation *trans, *table; > + bool do_translate; > + u16 code = 0; > > if (usage->code == KEY_FN) { > asc->fn_on = !!value; > @@ -189,8 +190,6 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, > } > > if (fnmode) { > - int do_translate; > - > if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI && > hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS) > table = macbookair_fn_keys; > @@ -202,25 +201,33 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, > trans = apple_find_translation (table, usage->code); > > if (trans) { > - if (test_bit(usage->code, asc->pressed_fn)) > - do_translate = 1; > - else if (trans->flags & APPLE_FLAG_FKEY) > - do_translate = (fnmode == 2 && asc->fn_on) || > - (fnmode == 1 && !asc->fn_on); > - else > - do_translate = asc->fn_on; > - > - if (do_translate) { > - if (value) > - set_bit(usage->code, asc->pressed_fn); > - else > - clear_bit(usage->code, asc->pressed_fn); > - > - input_event(input, usage->type, trans->to, > - value); > - > - return 1; > + if (test_bit(trans->from, input->key)) > + code = trans->from; > + else if (test_bit(trans->to, input->key)) > + code = trans->to; > + > + if (!code) { > + if (trans->flags & APPLE_FLAG_FKEY) { > + switch (fnmode) { > + case 1: > + do_translate = !asc->fn_on; > + break; > + case 2: > + do_translate = asc->fn_on; > + break; > + default: > + /* should never happen */ > + do_translate = false; > + } > + } else { > + do_translate = asc->fn_on; > + } > + > + code = do_translate ? trans->to : trans->from; > } > + > + input_event(input, usage->type, code, value); > + return 1; > } > > if (asc->quirks & APPLE_NUMLOCK_EMULATION && > -- > 2.19.2 >
On Tue, Sep 3, 2019 at 8:33 PM João Moreno <mail@joaomoreno.com> wrote: > > Hi Benjamin, > > On Tue, 3 Sep 2019 at 16:46, Benjamin Tissoires > <benjamin.tissoires@redhat.com> wrote: > > > > From: Joao Moreno <mail@joaomoreno.com> > > > > This fixes an issue in which key down events for function keys would be > > repeatedly emitted even after the user has raised the physical key. For > > example, the driver fails to emit the F5 key up event when going through > > the following steps: > > - fnmode=1: hold FN, hold F5, release FN, release F5 > > - fnmode=2: hold F5, hold FN, release F5, release FN > > > > The repeated F5 key down events can be easily verified using xev. > > > > Signed-off-by: Joao Moreno <mail@joaomoreno.com> > > Co-developed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > > > Hi Joao, > > > > last chance to pull back :) > > > > If you are still happy, I'll push this version > > > > Cheers, > > Benjamin > > > > Looks great. Thanks a bunch for your help! > Thanks. Applied to for-5.4/apple Cheers, Benjamin > Cheers, > João > > > drivers/hid/hid-apple.c | 49 +++++++++++++++++++++++------------------ > > 1 file changed, 28 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c > > index 81df62f48c4c..6ac8becc2372 100644 > > --- a/drivers/hid/hid-apple.c > > +++ b/drivers/hid/hid-apple.c > > @@ -54,7 +54,6 @@ MODULE_PARM_DESC(swap_opt_cmd, "Swap the Option (\"Alt\") and Command (\"Flag\") > > struct apple_sc { > > unsigned long quirks; > > unsigned int fn_on; > > - DECLARE_BITMAP(pressed_fn, KEY_CNT); > > DECLARE_BITMAP(pressed_numlock, KEY_CNT); > > }; > > > > @@ -181,6 +180,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, > > { > > struct apple_sc *asc = hid_get_drvdata(hid); > > const struct apple_key_translation *trans, *table; > > + bool do_translate; > > + u16 code = 0; > > > > if (usage->code == KEY_FN) { > > asc->fn_on = !!value; > > @@ -189,8 +190,6 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, > > } > > > > if (fnmode) { > > - int do_translate; > > - > > if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI && > > hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS) > > table = macbookair_fn_keys; > > @@ -202,25 +201,33 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, > > trans = apple_find_translation (table, usage->code); > > > > if (trans) { > > - if (test_bit(usage->code, asc->pressed_fn)) > > - do_translate = 1; > > - else if (trans->flags & APPLE_FLAG_FKEY) > > - do_translate = (fnmode == 2 && asc->fn_on) || > > - (fnmode == 1 && !asc->fn_on); > > - else > > - do_translate = asc->fn_on; > > - > > - if (do_translate) { > > - if (value) > > - set_bit(usage->code, asc->pressed_fn); > > - else > > - clear_bit(usage->code, asc->pressed_fn); > > - > > - input_event(input, usage->type, trans->to, > > - value); > > - > > - return 1; > > + if (test_bit(trans->from, input->key)) > > + code = trans->from; > > + else if (test_bit(trans->to, input->key)) > > + code = trans->to; > > + > > + if (!code) { > > + if (trans->flags & APPLE_FLAG_FKEY) { > > + switch (fnmode) { > > + case 1: > > + do_translate = !asc->fn_on; > > + break; > > + case 2: > > + do_translate = asc->fn_on; > > + break; > > + default: > > + /* should never happen */ > > + do_translate = false; > > + } > > + } else { > > + do_translate = asc->fn_on; > > + } > > + > > + code = do_translate ? trans->to : trans->from; > > } > > + > > + input_event(input, usage->type, code, value); > > + return 1; > > } > > > > if (asc->quirks & APPLE_NUMLOCK_EMULATION && > > -- > > 2.19.2 > >
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index 81df62f48c4c..6ac8becc2372 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -54,7 +54,6 @@ MODULE_PARM_DESC(swap_opt_cmd, "Swap the Option (\"Alt\") and Command (\"Flag\") struct apple_sc { unsigned long quirks; unsigned int fn_on; - DECLARE_BITMAP(pressed_fn, KEY_CNT); DECLARE_BITMAP(pressed_numlock, KEY_CNT); }; @@ -181,6 +180,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, { struct apple_sc *asc = hid_get_drvdata(hid); const struct apple_key_translation *trans, *table; + bool do_translate; + u16 code = 0; if (usage->code == KEY_FN) { asc->fn_on = !!value; @@ -189,8 +190,6 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, } if (fnmode) { - int do_translate; - if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI && hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS) table = macbookair_fn_keys; @@ -202,25 +201,33 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, trans = apple_find_translation (table, usage->code); if (trans) { - if (test_bit(usage->code, asc->pressed_fn)) - do_translate = 1; - else if (trans->flags & APPLE_FLAG_FKEY) - do_translate = (fnmode == 2 && asc->fn_on) || - (fnmode == 1 && !asc->fn_on); - else - do_translate = asc->fn_on; - - if (do_translate) { - if (value) - set_bit(usage->code, asc->pressed_fn); - else - clear_bit(usage->code, asc->pressed_fn); - - input_event(input, usage->type, trans->to, - value); - - return 1; + if (test_bit(trans->from, input->key)) + code = trans->from; + else if (test_bit(trans->to, input->key)) + code = trans->to; + + if (!code) { + if (trans->flags & APPLE_FLAG_FKEY) { + switch (fnmode) { + case 1: + do_translate = !asc->fn_on; + break; + case 2: + do_translate = asc->fn_on; + break; + default: + /* should never happen */ + do_translate = false; + } + } else { + do_translate = asc->fn_on; + } + + code = do_translate ? trans->to : trans->from; } + + input_event(input, usage->type, code, value); + return 1; } if (asc->quirks & APPLE_NUMLOCK_EMULATION &&