diff mbox series

[resend] HID: apple: fix up the F6 key on the Omoton KB066 keyboard

Message ID 20250117061254.196702-1-alexhenrie24@gmail.com (mailing list archive)
State Mainlined
Commit 819083cb6eedcc8495cbf84845877bcc741b93b3
Delegated to: Jiri Kosina
Headers show
Series [resend] HID: apple: fix up the F6 key on the Omoton KB066 keyboard | expand

Commit Message

Alex Henrie Jan. 17, 2025, 6:12 a.m. UTC
The Omoton KB066 is an Apple A1255 keyboard clone (HID product code
05ac:022c). On both keyboards, the F6 key becomes Num Lock when the Fn
key is held. But unlike its Apple exemplar, when the Omoton's F6 key is
pressed without Fn, it sends the usage code 0xC0301 from the reserved
section of the consumer page instead of the standard F6 usage code
0x7003F from the keyboard page. The nonstandard code is translated to
KEY_UNKNOWN and becomes useless on Linux. The Omoton KB066 is a pretty
popular keyboard, judging from its 29,058 reviews on Amazon at time of
writing, so let's account for its quirk to make it more usable.

By the way, it would be nice if we could automatically set fnmode to 0
for Omoton keyboards because they handle the Fn key internally and the
kernel's Fn key handling creates undesirable side effects such as making
F1 and F2 always Brightness Up and Brightness Down in fnmode=1 (the
default) or always F1 and F2 in fnmode=2. Unfortunately I don't think
there's a way to identify Bluetooth keyboards more specifically than the
HID product code which is obviously inaccurate. Users of Omoton
keyboards will just have to set fnmode to 0 manually to get full Fn key
functionality.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/hid/hid-apple.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jiri Kosina Feb. 3, 2025, 9:57 p.m. UTC | #1
On Thu, 16 Jan 2025, Alex Henrie wrote:

> The Omoton KB066 is an Apple A1255 keyboard clone (HID product code
> 05ac:022c). On both keyboards, the F6 key becomes Num Lock when the Fn
> key is held. But unlike its Apple exemplar, when the Omoton's F6 key is
> pressed without Fn, it sends the usage code 0xC0301 from the reserved
> section of the consumer page instead of the standard F6 usage code
> 0x7003F from the keyboard page. The nonstandard code is translated to
> KEY_UNKNOWN and becomes useless on Linux. The Omoton KB066 is a pretty
> popular keyboard, judging from its 29,058 reviews on Amazon at time of
> writing, so let's account for its quirk to make it more usable.
> 
> By the way, it would be nice if we could automatically set fnmode to 0
> for Omoton keyboards because they handle the Fn key internally and the
> kernel's Fn key handling creates undesirable side effects such as making
> F1 and F2 always Brightness Up and Brightness Down in fnmode=1 (the
> default) or always F1 and F2 in fnmode=2. Unfortunately I don't think
> there's a way to identify Bluetooth keyboards more specifically than the
> HID product code which is obviously inaccurate. Users of Omoton
> keyboards will just have to set fnmode to 0 manually to get full Fn key
> functionality.
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  drivers/hid/hid-apple.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 7e1ae2a2bcc2..9767d17941d0 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -545,6 +545,9 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
>  		}
>  	}
>  
> +	if (usage->hid == 0xc0301) /* Omoton KB066 quirk */
> +		code = KEY_F6;
> +

Sorry, it's not clear to me from the changelog why it's not possible to 
make this quirk apply only in case we're known to have 0x5ac/0x022c 
device. Is this VID/PID shared with other, differently-behaving devices?

Thanks,
Alex Henrie Feb. 5, 2025, 3:02 a.m. UTC | #2
On Mon, Feb 3, 2025 at 2:57 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Thu, 16 Jan 2025, Alex Henrie wrote:

> > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> > index 7e1ae2a2bcc2..9767d17941d0 100644
> > --- a/drivers/hid/hid-apple.c
> > +++ b/drivers/hid/hid-apple.c
> > @@ -545,6 +545,9 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
> >               }
> >       }
> >
> > +     if (usage->hid == 0xc0301) /* Omoton KB066 quirk */
> > +             code = KEY_F6;
> > +
>
> Sorry, it's not clear to me from the changelog why it's not possible to
> make this quirk apply only in case we're known to have 0x5ac/0x022c
> device. Is this VID/PID shared with other, differently-behaving devices?

The real Apple A1255 and the Omoton KB066 are both 05ac:022c. The
hid-apple driver is only used for devices that have VID 05ac, so
there's no need to check the VID again. We could restrict the quirk to
PID 022c only, but if the keyboard emits the reserved code 0xC0301,
it's surely an Omoton KB066 because no other keyboard uses that code.
What would be the advantage to adding a PID check?

-Alex
Jiri Kosina Feb. 7, 2025, 1:07 p.m. UTC | #3
On Thu, 16 Jan 2025, Alex Henrie wrote:

> The Omoton KB066 is an Apple A1255 keyboard clone (HID product code
> 05ac:022c). On both keyboards, the F6 key becomes Num Lock when the Fn
> key is held. But unlike its Apple exemplar, when the Omoton's F6 key is
> pressed without Fn, it sends the usage code 0xC0301 from the reserved
> section of the consumer page instead of the standard F6 usage code
> 0x7003F from the keyboard page. The nonstandard code is translated to
> KEY_UNKNOWN and becomes useless on Linux. The Omoton KB066 is a pretty
> popular keyboard, judging from its 29,058 reviews on Amazon at time of
> writing, so let's account for its quirk to make it more usable.
> 
> By the way, it would be nice if we could automatically set fnmode to 0
> for Omoton keyboards because they handle the Fn key internally and the
> kernel's Fn key handling creates undesirable side effects such as making
> F1 and F2 always Brightness Up and Brightness Down in fnmode=1 (the
> default) or always F1 and F2 in fnmode=2. Unfortunately I don't think
> there's a way to identify Bluetooth keyboards more specifically than the
> HID product code which is obviously inaccurate. Users of Omoton
> keyboards will just have to set fnmode to 0 manually to get full Fn key
> functionality.
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Now applied to hid.git#for-6.14/upstream-fixes, thanks.
Aditya Garg Feb. 12, 2025, 5:36 p.m. UTC | #4
Hi Alex

> On 17 Jan 2025, at 11:42 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
> 
> The Omoton KB066 is an Apple A1255 keyboard clone (HID product code
> 05ac:022c). On both keyboards, the F6 key becomes Num Lock when the Fn
> key is held. But unlike its Apple exemplar, when the Omoton's F6 key is
> pressed without Fn, it sends the usage code 0xC0301 from the reserved
> section of the consumer page instead of the standard F6 usage code
> 0x7003F from the keyboard page. The nonstandard code is translated to
> KEY_UNKNOWN and becomes useless on Linux. The Omoton KB066 is a pretty
> popular keyboard, judging from its 29,058 reviews on Amazon at time of
> writing, so let's account for its quirk to make it more usable.
> 
> By the way, it would be nice if we could automatically set fnmode to 0
> for Omoton keyboards because they handle the Fn key internally and the
> kernel's Fn key handling creates undesirable side effects such as making
> F1 and F2 always Brightness Up and Brightness Down in fnmode=1 (the
> default) or always F1 and F2 in fnmode=2. Unfortunately I don't think
> there's a way to identify Bluetooth keyboards more specifically than the
> HID product code which is obviously inaccurate. Users of Omoton
> keyboards will just have to set fnmode to 0 manually to get full Fn key
> functionality.

Regarding the the fnmode=0 thing, could you test this patch:

-->8—
From e2b2ef3f579800f11ee293fb45838a4004ccaf23 Mon Sep 17 00:00:00 2001
From: Aditya Garg <gargaditya08@live.com>
Date: Wed, 12 Feb 2025 22:57:58 +0530
Subject: [PATCH] HID: apple: Add quirk to disable fn key on some non-apple
 keyboards

