The hid-apple driver does not support chaining translations or
dependencies on other translations. This creates two problems:
1 - In Non-English keyboards of Macs, KEY_102ND and KEY_GRAVE are
swapped and the APPLE_ISO_TILDE_QUIRK is used to work around this
problem. The quirk is not set for the Macs where these bugs happen yet
(see the 2nd patch for that), but this can be forced by setting the
iso_layout parameter. Unfortunately, this only partially works.
KEY_102ND gets translated to KEY_GRAVE, but KEY_GRAVE does not get
translated to KEY_102ND, so both of them end up functioning as
KEY_GRAVE. This is because the driver translates the keys as if Fn was
pressed and the original is sent if it is not pressed, without any
further translations happening on the key[#463]. KEY_GRAVE is present at
macbookpro_no_esc_fn_keys[#195], so this is what happens:
- KEY_GRAVE -> KEY_ESC (as if Fn is pressed)
- KEY_GRAVE is returned (Fn isn't pressed, so translation is discarded)
- KEY_GRAVE -> KEY_102ND (this part is not reached!)
...
2 - In case the touchbar does not work, the driver supports sending
Escape when Fn+KEY_GRAVE is pressed. As mentioned previously, KEY_102ND
is actually KEY_GRAVE and needs to be translated before this happens.
Normally, these are the steps that should happen:
- KEY_102ND -> KEY_GRAVE
- KEY_GRAVE -> KEY_ESC (Fn is pressed)
- KEY_ESC is returned
Though this is what happens instead, as dependencies on other
translations are not supported:
- KEY_102ND -> KEY_ESC (Fn is pressed)
- KEY_ESC is returned
This patch fixes both bugs by ordering the translations correctly and by
making the translations continue and not return immediately after
translating a key so that chained translations work and translations can
depend on other ones.
This patch also simplifies the implementation of the swap_fn_leftctrl
option a little bit, as it makes it simply use a normal translation
instead adding extra code to translate a key to KEY_FN[#381]. This change
wasn't put in another patch as the code that translates the Fn key needs
to be changed because of the changes in the patch, and those changes
would be discarded with the next patch anyway (the part that originally
translates KEY_FN to KEY_LEFTCTRL needs to be made an else-if branch of
the part that transltes KEY_LEFTCTRL to KEY_FN).
Note: Line numbers (#XYZ) are for drivers/hid/hid-apple.c at commit
20afcc462579 ("HID: apple: Add "GANSS" to the non-Apple list").
Note: These bugs are only present on Macs with a keyboard with no
dedicated escape key and a non-English layout.
Signed-off-by: Kerem Karabay <[email protected]>
---
drivers/hid/hid-apple.c | 99 ++++++++++++++++++-----------------------
1 file changed, 44 insertions(+), 55 deletions(-)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 6970797cdc56..066e8a85daa5 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -314,6 +314,7 @@ static const struct apple_key_translation swapped_option_cmd_keys[] = {
static const struct apple_key_translation swapped_fn_leftctrl_keys[] = {
{ KEY_FN, KEY_LEFTCTRL },
+ { KEY_LEFTCTRL, KEY_FN },
{ }
};
@@ -375,24 +376,40 @@ 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;
+ u16 code = usage->code;
unsigned int real_fnmode;
- u16 fn_keycode = (swap_fn_leftctrl) ? (KEY_LEFTCTRL) : (KEY_FN);
-
- if (usage->code == fn_keycode) {
- asc->fn_on = !!value;
- input_event_with_scancode(input, usage->type, KEY_FN,
- usage->hid, value);
- return 1;
- }
-
if (fnmode == 3) {
real_fnmode = (asc->quirks & APPLE_IS_NON_APPLE) ? 2 : 1;
} else {
real_fnmode = fnmode;
}
+ if (swap_fn_leftctrl) {
+ trans = apple_find_translation(swapped_fn_leftctrl_keys, code);
+
+ if (trans)
+ code = trans->to;
+ }
+
+ if (iso_layout > 0 || (iso_layout < 0 && (asc->quirks & APPLE_ISO_TILDE_QUIRK) &&
+ hid->country == HID_COUNTRY_INTERNATIONAL_ISO)) {
+ trans = apple_find_translation(apple_iso_keyboard, code);
+
+ if (trans)
+ code = trans->to;
+ }
+
+ if (swap_opt_cmd) {
+ trans = apple_find_translation(swapped_option_cmd_keys, code);
+
+ if (trans)
+ code = trans->to;
+ }
+
+ if (code == KEY_FN)
+ asc->fn_on = !!value;
+
if (real_fnmode) {
if (hid->product == USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI ||
hid->product == USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO ||
@@ -430,15 +447,18 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
else
table = apple_fn_keys;
- trans = apple_find_translation (table, usage->code);
+ trans = apple_find_translation(table, code);
if (trans) {
- if (test_bit(trans->from, input->key))
+ bool from_is_set = test_bit(trans->from, input->key);
+ bool to_is_set = test_bit(trans->to, input->key);
+
+ if (from_is_set)
code = trans->from;
- else if (test_bit(trans->to, input->key))
+ else if (to_is_set)
code = trans->to;
- if (!code) {
+ if (!(from_is_set || to_is_set)) {
if (trans->flags & APPLE_FLAG_FKEY) {
switch (real_fnmode) {
case 1:
@@ -455,62 +475,31 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
do_translate = asc->fn_on;
}
- code = do_translate ? trans->to : trans->from;
+ if (do_translate)
+ code = trans->to;
}
-
- input_event_with_scancode(input, usage->type, code,
- usage->hid, value);
- return 1;
}
if (asc->quirks & APPLE_NUMLOCK_EMULATION &&
- (test_bit(usage->code, asc->pressed_numlock) ||
+ (test_bit(code, asc->pressed_numlock) ||
test_bit(LED_NUML, input->led))) {
- trans = apple_find_translation(powerbook_numlock_keys,
- usage->code);
+ trans = apple_find_translation(powerbook_numlock_keys, code);
if (trans) {
if (value)
- set_bit(usage->code,
- asc->pressed_numlock);
+ set_bit(code, asc->pressed_numlock);
else
- clear_bit(usage->code,
- asc->pressed_numlock);
+ clear_bit(code, asc->pressed_numlock);
- input_event_with_scancode(input, usage->type,
- trans->to, usage->hid, value);
+ code = trans->to;
}
-
- return 1;
}
}
- if (iso_layout > 0 || (iso_layout < 0 && (asc->quirks & APPLE_ISO_TILDE_QUIRK) &&
- hid->country == HID_COUNTRY_INTERNATIONAL_ISO)) {
- trans = apple_find_translation(apple_iso_keyboard, usage->code);
- if (trans) {
- input_event_with_scancode(input, usage->type,
- trans->to, usage->hid, value);
- return 1;
- }
- }
+ if (usage->code != code) {
+ input_event_with_scancode(input, usage->type, code, usage->hid, value);
- if (swap_opt_cmd) {
- trans = apple_find_translation(swapped_option_cmd_keys, usage->code);
- if (trans) {
- input_event_with_scancode(input, usage->type,
- trans->to, usage->hid, value);
- return 1;
- }
- }
-
- if (swap_fn_leftctrl) {
- trans = apple_find_translation(swapped_fn_leftctrl_keys, usage->code);
- if (trans) {
- input_event_with_scancode(input, usage->type,
- trans->to, usage->hid, value);
- return 1;
- }
+ return 1;
}
return 0;
--
2.37.2
The iso_layout parameter must be manually set to get the driver to
swap KEY_102ND and KEY_GRAVE. This patch eliminates the need to do that.
This is safe to do, as Macs with keyboards that do not need the quirk
will keep working the same way as the value of hid->country will be
different than HID_COUNTRY_INTERNATIONAL_ISO. This was tested by one
person with a Mac with the WELLSPRINGT2_J152F keyboard with a layout
that does not require the quirk to be set.
Signed-off-by: Kerem Karabay <[email protected]>
---
drivers/hid/hid-apple.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 066e8a85daa5..df994baf547d 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -1000,21 +1000,21 @@ static const struct hid_device_id apple_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING9_JIS),
.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J140K),
- .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL },
+ .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL | APPLE_ISO_TILDE_QUIRK },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J132),
- .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL },
+ .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL | APPLE_ISO_TILDE_QUIRK },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J680),
- .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL },
+ .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL | APPLE_ISO_TILDE_QUIRK },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J213),
- .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL },
+ .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL | APPLE_ISO_TILDE_QUIRK },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J214K),
- .driver_data = APPLE_HAS_FN },
+ .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J223),
- .driver_data = APPLE_HAS_FN },
+ .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J230K),
- .driver_data = APPLE_HAS_FN },
+ .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J152F),
- .driver_data = APPLE_HAS_FN },
+ .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_ANSI),
.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_ISO),
--
2.37.2
Hi,
I just sent a new version of this series, containing a bug fix, which can be
found at https://lore.kernel.org/all/[email protected]/.
Best wishes,
Kerem