Hi Mark, On 4/24/24 4:17 PM, Mark Pearson wrote: > Hi Hans, > > On Wed, Apr 24, 2024, at 8:28 AM, Hans de Goede wrote: >> Change the hotkey_reserved_mask initialization to hardcode the list >> of reserved keys. There are only a few reserved keys and the code to >> iterate over the keymap will be removed when moving to sparse-keymaps. >> >> Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> --- >> drivers/platform/x86/thinkpad_acpi.c | 21 +++++++++++++++------ >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/platform/x86/thinkpad_acpi.c >> b/drivers/platform/x86/thinkpad_acpi.c >> index 952bac635a18..cf5c741d1343 100644 >> --- a/drivers/platform/x86/thinkpad_acpi.c >> +++ b/drivers/platform/x86/thinkpad_acpi.c >> @@ -3545,6 +3545,19 @@ static int __init hotkey_init(struct >> ibm_init_struct *iibm) >> dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY, >> "using keymap number %lu\n", keymap_id); >> >> + /* Keys which should be reserved on both IBM and Lenovo models */ >> + hotkey_reserved_mask = TP_ACPI_HKEY_KBD_LIGHT_MASK | >> + TP_ACPI_HKEY_VOLUP_MASK | >> + TP_ACPI_HKEY_VOLDWN_MASK | >> + TP_ACPI_HKEY_MUTE_MASK; >> + /* >> + * Reserve brightness up/down unconditionally on IBM models, on Lenovo >> + * models these are disabled based on acpi_video_get_backlight_type(). >> + */ >> + if (keymap_id == TPACPI_KEYMAP_IBM_GENERIC) >> + hotkey_reserved_mask |= TP_ACPI_HKEY_BRGHTUP_MASK | >> + TP_ACPI_HKEY_BRGHTDWN_MASK; >> + >> hotkey_keycode_map = kmemdup(&tpacpi_keymaps[keymap_id], >> TPACPI_HOTKEY_MAP_SIZE, GFP_KERNEL); >> if (!hotkey_keycode_map) { >> @@ -3560,9 +3573,6 @@ static int __init hotkey_init(struct >> ibm_init_struct *iibm) >> if (hotkey_keycode_map[i] != KEY_RESERVED) { >> input_set_capability(tpacpi_inputdev, EV_KEY, >> hotkey_keycode_map[i]); >> - } else { >> - if (i < sizeof(hotkey_reserved_mask)*8) >> - hotkey_reserved_mask |= 1 << i; > > Just to check my understanding here - does this change mean that the keys marked as KEY_RESERVED in the lenovo map won't make it into the mask? > > e.g the ones in this block: > KEY_RESERVED, /* Mute held, 0x103 */ > KEY_BRIGHTNESS_MIN, /* Backlight off */ > KEY_RESERVED, /* Clipping tool */ > KEY_RESERVED, /* Cloud */ > KEY_RESERVED, > KEY_VOICECOMMAND, /* Voice */ > KEY_RESERVED, > KEY_RESERVED, /* Gestures */ > KEY_RESERVED, > KEY_RESERVED, > KEY_RESERVED, > KEY_CONFIG, /* Settings */ > KEY_RESERVED, /* New tab */ > KEY_REFRESH, /* Reload */ > KEY_BACK, /* Back */ > KEY_RESERVED, /* Microphone down */ > KEY_RESERVED, /* Microphone up */ > KEY_RESERVED, /* Microphone cancellation */ > KEY_RESERVED, /* Camera mode */ > KEY_RESERVED, /* Rotate display, 0x116 */ > > I'm not sure what the effect will be and I don't have the 2014 X1 Carbon (I assume it's the G1) available to test with unfortunately. Only the 32 original hotkeys are affected by any of the hotkey_*_mask values, note: if (i < sizeof(hotkey_reserved_mask)*8) hotkey_reserved_mask |= 1 << i; The (i < sizeof(hotkey_reserved_mask)*8) condition translates to (i < 32) so this code only ever set bits in hotkey_reserved_mask for the 32 original hotkeys. > Just wanted to be sure we weren't breaking something on older platforms. That is appreciated. As I mentioned in the cover letter I have been careful to try and not break older platforms, but double checking my work is appreciated. Regards, Hans > >> } >> } >> >> @@ -3587,9 +3597,8 @@ static int __init hotkey_init(struct >> ibm_init_struct *iibm) >> /* Disable brightness up/down on Lenovo thinkpads when >> * ACPI is handling them, otherwise it is plain impossible >> * for userspace to do something even remotely sane */ >> - hotkey_reserved_mask |= >> - (1 << TP_ACPI_HOTKEYSCAN_FNHOME) >> - | (1 << TP_ACPI_HOTKEYSCAN_FNEND); >> + hotkey_reserved_mask |= TP_ACPI_HKEY_BRGHTUP_MASK | >> + TP_ACPI_HKEY_BRGHTDWN_MASK; >> hotkey_unmap(TP_ACPI_HOTKEYSCAN_FNHOME); >> hotkey_unmap(TP_ACPI_HOTKEYSCAN_FNEND); >> } >> -- >> 2.44.0 > _______________________________________________ ibm-acpi-devel mailing list ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel