Hi Bryan,
On 5/5/22 21:12, Bryan Cain wrote:
> Keychron's C-series and K-series of keyboards copy the vendor and
> product IDs of an Apple keyboard, but only behave like that device when
> set to "Mac" mode. In "Windows" mode, the Fn key doesn't generate a
> scancode, so it's impossible to use the F1-F12 keys when fnmode is set
> to its default value of 1.
>
> To fix this, make fnmode default to the new value of 3, which behaves
> like fnmode=2 for Keychron keyboards and like fnmode=1 for actual Apple
> keyboards. This way, Keychron devices are fully usable in both "Windows"
> and "Mac" modes, while behavior is unchanged for everything else.
>
> Signed-off-by: Bryan Cain <bryancain3@xxxxxxxxx>
Thank you for doing this. This is pretty much what I had in mind
when discussing things in the previous thread, but I obviously
never got around to implementing this.
The patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Regards,
Hans
> ---
> drivers/hid/hid-apple.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 0cf35caee9fa..42a568902f49 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -21,6 +21,7 @@
> #include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/timer.h>
> +#include <linux/string.h>
>
> #include "hid-ids.h"
>
> @@ -35,16 +36,17 @@
> #define APPLE_NUMLOCK_EMULATION BIT(8)
> #define APPLE_RDESC_BATTERY BIT(9)
> #define APPLE_BACKLIGHT_CTL BIT(10)
> +#define APPLE_IS_KEYCHRON BIT(11)
>
> #define APPLE_FLAG_FKEY 0x01
>
> #define HID_COUNTRY_INTERNATIONAL_ISO 13
> #define APPLE_BATTERY_TIMEOUT_MS 60000
>
> -static unsigned int fnmode = 1;
> +static unsigned int fnmode = 3;
> module_param(fnmode, uint, 0644);
> MODULE_PARM_DESC(fnmode, "Mode of fn key on Apple keyboards (0 = disabled, "
> - "[1] = fkeyslast, 2 = fkeysfirst)");
> + "1 = fkeyslast, 2 = fkeysfirst, [3] = auto)");
>
> static int iso_layout = -1;
> module_param(iso_layout, int, 0644);
> @@ -349,6 +351,7 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
> const struct apple_key_translation *trans, *table;
> bool do_translate;
> u16 code = 0;
> + unsigned int real_fnmode;
>
> u16 fn_keycode = (swap_fn_leftctrl) ? (KEY_LEFTCTRL) : (KEY_FN);
>
> @@ -359,7 +362,13 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
> return 1;
> }
>
> - if (fnmode) {
> + if (fnmode == 3) {
> + real_fnmode = (asc->quirks & APPLE_IS_KEYCHRON) ? 2 : 1;
> + } else {
> + real_fnmode = fnmode;
> + }
> +
> + if (real_fnmode) {
> if (hid->product == USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI ||
> hid->product == USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO ||
> hid->product == USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS ||
> @@ -406,7 +415,7 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
>
> if (!code) {
> if (trans->flags & APPLE_FLAG_FKEY) {
> - switch (fnmode) {
> + switch (real_fnmode) {
> case 1:
> do_translate = !asc->fn_on;
> break;
> @@ -660,6 +669,11 @@ static int apple_input_configured(struct hid_device *hdev,
> asc->quirks &= ~APPLE_HAS_FN;
> }
>
> + if (strncmp(hdev->name, "Keychron", 8) == 0) {
> + hid_info(hdev, "Keychron keyboard detected; function keys will default to fnmode=2 behavior\n");
> + asc->quirks |= APPLE_IS_KEYCHRON;
> + }
> +
> return 0;
> }
>