---
 drivers/hid/hid-apple.c | 54 +++++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 49812a76b..9d4cbe636 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -42,6 +42,7 @@
 #define APPLE_BACKLIGHT_CTL	BIT(10)
 #define APPLE_IS_NON_APPLE	BIT(11)
 #define APPLE_MAGIC_BACKLIGHT	BIT(12)
+#define APPLE_DISABLE_FN	BIT(13)
 
 #define APPLE_FLAG_FKEY		0x01
 
@@ -89,6 +90,19 @@ struct apple_sc_backlight {
 	struct hid_device *hdev;
 };
 
+struct apple_backlight_config_report {
+	u8 report_id;
+	u8 version;
+	u16 backlight_off, backlight_on_min, backlight_on_max;
+};
+
+struct apple_backlight_set_report {
+	u8 report_id;
+	u8 version;
+	u16 backlight;
+	u16 rate;
+};
+
 struct apple_magic_backlight {
 	struct led_classdev cdev;
 	struct hid_report *brightness;
@@ -152,20 +166,6 @@ static const struct apple_key_translation magic_keyboard_2015_fn_keys[] = {
 	{ }
 };
 
-struct apple_backlight_config_report {
-	u8 report_id;
-	u8 version;
-	u16 backlight_off, backlight_on_min, backlight_on_max;
-};
-
-struct apple_backlight_set_report {
-	u8 report_id;
-	u8 version;
-	u16 backlight;
-	u16 rate;
-};
-
-
 static const struct apple_key_translation apple2021_fn_keys[] = {
 	{ KEY_BACKSPACE, KEY_DELETE },
 	{ KEY_ENTER,	KEY_INSERT },
@@ -364,6 +364,10 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
 	{ "WKB603" },
 };
 
+static const struct apple_non_apple_keyboard non_apple_keyboards_disable_fn[] = {
+	{ "Omoton" },
+};
+
 static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
 {
 	int i;
@@ -378,6 +382,20 @@ static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
 	return false;
 }
 
+static bool apple_disable_fn_key(struct hid_device *hdev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(non_apple_keyboards_disable_fn); i++) {
+		char *non_apple = non_apple_keyboards_disable_fn[i].name;
+
+		if (strncmp(hdev->name, non_apple, strlen(non_apple)) == 0)
+			return true;
+	}
+
+	return false;
+}
+
 static inline void apple_setup_key_translation(struct input_dev *input,
 		const struct apple_key_translation *table)
 {
@@ -419,7 +437,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 	unsigned int real_fnmode;
 
 	if (fnmode == 3) {
-		real_fnmode = (asc->quirks & APPLE_IS_NON_APPLE) ? 2 : 1;
+		real_fnmode = (asc->quirks & APPLE_DISABLE_FN) ? 0 :
+			      ((asc->quirks & APPLE_IS_NON_APPLE) ? 2 : 1);
 	} else {
 		real_fnmode = fnmode;
 	}
@@ -738,6 +757,11 @@ static int apple_input_configured(struct hid_device *hdev,
 		asc->quirks |= APPLE_IS_NON_APPLE;
 	}
 
+	if (apple_disable_fn_key(hdev)) {
+		hid_info(hdev, "Disable fn key quirk detected; fnmode=0 behaviour will be followed by default\n");
+		asc->quirks |= APPLE_DISABLE_FN;
+	}
+
 	return 0;
 }
