linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] HID: apple: Fix stuck function keys when using FN
@ 2019-09-03 14:46 Benjamin Tissoires
  2019-09-03 18:33 ` João Moreno
  0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Tissoires @ 2019-09-03 14:46 UTC (permalink / raw)
  To: Joao Moreno, Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires

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

 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] HID: apple: Fix stuck function keys when using FN
  2019-09-03 14:46 [PATCH v2] HID: apple: Fix stuck function keys when using FN Benjamin Tissoires
@ 2019-09-03 18:33 ` João Moreno
  2019-09-04  7:25   ` Benjamin Tissoires
  0 siblings, 1 reply; 3+ messages in thread
From: João Moreno @ 2019-09-03 18:33 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml

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
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] HID: apple: Fix stuck function keys when using FN
  2019-09-03 18:33 ` João Moreno
@ 2019-09-04  7:25   ` Benjamin Tissoires
  0 siblings, 0 replies; 3+ messages in thread
From: Benjamin Tissoires @ 2019-09-04  7:25 UTC (permalink / raw)
  To: João Moreno; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-09-04  7:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 14:46 [PATCH v2] HID: apple: Fix stuck function keys when using FN Benjamin Tissoires
2019-09-03 18:33 ` João Moreno
2019-09-04  7:25   ` Benjamin Tissoires

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).