On Mon, Apr 29, 2024, at 5:57 AM, Hans de Goede wrote:
> Hi Mark,
>
> On 4/24/24 8:19 PM, Mark Pearson wrote:
>> Hi Hans,
>>
>> On Wed, Apr 24, 2024, at 8:28 AM, Hans de Goede wrote:
>>> From: Mark Pearson <mpearson-len...@squebb.ca>
>>>
>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>> This handles the doubletap event and sends the KEY_PROG4 event to
>>> userspace. Despite the driver itself not using KEY_PROG1 - KEY_PROG3 this
>>> still uses KEY_PROG4 because of some keys being remapped to KEY_PROG1 -
>>> KEY_PROG3 by default by the upstream udev hwdb containing:
>>>
>>> evdev:name:ThinkPad Extra Buttons:dmi:bvn*:bvr*:bd*:svnLENOVO*:pn*:*
>>> ...
>>> KEYBOARD_KEY_17=prog1
>>> KEYBOARD_KEY_1a=f20 # Microphone mute button
>>> KEYBOARD_KEY_45=bookmarks
>>> KEYBOARD_KEY_46=prog2 # Fn + PrtSc, on Windows: Snipping tool
>>> KEYBOARD_KEY_4a=prog3 # Fn + Right shift, on Windows: No idea
>>>
>>> Signed-off-by: Mark Pearson <mpearson-len...@squebb.ca>
>>> Signed-off-by: Vishnu Sankar <vishnu...@gmail.com>
>>> Link:
>>> https://lore.kernel.org/r/20240417173124.9953-2-mpearson-len...@squebb.ca
>>> [hdego...@redhat.com: Adjust for switch to sparse-keymap keymaps]
>>> Tested-by: Mark Pearson <mpearson-len...@squebb.ca>
>>> Signed-off-by: Hans de Goede <hdego...@redhat.com>
>>> ---
>>> drivers/platform/x86/thinkpad_acpi.c | 19 +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c
>>> b/drivers/platform/x86/thinkpad_acpi.c
>>> index a53b00fecf1a..b6d6466215e1 100644
>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>> @@ -248,6 +248,9 @@ enum tpacpi_hkey_event_t {
>>>
>>> /* Misc */
>>> TP_HKEY_EV_RFKILL_CHANGED = 0x7000, /* rfkill switch changed */
>>> +
>>> + /* Misc2 */
>>> + TP_HKEY_EV_TRACK_DOUBLETAP = 0x8036, /* trackpoint doubletap */
>>> };
>>>
>>>
>>> /****************************************************************************
>>> @@ -3268,6 +3271,7 @@ static const struct key_entry keymap_lenovo[]
>>> __initconst = {
>>> * after switching to sparse keymap support. The mappings above use
>>> translated
>>> * scancodes to preserve uAPI compatibility, see
>>> tpacpi_input_send_key().
>>> */
>>> + { KE_KEY, TP_HKEY_EV_TRACK_DOUBLETAP /* 0x8036 */, { KEY_PROG4 } },
>>> { KE_END }
>>> };
>>>
>>> @@ -3817,6 +3821,17 @@ static bool hotkey_notify_6xxx(const u32 hkey,
>>> bool *send_acpi_ev)
>>> return true;
>>> }
>>>
>>> +static bool hotkey_notify_8xxx(const u32 hkey, bool *send_acpi_ev)
>>> +{
>>> + switch (hkey) {
>>> + case TP_HKEY_EV_TRACK_DOUBLETAP:
>>> + tpacpi_input_send_key(hkey, send_acpi_ev);
>>> + return true;
>>> + default:
>>> + return false;
>>> + }
>>> +}
>>> +
>>> static void hotkey_notify(struct ibm_struct *ibm, u32 event)
>>> {
>>> u32 hkey;
>>> @@ -3893,6 +3908,10 @@ static void hotkey_notify(struct ibm_struct
>>> *ibm, u32 event)
>>> known_ev = true;
>>> }
>>> break;
>>> + case 8:
>>> + /* 0x8000-0x8FFF: misc2 */
>>> + known_ev = hotkey_notify_8xxx(hkey, &send_acpi_ev);
>>> + break;
>>> }
>>> if (!known_ev) {
>>> pr_notice("unhandled HKEY event 0x%04x\n", hkey);
>>> --
>>> 2.44.0
>>
>> Instead of needing hotkey_notify_8xxx, now we are using the sparse_keymap
>> can we just use hotkey_notify_hotkey for case 8? No need to check what the
>> hkey is either.
>
> I prefer to keep things consistent and have each case #: path call a separate
> helper for those #xxx codes.
>
> ATM some of the simpler cases handle things directly, but as more
> handling for
> different events get added that becomes a bit messy IMHO. I would
> actually
> like to see those other cases converted to use a small helper function
> too
> (with a switch-case in the helper for future proofing) to make things
> consistent.
>
Got it - no problem.
> Patches to do this small cleanup are welcome.
>
Sounds good. Will look at doing this.
Mark
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