Aditya Garg Feb. 12, 2025, 5:43 p.m. UTC | #5
> On 12 Feb 2025, at 11:06 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> Hi Alex
> 
>> On 17 Jan 2025, at 11:42 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
>> 
>> The Omoton KB066 is an Apple A1255 keyboard clone (HID product code
>> 05ac:022c). On both keyboards, the F6 key becomes Num Lock when the Fn
>> key is held. But unlike its Apple exemplar, when the Omoton's F6 key is
>> pressed without Fn, it sends the usage code 0xC0301 from the reserved
>> section of the consumer page instead of the standard F6 usage code
>> 0x7003F from the keyboard page. The nonstandard code is translated to
>> KEY_UNKNOWN and becomes useless on Linux. The Omoton KB066 is a pretty
>> popular keyboard, judging from its 29,058 reviews on Amazon at time of
>> writing, so let's account for its quirk to make it more usable.
>> 
>> By the way, it would be nice if we could automatically set fnmode to 0
>> for Omoton keyboards because they handle the Fn key internally and the
>> kernel's Fn key handling creates undesirable side effects such as making
>> F1 and F2 always Brightness Up and Brightness Down in fnmode=1 (the
>> default) or always F1 and F2 in fnmode=2. Unfortunately I don't think
>> there's a way to identify Bluetooth keyboards more specifically than the
>> HID product code which is obviously inaccurate. Users of Omoton
>> keyboards will just have to set fnmode to 0 manually to get full Fn key
>> functionality.
> 
> Regarding the the fnmode=0 thing, could you test this patch:
> 
> -->8—
> From e2b2ef3f579800f11ee293fb45838a4004ccaf23 Mon Sep 17 00:00:00 2001
> From: Aditya Garg <gargaditya08@live.com>
> Date: Wed, 12 Feb 2025 22:57:58 +0530
> Subject: [PATCH] HID: apple: Add quirk to disable fn key on some non-apple
> keyboards
> 
> ---
> drivers/hid/hid-apple.c | 54 +++++++++++++++++++++++++++++------------
> 1 file changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 49812a76b..9d4cbe636 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -42,6 +42,7 @@
> #define APPLE_BACKLIGHT_CTL BIT(10)
> #define APPLE_IS_NON_APPLE BIT(11)
> #define APPLE_MAGIC_BACKLIGHT BIT(12)
> +#define APPLE_DISABLE_FN BIT(13)
> 
> #define APPLE_FLAG_FKEY 0x01
> 
> @@ -89,6 +90,19 @@ struct apple_sc_backlight {
> struct hid_device *hdev;
> };
> 
> +struct apple_backlight_config_report {
> + u8 report_id;
> + u8 version;
> + u16 backlight_off, backlight_on_min, backlight_on_max;
> +};
> +
> +struct apple_backlight_set_report {
> + u8 report_id;
> + u8 version;
> + u16 backlight;
> + u16 rate;
> +};
> +
> struct apple_magic_backlight {
> struct led_classdev cdev;
> struct hid_report *brightness;
> @@ -152,20 +166,6 @@ static const struct apple_key_translation magic_keyboard_2015_fn_keys[] = {
> { }
> };
> 
> -struct apple_backlight_config_report {
> - u8 report_id;
> - u8 version;
> - u16 backlight_off, backlight_on_min, backlight_on_max;
> -};
> -
> -struct apple_backlight_set_report {
> - u8 report_id;
> - u8 version;
> - u16 backlight;
> - u16 rate;
> -};
> -
> -
> static const struct apple_key_translation apple2021_fn_keys[] = {
> { KEY_BACKSPACE, KEY_DELETE },
> { KEY_ENTER, KEY_INSERT },
> @@ -364,6 +364,10 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
> { "WKB603" },
> };
> 
> +static const struct apple_non_apple_keyboard non_apple_keyboards_disable_fn[] = {
> + { "Omoton" },

You could try replacing Omoton with OMOTON as well here if it does not work. Alternatively, you could try logging hdev->name for this device and put it in this table to get the correct name.
Alex Henrie Feb. 16, 2025, 1:08 a.m. UTC | #6
On Wed, Feb 12, 2025 at 10:44 AM Aditya Garg <gargaditya08@live.com> wrote:

> > On 12 Feb 2025, at 11:06 PM, Aditya Garg <gargaditya08@live.com> wrote:

> >> On 17 Jan 2025, at 11:42 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
> >>
> >> The Omoton KB066 is an Apple A1255 keyboard clone (HID product code
> >> 05ac:022c). On both keyboards, the F6 key becomes Num Lock when the Fn
> >> key is held. But unlike its Apple exemplar, when the Omoton's F6 key is
> >> pressed without Fn, it sends the usage code 0xC0301 from the reserved
> >> section of the consumer page instead of the standard F6 usage code
> >> 0x7003F from the keyboard page. The nonstandard code is translated to
> >> KEY_UNKNOWN and becomes useless on Linux. The Omoton KB066 is a pretty
> >> popular keyboard, judging from its 29,058 reviews on Amazon at time of
> >> writing, so let's account for its quirk to make it more usable.
> >>
> >> By the way, it would be nice if we could automatically set fnmode to 0
> >> for Omoton keyboards because they handle the Fn key internally and the
> >> kernel's Fn key handling creates undesirable side effects such as making
> >> F1 and F2 always Brightness Up and Brightness Down in fnmode=1 (the
> >> default) or always F1 and F2 in fnmode=2. Unfortunately I don't think
> >> there's a way to identify Bluetooth keyboards more specifically than the
> >> HID product code which is obviously inaccurate. Users of Omoton
> >> keyboards will just have to set fnmode to 0 manually to get full Fn key
> >> functionality.
> >
> > Regarding the the fnmode=0 thing, could you test this patch:
> >
> > -->8—
> > From e2b2ef3f579800f11ee293fb45838a4004ccaf23 Mon Sep 17 00:00:00 2001
> > From: Aditya Garg <gargaditya08@live.com>
> > Date: Wed, 12 Feb 2025 22:57:58 +0530
> > Subject: [PATCH] HID: apple: Add quirk to disable fn key on some non-apple
> > keyboards
> >
> > ---
> > drivers/hid/hid-apple.c | 54 +++++++++++++++++++++++++++++------------
> > 1 file changed, 39 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> > index 49812a76b..9d4cbe636 100644
> > --- a/drivers/hid/hid-apple.c
> > +++ b/drivers/hid/hid-apple.c
> > @@ -42,6 +42,7 @@
> > #define APPLE_BACKLIGHT_CTL BIT(10)
> > #define APPLE_IS_NON_APPLE BIT(11)
> > #define APPLE_MAGIC_BACKLIGHT BIT(12)
> > +#define APPLE_DISABLE_FN BIT(13)
> >
> > #define APPLE_FLAG_FKEY 0x01
> >
> > @@ -89,6 +90,19 @@ struct apple_sc_backlight {
> > struct hid_device *hdev;
> > };
> >
> > +struct apple_backlight_config_report {
> > + u8 report_id;
> > + u8 version;
> > + u16 backlight_off, backlight_on_min, backlight_on_max;
> > +};
> > +
> > +struct apple_backlight_set_report {
> > + u8 report_id;
> > + u8 version;
> > + u16 backlight;
> > + u16 rate;
> > +};
> > +
> > struct apple_magic_backlight {
> > struct led_classdev cdev;
> > struct hid_report *brightness;
> > @@ -152,20 +166,6 @@ static const struct apple_key_translation magic_keyboard_2015_fn_keys[] = {
> > { }
> > };
> >
> > -struct apple_backlight_config_report {
> > - u8 report_id;
> > - u8 version;
> > - u16 backlight_off, backlight_on_min, backlight_on_max;
> > -};
> > -
> > -struct apple_backlight_set_report {
> > - u8 report_id;
> > - u8 version;
> > - u16 backlight;
> > - u16 rate;
> > -};
> > -
> > -
> > static const struct apple_key_translation apple2021_fn_keys[] = {
> > { KEY_BACKSPACE, KEY_DELETE },
> > { KEY_ENTER, KEY_INSERT },
> > @@ -364,6 +364,10 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
> > { "WKB603" },
> > };
> >
> > +static const struct apple_non_apple_keyboard non_apple_keyboards_disable_fn[] = {
> > + { "Omoton" },
>
> You could try replacing Omoton with OMOTON as well here if it does not work. Alternatively, you could try logging hdev->name for this device and put it in this table to get the correct name.

On the Omoton KB066, hdev->name is "Bluetooth Keyboard". I think I saw
that name in Blueman and assumed that it was just a default for when a
keyboard has no name, but you're right, that string must be coming
from the keyboard itself. After changing "Omoton" to "Bluetooth
Keyboard" in non_apple_keyboards_disable_fn, your patch works!

Unfortunately, this keyboard is even more messed up than I realized.
It is indeed sending different key codes depending on whether or not
the Fn key is held. For example, F11 and F12 are Volume Down and
Volume Up by default, but ordinary F11 and F12 when Fn is held.
However, I was wrong about F1 and F2: With or without Fn, the Omoton
_never_ changes F1 to Brightness Down or F2 to Brightness Up. I
managed to pick the one bad example: All of the other special keys
(Esc, F3 through F12, and Del) are translated internally, but not F1
or F2. That also means that I was wrong about the Fn key not doing
anything in fnmode=2. When Fn is held, all of the special keys do
become ordinary keys in both fnmode=0 and fnmode=2, except for F1 and
F2 which remain ordinary F1 and F2.

When connected to macOS, the results are similar: All of the special
keys are special keys regardless of whether Fn is held, except F1 and
F2, which by default are always translated to Brightness Down and
Brightness Up like in Linux's fnmode=1. If the setting "Use F1, F2,
etc. keys as standard function keys" is enabled in the system keyboard
settings, all of the special keys change to ordinary keys when Fn is
held, except for F1 and F2 which remain F1 and F2, just like in
Linux's fnmode=0 and fnmode=2.

It seems to me that the best way to support the Omoton KB066 would be
to give it its own key translation table that has F1 and F2 and
nothing else. But on the bright side, pun humorously intended, we
wouldn't have to change the default fnmode. Just like on macOS, F1 and
F2 would always be Brightness Down and Brightness Up, unless key
translation is disabled. But unlike macOS (and the current state of
Linux), we wouldn't undo the keyboard's own Fn key handling for the
special keys that it translates internally.

Circling back to the original problem of distinguishing between the
Apple A1255 and the Omoton KB066, changing an Apple keyboard's name in
the macOS keyboard settings also changes the name that it reports to
Linux. Because "Bluetooth Keyboard" is so generic that someone might
actually give their keyboard that name intentionally, if we add
special behavior to look for that name, I think we should restrict it
to PID 022c. For example, we could add the new quirk flag to
USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI in the apple_devices table and
then clear the flag if the keyboard's name is not "Bluetooth
Keyboard".

Thanks for the help!

-Alex
Aditya Garg Feb. 16, 2025, 6:06 a.m. UTC | #7
So if I understand correctly

If Fn mode is 0 or 2:

F1 and F2 are brightness keys
Rest are function keys

If Fn mode is 1:

F1 and F2 are function keys
Rest are media keys

Fn key does not work in any mode

In case I am wrong, can you share what exactly happens in each mode with fn on as well as off?


> On 16 Feb 2025, at 6:38 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
> 
> On Wed, Feb 12, 2025 at 10:44 AM Aditya Garg <gargaditya08@live.com> wrote:
> 
>>> On 12 Feb 2025, at 11:06 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
>>>> On 17 Jan 2025, at 11:42 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
>>>> 
>>>> The Omoton KB066 is an Apple A1255 keyboard clone (HID product code
>>>> 05ac:022c). On both keyboards, the F6 key becomes Num Lock when the Fn
>>>> key is held. But unlike its Apple exemplar, when the Omoton's F6 key is
>>>> pressed without Fn, it sends the usage code 0xC0301 from the reserved
>>>> section of the consumer page instead of the standard F6 usage code
>>>> 0x7003F from the keyboard page. The nonstandard code is translated to
>>>> KEY_UNKNOWN and becomes useless on Linux. The Omoton KB066 is a pretty
>>>> popular keyboard, judging from its 29,058 reviews on Amazon at time of
>>>> writing, so let's account for its quirk to make it more usable.
>>>> 
>>>> By the way, it would be nice if we could automatically set fnmode to 0
>>>> for Omoton keyboards because they handle the Fn key internally and the
>>>> kernel's Fn key handling creates undesirable side effects such as making
>>>> F1 and F2 always Brightness Up and Brightness Down in fnmode=1 (the
>>>> default) or always F1 and F2 in fnmode=2. Unfortunately I don't think
>>>> there's a way to identify Bluetooth keyboards more specifically than the
>>>> HID product code which is obviously inaccurate. Users of Omoton
>>>> keyboards will just have to set fnmode to 0 manually to get full Fn key
>>>> functionality.
>>> 
>>> Regarding the the fnmode=0 thing, could you test this patch:
>>> 
>>> -->8—
>>> From e2b2ef3f579800f11ee293fb45838a4004ccaf23 Mon Sep 17 00:00:00 2001
>>> From: Aditya Garg <gargaditya08@live.com>
>>> Date: Wed, 12 Feb 2025 22:57:58 +0530
>>> Subject: [PATCH] HID: apple: Add quirk to disable fn key on some non-apple
>>> keyboards
>>> 
>>> ---
>>> drivers/hid/hid-apple.c | 54 +++++++++++++++++++++++++++++------------
>>> 1 file changed, 39 insertions(+), 15 deletions(-)
>>> 
>>> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
>>> index 49812a76b..9d4cbe636 100644
>>> --- a/drivers/hid/hid-apple.c
>>> +++ b/drivers/hid/hid-apple.c
>>> @@ -42,6 +42,7 @@
>>> #define APPLE_BACKLIGHT_CTL BIT(10)
>>> #define APPLE_IS_NON_APPLE BIT(11)
>>> #define APPLE_MAGIC_BACKLIGHT BIT(12)
>>> +#define APPLE_DISABLE_FN BIT(13)
>>> 
>>> #define APPLE_FLAG_FKEY 0x01
>>> 
>>> @@ -89,6 +90,19 @@ struct apple_sc_backlight {
>>> struct hid_device *hdev;
>>> };
>>> 
>>> +struct apple_backlight_config_report {
>>> + u8 report_id;
>>> + u8 version;
>>> + u16 backlight_off, backlight_on_min, backlight_on_max;
>>> +};
>>> +
>>> +struct apple_backlight_set_report {
>>> + u8 report_id;
>>> + u8 version;
>>> + u16 backlight;
>>> + u16 rate;
>>> +};
>>> +
>>> struct apple_magic_backlight {
>>> struct led_classdev cdev;
>>> struct hid_report *brightness;
>>> @@ -152,20 +166,6 @@ static const struct apple_key_translation magic_keyboard_2015_fn_keys[] = {
>>> { }
>>> };
>>> 
>>> -struct apple_backlight_config_report {
>>> - u8 report_id;
>>> - u8 version;
>>> - u16 backlight_off, backlight_on_min, backlight_on_max;
>>> -};
>>> -
>>> -struct apple_backlight_set_report {
>>> - u8 report_id;
>>> - u8 version;
>>> - u16 backlight;
>>> - u16 rate;
>>> -};
>>> -
>>> -
>>> static const struct apple_key_translation apple2021_fn_keys[] = {
>>> { KEY_BACKSPACE, KEY_DELETE },
>>> { KEY_ENTER, KEY_INSERT },
>>> @@ -364,6 +364,10 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
>>> { "WKB603" },
>>> };
>>> 
>>> +static const struct apple_non_apple_keyboard non_apple_keyboards_disable_fn[] = {
>>> + { "Omoton" },
>> 
>> You could try replacing Omoton with OMOTON as well here if it does not work. Alternatively, you could try logging hdev->name for this device and put it in this table to get the correct name.
> 
> On the Omoton KB066, hdev->name is "Bluetooth Keyboard". I think I saw
> that name in Blueman and assumed that it was just a default for when a
> keyboard has no name, but you're right, that string must be coming
> from the keyboard itself. After changing "Omoton" to "Bluetooth
> Keyboard" in non_apple_keyboards_disable_fn, your patch works!
> 
> Unfortunately, this keyboard is even more messed up than I realized.
> It is indeed sending different key codes depending on whether or not
> the Fn key is held. For example, F11 and F12 are Volume Down and
> Volume Up by default, but ordinary F11 and F12 when Fn is held.
> However, I was wrong about F1 and F2: With or without Fn, the Omoton
> _never_ changes F1 to Brightness Down or F2 to Brightness Up. I
> managed to pick the one bad example: All of the other special keys
> (Esc, F3 through F12, and Del) are translated internally, but not F1
> or F2. That also means that I was wrong about the Fn key not doing
> anything in fnmode=2. When Fn is held, all of the special keys do
> become ordinary keys in both fnmode=0 and fnmode=2, except for F1 and
> F2 which remain ordinary F1 and F2.
> 
> When connected to macOS, the results are similar: All of the special
> keys are special keys regardless of whether Fn is held, except F1 and
> F2, which by default are always translated to Brightness Down and
> Brightness Up like in Linux's fnmode=1. If the setting "Use F1, F2,
> etc. keys as standard function keys" is enabled in the system keyboard
> settings, all of the special keys change to ordinary keys when Fn is
> held, except for F1 and F2 which remain F1 and F2, just like in
> Linux's fnmode=0 and fnmode=2.
> 
> It seems to me that the best way to support the Omoton KB066 would be
> to give it its own key translation table that has F1 and F2 and
> nothing else. But on the bright side, pun humorously intended, we
> wouldn't have to change the default fnmode. Just like on macOS, F1 and
> F2 would always be Brightness Down and Brightness Up, unless key
> translation is disabled. But unlike macOS (and the current state of
> Linux), we wouldn't undo the keyboard's own Fn key handling for the
> special keys that it translates internally.
> 
> Circling back to the original problem of distinguishing between the
> Apple A1255 and the Omoton KB066, changing an Apple keyboard's name in
> the macOS keyboard settings also changes the name that it reports to
> Linux. Because "Bluetooth Keyboard" is so generic that someone might
> actually give their keyboard that name intentionally, if we add
> special behavior to look for that name, I think we should restrict it
> to PID 022c. For example, we could add the new quirk flag to
> USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI in the apple_devices table and
> then clear the flag if the keyboard's name is not "Bluetooth
> Keyboard".
> 
> Thanks for the help!
> 
> -Alex
Aditya Garg Feb. 16, 2025, 6:45 a.m. UTC | #8
I think its best to disable the internal translation of this keyboard and let the kernel manage it. It can be done by implementing a fixup table that first translates all the media controls to their respective F keys, and other similar internal translations, so that the keyboard can mimic the original Apple version. Like the all the 3 fn modes should also work.

Also looking at the keyboard pic on https://www.amazon.in/OMOTON-Ultra-Slim-Bluetooth-Keyboard-Compatible/dp/B07S7VPQG6?th=1, the translation table for magic keyboard aluminium seems quite different from what this keyboard keys show.

> On 16 Feb 2025, at 11:36 AM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> So if I understand correctly
> 
> If Fn mode is 0 or 2:
> 
> F1 and F2 are brightness keys
> Rest are function keys
> 
> If Fn mode is 1:
> 
> F1 and F2 are function keys
> Rest are media keys
> 
> Fn key does not work in any mode
> 
> In case I am wrong, can you share what exactly happens in each mode with fn on as well as off?
> 
> 
>> On 16 Feb 2025, at 6:38 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
>> 
>> On Wed, Feb 12, 2025 at 10:44 AM Aditya Garg <gargaditya08@live.com> wrote:
>> 
>>>> On 12 Feb 2025, at 11:06 PM, Aditya Garg <gargaditya08@live.com> wrote:
>> 
>>>>> On 17 Jan 2025, at 11:42 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
>>>>> 
>>>>> The Omoton KB066 is an Apple A1255 keyboard clone (HID product code
>>>>> 05ac:022c). On both keyboards, the F6 key becomes Num Lock when the Fn
>>>>> key is held. But unlike its Apple exemplar, when the Omoton's F6 key is
>>>>> pressed without Fn, it sends the usage code 0xC0301 from the reserved
>>>>> section of the consumer page instead of the standard F6 usage code
>>>>> 0x7003F from the keyboard page. The nonstandard code is translated to
>>>>> KEY_UNKNOWN and becomes useless on Linux. The Omoton KB066 is a pretty
>>>>> popular keyboard, judging from its 29,058 reviews on Amazon at time of
>>>>> writing, so let's account for its quirk to make it more usable.
>>>>> 
>>>>> By the way, it would be nice if we could automatically set fnmode to 0
>>>>> for Omoton keyboards because they handle the Fn key internally and the
>>>>> kernel's Fn key handling creates undesirable side effects such as making
>>>>> F1 and F2 always Brightness Up and Brightness Down in fnmode=1 (the
>>>>> default) or always F1 and F2 in fnmode=2. Unfortunately I don't think
>>>>> there's a way to identify Bluetooth keyboards more specifically than the
>>>>> HID product code which is obviously inaccurate. Users of Omoton
>>>>> keyboards will just have to set fnmode to 0 manually to get full Fn key
>>>>> functionality.
>>>> 
>>>> Regarding the the fnmode=0 thing, could you test this patch:
>>>> 
>>>> -->8—
>>>> From e2b2ef3f579800f11ee293fb45838a4004ccaf23 Mon Sep 17 00:00:00 2001
>>>> From: Aditya Garg <gargaditya08@live.com>
>>>> Date: Wed, 12 Feb 2025 22:57:58 +0530
>>>> Subject: [PATCH] HID: apple: Add quirk to disable fn key on some non-apple
>>>> keyboards
>>>> 
>>>> ---
>>>> drivers/hid/hid-apple.c | 54 +++++++++++++++++++++++++++++------------
>>>> 1 file changed, 39 insertions(+), 15 deletions(-)
>>>> 
>>>> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
>>>> index 49812a76b..9d4cbe636 100644
>>>> --- a/drivers/hid/hid-apple.c
>>>> +++ b/drivers/hid/hid-apple.c
>>>> @@ -42,6 +42,7 @@
>>>> #define APPLE_BACKLIGHT_CTL BIT(10)
>>>> #define APPLE_IS_NON_APPLE BIT(11)
>>>> #define APPLE_MAGIC_BACKLIGHT BIT(12)
>>>> +#define APPLE_DISABLE_FN BIT(13)
>>>> 
>>>> #define APPLE_FLAG_FKEY 0x01
>>>> 
>>>> @@ -89,6 +90,19 @@ struct apple_sc_backlight {
>>>> struct hid_device *hdev;
>>>> };
>>>> 
>>>> +struct apple_backlight_config_report {
>>>> + u8 report_id;
>>>> + u8 version;
>>>> + u16 backlight_off, backlight_on_min, backlight_on_max;
>>>> +};
>>>> +
>>>> +struct apple_backlight_set_report {
>>>> + u8 report_id;
>>>> + u8 version;
>>>> + u16 backlight;
>>>> + u16 rate;
>>>> +};
>>>> +
>>>> struct apple_magic_backlight {
>>>> struct led_classdev cdev;
>>>> struct hid_report *brightness;
>>>> @@ -152,20 +166,6 @@ static const struct apple_key_translation magic_keyboard_2015_fn_keys[] = {
>>>> { }
>>>> };
>>>> 
>>>> -struct apple_backlight_config_report {
>>>> - u8 report_id;
>>>> - u8 version;
>>>> - u16 backlight_off, backlight_on_min, backlight_on_max;
>>>> -};
>>>> -
>>>> -struct apple_backlight_set_report {
>>>> - u8 report_id;
>>>> - u8 version;
>>>> - u16 backlight;
>>>> - u16 rate;
>>>> -};
>>>> -
>>>> -
>>>> static const struct apple_key_translation apple2021_fn_keys[] = {
>>>> { KEY_BACKSPACE, KEY_DELETE },
>>>> { KEY_ENTER, KEY_INSERT },
>>>> @@ -364,6 +364,10 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
>>>> { "WKB603" },
>>>> };
>>>> 
>>>> +static const struct apple_non_apple_keyboard non_apple_keyboards_disable_fn[] = {
>>>> + { "Omoton" },
>>> 
>>> You could try replacing Omoton with OMOTON as well here if it does not work. Alternatively, you could try logging hdev->name for this device and put it in this table to get the correct name.
>> 
>> On the Omoton KB066, hdev->name is "Bluetooth Keyboard". I think I saw
>> that name in Blueman and assumed that it was just a default for when a
>> keyboard has no name, but you're right, that string must be coming
>> from the keyboard itself. After changing "Omoton" to "Bluetooth
>> Keyboard" in non_apple_keyboards_disable_fn, your patch works!
>> 
>> Unfortunately, this keyboard is even more messed up than I realized.
>> It is indeed sending different key codes depending on whether or not
>> the Fn key is held. For example, F11 and F12 are Volume Down and
>> Volume Up by default, but ordinary F11 and F12 when Fn is held.
>> However, I was wrong about F1 and F2: With or without Fn, the Omoton
>> _never_ changes F1 to Brightness Down or F2 to Brightness Up. I
>> managed to pick the one bad example: All of the other special keys
>> (Esc, F3 through F12, and Del) are translated internally, but not F1
>> or F2. That also means that I was wrong about the Fn key not doing
>> anything in fnmode=2. When Fn is held, all of the special keys do
>> become ordinary keys in both fnmode=0 and fnmode=2, except for F1 and
>> F2 which remain ordinary F1 and F2.
>> 
>> When connected to macOS, the results are similar: All of the special
>> keys are special keys regardless of whether Fn is held, except F1 and
>> F2, which by default are always translated to Brightness Down and
>> Brightness Up like in Linux's fnmode=1. If the setting "Use F1, F2,
>> etc. keys as standard function keys" is enabled in the system keyboard
>> settings, all of the special keys change to ordinary keys when Fn is
>> held, except for F1 and F2 which remain F1 and F2, just like in
>> Linux's fnmode=0 and fnmode=2.
>> 
>> It seems to me that the best way to support the Omoton KB066 would be
>> to give it its own key translation table that has F1 and F2 and
>> nothing else. But on the bright side, pun humorously intended, we
>> wouldn't have to change the default fnmode. Just like on macOS, F1 and
>> F2 would always be Brightness Down and Brightness Up, unless key
>> translation is disabled. But unlike macOS (and the current state of
>> Linux), we wouldn't undo the keyboard's own Fn key handling for the
>> special keys that it translates internally.
>> 
>> Circling back to the original problem of distinguishing between the
>> Apple A1255 and the Omoton KB066, changing an Apple keyboard's name in
>> the macOS keyboard settings also changes the name that it reports to
>> Linux. Because "Bluetooth Keyboard" is so generic that someone might
>> actually give their keyboard that name intentionally, if we add
>> special behavior to look for that name, I think we should restrict it
>> to PID 022c. For example, we could add the new quirk flag to
>> USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI in the apple_devices table and
>> then clear the flag if the keyboard's name is not "Bluetooth
>> Keyboard".
>> 
>> Thanks for the help!
>> 
>> -Alex
> 
>
Alex Henrie Feb. 17, 2025, 4:13 a.m. UTC | #9
On Sat, Feb 15, 2025 at 11:06 PM Aditya Garg <gargaditya08@live.com> wrote:

> In case I am wrong, can you share what exactly happens in each mode with fn on as well as off?

In fnmode=0 and fnmode=2, F1 and F2 are F1 and F2, and the rest are
special keys when Fn is not held and ordinary function keys when Fn is
held.

In fnmode=1, F1 and F2 are Brightness Down and Brightness Up, and the
rest are always special keys, although holding Fn changes some of the
special keys to different special keys.

In all modes, Home becomes Escape when Fn is held, Lock (which is
actually Power) becomes Delete when Fn is held, and F6 always sends a
reserved key code.

On Sat, Feb 15, 2025 at 11:45 PM Aditya Garg <gargaditya08@live.com> wrote:
>
> I think its best to disable the internal translation of this keyboard and let the kernel manage it. It can be done by implementing a fixup table that first translates all the media controls to their respective F keys, and other similar internal translations, so that the keyboard can mimic the original Apple version. Like the all the 3 fn modes should also work.

The trouble is, we have no way to read the state of the Omoton's Fn
key in software. The Fn key is entirely internal to the keyboard. I
even looked at the raw HID reports with and without Fn pressed, and
there is nothing. So either we translate F1 and F2 to Brightness Down
and Brightness Up (in fnmode=1) or we don't translate them at all (in
fnmode=0 and fnmode=2); we can't conditionally translate depending on
the Fn key.

But for all the other special keys, what you are saying makes sense
and is a good idea: In fnmode=0, we can translate all of the special
keys to be ordinary keys, and in fnmode=2, we can translate them to
their opposites. For example, if the keyboard sends Volume Down, in
fnmode=0 and fnmode=2 we'd translate it to F11 (because the keyboard
only sends Volume Down when Fn is not held), and if it sends F11, in
fnmode=2 we'd translate it to Volume Down (because the keyboard only
sends F11 when Fn is held).

> Also looking at the keyboard pic on https://www.amazon.in/OMOTON-Ultra-Slim-Bluetooth-Keyboard-Compatible/dp/B07S7VPQG6?th=1, the translation table for magic keyboard aluminium seems quite different from what this keyboard keys show.

Let's please not change the special keys to be different from what
their labels show. For example, F3 is Search on the Omoton KB066 and
Scale on the Apple A1255. Personally I wouldn't even change Power to
Screen Lock to match its icon, although I'm okay with changing it if
others feel strongly.

-Alex
Aditya Garg Feb. 17, 2025, 5:18 a.m. UTC | #10
> On 17 Feb 2025, at 9:43 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
> 
> On Sat, Feb 15, 2025 at 11:06 PM Aditya Garg <gargaditya08@live.com> wrote:
> 
>> In case I am wrong, can you share what exactly happens in each mode with fn on as well as off?
> 
> In fnmode=0 and fnmode=2, F1 and F2 are F1 and F2, and the rest are
> special keys when Fn is not held and ordinary function keys when Fn is
> held.
> 
> In fnmode=1, F1 and F2 are Brightness Down and Brightness Up, and the
> rest are always special keys, although holding Fn changes some of the
> special keys to different special keys.
> 
> In all modes, Home becomes Escape when Fn is held, Lock (which is
> actually Power) becomes Delete when Fn is held, and F6 always sends a
> reserved key code.
> 

So judging from these observations:

1. Fn key is internal to the keyboard, and does not send KEY_FN like Apple Keyboards
2. F1 and F2 are broken as far as Fn translation is considered.
3. Since fnmode=0 shows the actual state of the keyboard, internal translation of rest keys is just fine.

This makes it impossible to fixup this keyboard since Fn is not there in raw HID reports.

We can then just make fnmode=0 as default for this keyboard, and optionally translate F1 and F2 permanently to media keys.

BTW, are there any differences in HID reports if Fn+F1 and only F1 are pressed?

> On Sat, Feb 15, 2025 at 11:45 PM Aditya Garg <gargaditya08@live.com> wrote:
>> 
>> I think its best to disable the internal translation of this keyboard and let the kernel manage it. It can be done by implementing a fixup table that first translates all the media controls to their respective F keys, and other similar internal translations, so that the keyboard can mimic the original Apple version. Like the all the 3 fn modes should also work.
> 
> The trouble is, we have no way to read the state of the Omoton's Fn
> key in software. The Fn key is entirely internal to the keyboard. I
> even looked at the raw HID reports with and without Fn pressed, and
> there is nothing. So either we translate F1 and F2 to Brightness Down
> and Brightness Up (in fnmode=1) or we don't translate them at all (in
> fnmode=0 and fnmode=2); we can't conditionally translate depending on
> the Fn key.
> 
> But for all the other special keys, what you are saying makes sense
> and is a good idea: In fnmode=0, we can translate all of the special
> keys to be ordinary keys, and in fnmode=2, we can translate them to
> their opposites. For example, if the keyboard sends Volume Down, in
> fnmode=0 and fnmode=2 we'd translate it to F11 (because the keyboard
> only sends Volume Down when Fn is not held), and if it sends F11, in
> fnmode=2 we'd translate it to Volume Down (because the keyboard only
> sends F11 when Fn is held).

My idea was based on the assumption that Fn is sending KEY_FN. Now, if my idea is implemented, the user would have to manually change the fnmod to switch between media and function keys, which is quite inconvenient, just because 2 keys are broken.

I would like to think it as fmode=0 with F1 and F2 broken because of manufacturing defect.

You also said F6 shows a reserved keycode, which IIRC, you sent in a patch to translate to F6. Is the keycode same with and without Fn? I am thinking of using this reserved keycode as a toggle key for the hid-apple driver to switch the modes between media and function keys.
>> Also looking at the keyboard pic on https://www.amazon.in/OMOTON-Ultra-Slim-Bluetooth-Keyboard-Compatible/dp/B07S7VPQG6?th=1, the translation table for magic keyboard aluminium seems quite different from what this keyboard keys show.
> 
> Let's please not change the special keys to be different from what
> their labels show. For example, F3 is Search on the Omoton KB066 and
> Scale on the Apple A1255. Personally I wouldn't even change Power to
> Screen Lock to match its icon, although I'm okay with changing it if
> others feel strongly.

In case you have an Apple A1255, could you check whether hdev->name is different from "Bluetooth Keyboard”. We don’t really want to break the original Apple version just because of this defective clone.
Alex Henrie Feb. 17, 2025, 6:42 a.m. UTC | #11
On Sun, Feb 16, 2025 at 10:18 PM Aditya Garg <gargaditya08@live.com> wrote:

> BTW, are there any differences in HID reports if Fn+F1 and only F1 are pressed?

No.

> You also said F6 shows a reserved keycode, which IIRC, you sent in a patch to translate to F6. Is the keycode same with and without Fn?

When Fn is held, F6 is plain F6 and no translation is required.

> In case you have an Apple A1255, could you check whether hdev->name is different from "Bluetooth Keyboard”. We don’t really want to break the original Apple version just because of this defective clone.

If I understand correctly Mac OS sets each Apple keyboard's internal
name to "<username>'s keyboard" by default, and that's what mine is,
but a user could conceivably override that with "Bluetooth Keyboard".
It's also possible "Bluetooth Keyboard" is the name that all A1255's
had when they walked out of the factory, before they were connected to
an Apple device.

-Alex
Aditya Garg Feb. 17, 2025, 10:02 a.m. UTC | #12
Hi Alex

> If I understand correctly Mac OS sets each Apple keyboard's internal
> name to "<username>'s keyboard" by default, and that's what mine is,
> but a user could conceivably override that with "Bluetooth Keyboard".
> It's also possible "Bluetooth Keyboard" is the name that all A1255's
> had when they walked out of the factory, before they were connected to
> an Apple device.
> 
> -Alex

Can you to test the patch at the bottom of this message?

Before you apply the patch, you first need to apply these 3 patches I have sent upstream:

https://lore.kernel.org/all/FE7D2C98-2BF5-48C2-8183-68EC1085C1EC@live.com/t/#u

Then you have to apply _only_ PATCH 4/4 here:

https://patchwork.kernel.org/project/linux-input/patch/7747D0EE-DA32-4B6A-AB63-DB30C5E856BE@live.com/

Then apply the patch.

Then see if Fn+F6 switches the media to function keys or not, and media keys work by default or not.

—>8—

From 6fd840590b58689b1d99a523156266f790c8e98d Mon Sep 17 00:00:00 2001
From: Aditya Garg <gargaditya08@live.com>
Date: Mon, 17 Feb 2025 15:20:58 +0530
Subject: [PATCH] omoton

---
 drivers/hid/hid-apple.c | 53 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 4e8b01793..eaafa285a 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -42,6 +42,7 @@
 #define APPLE_BACKLIGHT_CTL	BIT(10)
 #define APPLE_IS_NON_APPLE	BIT(11)
 #define APPLE_MAGIC_BACKLIGHT	BIT(12)
+#define APPLE_IS_OMOTON		BIT(13)
 
 #define APPLE_FLAG_FKEY			0x01
 #define APPLE_FLAG_DONT_TRANSLATE	0x02
@@ -53,6 +54,8 @@
 #define APPLE_MAGIC_REPORT_ID_POWER		3
 #define APPLE_MAGIC_REPORT_ID_BRIGHTNESS	1
 
+#define KEY_F6_OMOTON	0xc0301
+
 static unsigned int fnmode = 3;
 module_param(fnmode, uint, 0644);
 MODULE_PARM_DESC(fnmode, "Mode of fn key on Apple keyboards (0 = disabled, "
@@ -81,6 +84,8 @@ MODULE_PARM_DESC(swap_fn_leftctrl, "Swap the Fn and left Control keys. "
 		"(For people who want to keep PC keyboard muscle memory. "
 		"[0] = as-is, Mac layout, 1 = swapped, PC layout)");
 
+bool omoton_media_key;
+
 struct apple_non_apple_keyboard {
 	char *name;
 };
@@ -276,6 +281,25 @@ static const struct apple_key_translation powerbook_numlock_keys[] = {
 	{ }
 };
 
+static const struct apple_key_translation omoton_media_keys[] = {
+	{ KEY_F1,	KEY_BRIGHTNESSDOWN },
+	{ KEY_F2,	KEY_BRIGHTNESSUP },
+	{ }
+};
+
+static const struct apple_key_translation omoton_function_keys[] = {
+	{ KEY_SEARCH,		KEY_F3 },
+	{ KEY_EJECTCD,		KEY_F4 },
+	{ KEY_NUMLOCK,		KEY_F5 },
+	{ KEY_PREVIOUSSONG,	KEY_F7 },
+	{ KEY_PLAYPAUSE,	KEY_F8 },
+	{ KEY_NEXTSONG,		KEY_F9 },
+	{ KEY_MUTE,		KEY_F10 },
+	{ KEY_VOLUMEDOWN,	KEY_F11 },
+	{ KEY_VOLUMEUP,		KEY_F12 },
+	{ }
+};
+
 static const struct apple_key_translation apple_iso_keyboard[] = {
 	{ KEY_GRAVE,	KEY_102ND },
 	{ KEY_102ND,	KEY_GRAVE },
@@ -377,6 +401,25 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 		real_fnmode = fnmode;
 	}
 
+	/* Omoton KB066 quirk */
+	if (asc->quirks & APPLE_IS_OMOTON) {
+		real_fnmode = 0;
+		if (usage->hid == KEY_F6_OMOTON) 
+			code = KEY_F6;
+
+		if (usage->code == KEY_F6) {
+			if (value == 1)
+				omoton_media_key = !omoton_media_key;
+			code = KEY_UNKNOWN;
+		}
+
+		table = omoton_media_key ? omoton_media_keys : omoton_function_keys;
+		trans = apple_find_translation(table, code);
+
+		if (trans)
+			code = trans->to;
+	}
+
 	if (swap_fn_leftctrl) {
 		trans = apple_find_translation(swapped_fn_leftctrl_keys, code);
 
@@ -511,9 +554,6 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 		}
 	}
 
-	if (usage->hid == 0xc0301) /* Omoton KB066 quirk */
-		code = KEY_F6;
-
 	if (usage->code != code) {
 		input_event_with_scancode(input, usage->type, code, usage->hid, value);
 
@@ -701,6 +741,12 @@ static int apple_input_configured(struct hid_device *hdev,
 		asc->quirks |= APPLE_IS_NON_APPLE;
 	}
 
+	if (strncmp(hdev->name, "Bluetooth Keyboard", 18) == 0 &&
+			hdev->product == USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI) {
+		hid_info(hdev, "Omoton keyboard detected; use Fn+F6 to toggle between media and function keys\n");
+		asc->quirks |= APPLE_IS_OMOTON;
+	}
+
 	return 0;
 }
 
@@ -897,6 +943,7 @@ static int apple_probe(struct hid_device *hdev,
 	mod_timer(&asc->battery_timer,
 		  jiffies + msecs_to_jiffies(APPLE_BATTERY_TIMEOUT_MS));
 	apple_fetch_battery(hdev);
+	omoton_media_key = true;
 
 	if (quirks & APPLE_BACKLIGHT_CTL)
 		apple_backlight_init(hdev);
Aditya Garg Feb. 17, 2025, 10:03 a.m. UTC | #13
To make things easier, I am also sending hid-apple.c as an attachment

> Can you to test the patch at the bottom of this message?
>
Alex Henrie Feb. 24, 2025, 2:46 a.m. UTC | #14
On Sun, Feb 16, 2025 at 11:42 PM Alex Henrie <alexhenrie24@gmail.com> wrote:

> If I understand correctly Mac OS sets each Apple keyboard's internal
> name to "<username>'s keyboard" by default, and that's what mine is,
> but a user could conceivably override that with "Bluetooth Keyboard".
> It's also possible "Bluetooth Keyboard" is the name that all A1255's
> had when they walked out of the factory, before they were connected to
> an Apple device.

Today I successfully used a MacBook5,1 from 2008 to factory-reset my
A1255 keyboard, by holding Shift and Option while clicking the
Bluetooth icon in the menu bar and clicking Debug > Factory reset all
connected Apple devices. After the reset, the keyboard's name reverted
to "Apple Wireless Keyboard". Now that we know that the name
"Bluetooth Keyboard" is not used by default on a real Apple keyboard
and is unlikely to have been set manually, I think it will be fine to
have the hid-apple driver behave differently if the PID is 022c and
the keyboard name is "Bluetooth Keyboard".

-Alex
Alex Henrie Feb. 24, 2025, 2:50 a.m. UTC | #15
On Mon, Feb 17, 2025 at 3:02 AM Aditya Garg <gargaditya08@live.com> wrote:

> Can you to test the patch at the bottom of this message?

I tried it and the patch works. However, I don't think it is the right approach.

> Then see if Fn+F6 switches the media to function keys or not, and media keys work by default or not.

The main problem I have with this idea is that there is nothing to
indicate to the user that Fn+F6 switches between Fn modes. If the user
presses Fn+F6 trying to actually type F6, they will be very confused.

What all of this discussion tells me is that it's not possible to make
the Omoton KB066 work perfectly, and it's not worth our time to try.
I'm not even convinced anymore that my original patch was a good idea.
Since we know now that we can detect the Omoton reliably enough based
on its name and its PID, I suggest that we simply add "Bluetooth
Keyboard" to the non_apple_keyboards table, with a new flag to
indicate that the name must match exactly and a new field to indicate
that the PID must be 022c. Being in the table will effectively disable
the counterproductive Fn key handling because fnmode=2 is equivalent
to fnmode=0 on the Omoton.

I will send a new patch.

-Alex
Alex Henrie Feb. 24, 2025, 4:44 a.m. UTC | #16
On Sun, Feb 23, 2025 at 7:50 PM Alex Henrie <alexhenrie24@gmail.com> wrote:

> fnmode=2 is equivalent
> to fnmode=0 on the Omoton.

Ugh, I just discovered that I was wrong about that too: In fnmode=2,
F6 is still converted to Num Lock. fnmode=0 really would be the better
default for the Omoton.

-Alex
Aditya Garg Feb. 24, 2025, 5:05 a.m. UTC | #17
> On 24 Feb 2025, at 10:14 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
> 
> On Sun, Feb 23, 2025 at 7:50 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
> 
>> fnmode=2 is equivalent
>> to fnmode=0 on the Omoton.
> 
> Ugh, I just discovered that I was wrong about that too: In fnmode=2,
> F6 is still converted to Num Lock. fnmode=0 really would be the better
> default for the Omoton.
> 
Removing APPLE_HAS_FN quirk seems to be a better idea tbh
> -Alex
Alex Henrie Feb. 24, 2025, 5:18 a.m. UTC | #18
On Sun, Feb 23, 2025 at 10:05 PM Aditya Garg <gargaditya08@live.com> wrote:

> > On Sun, Feb 23, 2025 at 7:50 PM Alex Henrie <alexhenrie24@gmail.com> wrote:

> > fnmode=0 really would be the better
> > default for the Omoton.
> >
> Removing APPLE_HAS_FN quirk seems to be a better idea tbh

I agree. I'll send a patch shortly that will do exactly that.

-Alex
Aditya Garg Feb. 24, 2025, 5:30 a.m. UTC | #19
> On 24 Feb 2025, at 10:49 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
> 
> On Sun, Feb 23, 2025 at 10:05 PM Aditya Garg <gargaditya08@live.com> wrote:
> 
>>> On Sun, Feb 23, 2025 at 7:50 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
> 
>>> fnmode=0 really would be the better
>>> default for the Omoton.
>>> 
>> Removing APPLE_HAS_FN quirk seems to be a better idea tbh
> 
> I agree. I'll send a patch shortly that will do exactly that.

I just made a patch. I'll sent it soon.
> 
> -Alex
Alex Henrie Feb. 24, 2025, 5:40 a.m. UTC | #20
On Sun, Feb 23, 2025 at 10:30 PM Aditya Garg <gargaditya08@live.com> wrote:

> > On 24 Feb 2025, at 10:49 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
> >
> > On Sun, Feb 23, 2025 at 10:05 PM Aditya Garg <gargaditya08@live.com> wrote:

> >> Removing APPLE_HAS_FN quirk seems to be a better idea tbh
> >
> > I agree. I'll send a patch shortly that will do exactly that.
>
> I just made a patch. I'll sent it soon.

I was going to spend a little more time looking over the patch I wrote
before I sent it, but I just sent it to avoid wasting your time on
duplicated effort. The most difficult part of the patch was writing a
clear explanation in the commit message.

-Alex
Aditya Garg Feb. 24, 2025, 5:46 a.m. UTC | #21
> On 24 Feb 2025, at 11:10 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
> 
> On Sun, Feb 23, 2025 at 10:30 PM Aditya Garg <gargaditya08@live.com> wrote:
> 
>>> On 24 Feb 2025, at 10:49 AM, Alex Henrie <alexhenrie24@gmail.com> wrote:
>>> 
>>> On Sun, Feb 23, 2025 at 10:05 PM Aditya Garg <gargaditya08@live.com> wrote:
> 
>>>> Removing APPLE_HAS_FN quirk seems to be a better idea tbh
>>> 
>>> I agree. I'll send a patch shortly that will do exactly that.
>> 
>> I just made a patch. I'll sent it soon.
> 
> I was going to spend a little more time looking over the patch I wrote
> before I sent it, but I just sent it to avoid wasting your time on
> duplicated effort. The most difficult part of the patch was writing a
> clear explanation in the commit message.

Commit messages are definitely a PITA LOL

Could you see this patch btw. It also addresses some minor code style issues in the driver.

—>8—

From 09472be6428dd74064b2165a2a78e61e049c2667 Mon Sep 17 00:00:00 2001
From: Aditya Garg <gargaditya08@live.com>
Date: Mon, 24 Feb 2025 11:10:42 +0530
Subject: [PATCH] fix

Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 drivers/hid/hid-apple.c | 63 ++++++++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 49812a76b..18b779403 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -30,7 +30,7 @@
 #include "hid-ids.h"
 
 #define APPLE_RDESC_JIS		BIT(0)
-#define APPLE_IGNORE_MOUSE	BIT(1)
+/* BIT(1) reserved, was: APPLE_IGNORE_MOUSE */
 #define APPLE_HAS_FN		BIT(2)
 /* BIT(3) reserved, was: APPLE_HIDDEV */
 #define APPLE_ISO_TILDE_QUIRK	BIT(4)
@@ -84,11 +84,29 @@ struct apple_non_apple_keyboard {
 	char *name;
 };
 
+struct apple_non_apple_keyboard_disable_fn {
+	char *name;
+	u32 product_id;
+};
+
 struct apple_sc_backlight {
 	struct led_classdev cdev;
 	struct hid_device *hdev;
 };
 
+struct apple_backlight_config_report {
+	u8 report_id;
+	u8 version;
+	u16 backlight_off, backlight_on_min, backlight_on_max;
+};
+
+struct apple_backlight_set_report {
+	u8 report_id;
+	u8 version;
+	u16 backlight;
+	u16 rate;
+};
+
 struct apple_magic_backlight {
 	struct led_classdev cdev;
 	struct hid_report *brightness;
@@ -152,20 +170,6 @@ static const struct apple_key_translation magic_keyboard_2015_fn_keys[] = {
 	{ }
 };
 
-struct apple_backlight_config_report {
-	u8 report_id;
-	u8 version;
-	u16 backlight_off, backlight_on_min, backlight_on_max;
-};
-
-struct apple_backlight_set_report {
-	u8 report_id;
-	u8 version;
-	u16 backlight;
-	u16 rate;
-};
-
-
 static const struct apple_key_translation apple2021_fn_keys[] = {
 	{ KEY_BACKSPACE, KEY_DELETE },
 	{ KEY_ENTER,	KEY_INSERT },
@@ -352,6 +356,7 @@ static const struct apple_key_translation swapped_fn_leftctrl_keys[] = {
 	{ }
 };
 
+/* We want the default fnmode be 2 in these keyboards */
 static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
 	{ "SONiX USB DEVICE" },
 	{ "Keychron" },
@@ -364,6 +369,11 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
 	{ "WKB603" },
 };
 
+/* Some non Apple keyboards report they have fn, but do internal translation of their keys */
+static const struct apple_non_apple_keyboard_disable_fn non_apple_keyboards_disable_fn[] = {
+	{ "Bluetooth Keyboard",	USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI},
+};
+
 static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
 {
 	int i;
@@ -378,6 +388,22 @@ static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
 	return false;
 }
 
+static bool apple_disable_fn(struct hid_device *hdev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(non_apple_keyboards_disable_fn); i++) {
+		char *non_apple = non_apple_keyboards_disable_fn[i].name;
+		u32 pid = non_apple_keyboards_disable_fn[i].product_id;
+
+		if (strncmp(hdev->name, non_apple, strlen(non_apple)) == 0 &&
+			hdev->product == pid)
+			return true;
+	}
+
+	return false;
+}
+
 static inline void apple_setup_key_translation(struct input_dev *input,
 		const struct apple_key_translation *table)
 {
@@ -546,9 +572,6 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 		}
 	}
 
-	if (usage->hid == 0xc0301) /* Omoton KB066 quirk */
-		code = KEY_F6;
-
 	if (usage->code != code) {
 		input_event_with_scancode(input, usage->type, code, usage->hid, value);
 
@@ -728,8 +751,8 @@ static int apple_input_configured(struct hid_device *hdev,
 {
 	struct apple_sc *asc = hid_get_drvdata(hdev);
 
-	if ((asc->quirks & APPLE_HAS_FN) && !asc->fn_found) {
-		hid_info(hdev, "Fn key not found (Apple Wireless Keyboard clone?), disabling Fn key handling\n");
+	if (((asc->quirks & APPLE_HAS_FN) && !asc->fn_found) || apple_disable_fn(hdev)) {
+		hid_info(hdev, "Fn key not found or has internal handling (Apple Wireless Keyboard clone?), disabling Fn key handling\n");
 		asc->quirks &= ~APPLE_HAS_FN;
 	}
Alex Henrie Feb. 24, 2025, 6:01 a.m. UTC | #22
On Sun, Feb 23, 2025 at 10:47 PM Aditya Garg <gargaditya08@live.com> wrote:

> Could you see this patch btw. It also addresses some minor code style issues in the driver.

Each patch should do one small thing. Style issues in unrelated code
should be addressed in separate patches.

> +               if (strncmp(hdev->name, non_apple, strlen(non_apple)) == 0 &&

This will match any keyboard whose name starts with "Bluetooth
Keyboard", instead of keyboards whose names are exactly "Bluetooth
Keyboard". A name such as "Bluetooth Keyboard 16" is only possible if
the keyboard is a real Apple keyboard that has been renamed in Mac OS,
which means that it is not an Omoton.

All in all, the general idea of having a table like in your patch is
not a bad idea, but like I said in my previous email, it seems
superfluous at this point in time.

-Alex
diff mbox series

Patch

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 7e1ae2a2bcc2..9767d17941d0 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -545,6 +545,9 @@  static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 		}
 	}
 
+	if (usage->hid == 0xc0301) /* Omoton KB066 quirk */
+		code = KEY_F6;
+
 	if (usage->code != code) {
 		input_event_with_scancode(input, usage->type, code, usage->hid, value);