diff mbox series

HID: apple: Add 2021 magic keyboard FN key mapping

Message ID 20211108125038.125579-1-benjamin@sipsolutions.net (mailing list archive)
State Mainlined
Commit 531cb56972f2773c941499fcfb639cd5128dfb27
Delegated to: Jiri Kosina
Headers show
Series HID: apple: Add 2021 magic keyboard FN key mapping | expand

Commit Message

Benjamin Berg Nov. 8, 2021, 12:50 p.m. UTC 
From: Benjamin Berg <bberg@redhat.com>

The new 2021 apple models have a different FN key assignment. Add a new
translation table and use that for the 2021 magic keyboard.

Signed-off-by: Benjamin Berg <bberg@redhat.com>
---
 drivers/hid/hid-apple.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

Comments

Alex Henrie Dec. 2, 2021, 6:18 a.m. UTC | #1 
On Mon, Nov 8, 2021 at 5:50 AM Benjamin Berg <benjamin@sipsolutions.net> wrote:
>
> +               if (hid->product == USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_2021)
> +                       table = apple2021_fn_keys;

This will have to be updated to cover
USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_2021 and
USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 too.

Other than that, the patch looks good, thanks!

Tested-by: Alex Henrie <alexhenrie24@gmail.com>
José Expósito Jan. 2, 2022, 5:54 p.m. UTC | #2 
Hi,

Thanks a lot for the patch Benjamin.

I tested it on the Magic Keyboard 2021 without fingerprint reader.
ANSI, ISO and JIS versions.

Works as expected a code looks good to me.

As sugested by Alex, I mailed a patch adding support for the
keyboards with fingerprint reader and/or numpad [1].

Tested-by: José Expósito <jose.exposito89@gmail.com>

[1] https://lore.kernel.org/linux-input/20220102175113.174642-1-jose.exposito89@gmail.com/T/
José Expósito Jan. 3, 2022, 6:04 p.m. UTC | #3 
On Sun, Jan 02, 2022 at 06:54:39PM +0100, José Expósito wrote: 
> Works as expected a code looks good to me.

Actually, just a little comment on the code, sorry for the extra email:

> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 2c9c5faa74a9..61dfe983828f 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -70,6 +70,28 @@ struct apple_key_translation {
>  	u8 flags;
> };
>
> +static const struct apple_key_translation apple2021_fn_keys[] = {

Since this driver handles many devices, I think that renaming this
struct to "magic_keyboard_2021_fn_keys" would help to quickly
understand which is the target device.
Janne Grunau Jan. 4, 2022, 10:51 a.m. UTC | #4 
Hej,

On 2022-01-03 19:04:18 +0100, José Expósito wrote:
> On Sun, Jan 02, 2022 at 06:54:39PM +0100, José Expósito wrote: 
> > Works as expected a code looks good to me.
> 
> Actually, just a little comment on the code, sorry for the extra email:
> 
> > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> > index 2c9c5faa74a9..61dfe983828f 100644
> > --- a/drivers/hid/hid-apple.c
> > +++ b/drivers/hid/hid-apple.c
> > @@ -70,6 +70,28 @@ struct apple_key_translation {
> >  	u8 flags;
> > };
> >
> > +static const struct apple_key_translation apple2021_fn_keys[] = {
> 
> Since this driver handles many devices, I think that renaming this
> struct to "magic_keyboard_2021_fn_keys" would help to quickly
> understand which is the target device.

A more generic name will make sense in the future since the Apple 
silicon based MacBooks (except for the 2020 13-inch MacBook Pro) use the 
same mapping. Keyboard and trackpad on those devices use HID over SPI 
and identify themself as "Apple Internal Keyboard / Trackpad".

The HID over SPI low level transport is still work in progress but the 
keyboard works as expected with minimal changes to hid-apple.c.

I can of course just rename the struct in the commit adding support for 
the MacBooks.

best regards

Janne
diff mbox series

Patch

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 2c9c5faa74a9..61dfe983828f 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -70,6 +70,28 @@  struct apple_key_translation {
 	u8 flags;
 };
 
+static const struct apple_key_translation apple2021_fn_keys[] = {
+	{ KEY_BACKSPACE, KEY_DELETE },
+	{ KEY_ENTER,	KEY_INSERT },
+	{ KEY_F1,	KEY_BRIGHTNESSDOWN, APPLE_FLAG_FKEY },
+	{ KEY_F2,	KEY_BRIGHTNESSUP,   APPLE_FLAG_FKEY },
+	{ KEY_F3,	KEY_SCALE,          APPLE_FLAG_FKEY },
+	{ KEY_F4,	KEY_SEARCH,         APPLE_FLAG_FKEY },
+	{ KEY_F5,	KEY_MICMUTE,        APPLE_FLAG_FKEY },
+	{ KEY_F6,	KEY_SLEEP,          APPLE_FLAG_FKEY },
+	{ KEY_F7,	KEY_PREVIOUSSONG,   APPLE_FLAG_FKEY },
+	{ KEY_F8,	KEY_PLAYPAUSE,      APPLE_FLAG_FKEY },
+	{ KEY_F9,	KEY_NEXTSONG,       APPLE_FLAG_FKEY },
+	{ KEY_F10,	KEY_MUTE,           APPLE_FLAG_FKEY },
+	{ KEY_F11,	KEY_VOLUMEDOWN,     APPLE_FLAG_FKEY },
+	{ KEY_F12,	KEY_VOLUMEUP,       APPLE_FLAG_FKEY },
+	{ KEY_UP,	KEY_PAGEUP },
+	{ KEY_DOWN,	KEY_PAGEDOWN },
+	{ KEY_LEFT,	KEY_HOME },
+	{ KEY_RIGHT,	KEY_END },
+	{ }
+};
+
 static const struct apple_key_translation macbookair_fn_keys[] = {
 	{ KEY_BACKSPACE, KEY_DELETE },
 	{ KEY_ENTER,	KEY_INSERT },
@@ -214,7 +236,9 @@  static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 	}
 
 	if (fnmode) {
-		if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI &&
+		if (hid->product == USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_2021)
+			table = apple2021_fn_keys;
+		else if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI &&
 				hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS)
 			table = macbookair_fn_keys;
 		else if (hid->product < 0x21d || hid->product >= 0x300)
@@ -376,6 +400,9 @@  static void apple_setup_input(struct input_dev *input)
 	for (trans = apple_iso_keyboard; trans->from; trans++)
 		set_bit(trans->to, input->keybit);
 
+	for (trans = apple2021_fn_keys; trans->from; trans++)
+		set_bit(trans->to, input->keybit);
+
 	if (swap_fn_leftctrl) {
 		for (trans = swapped_fn_leftctrl_keys; trans->from; trans++)
 			set_bit(trans->to, input->keybit);