Platform-driver-x86 archive mirror
 help / color / mirror / Atom feed
From: "Mark Pearson" <mpearson-lenovo@squebb.ca>
To: "Hans de Goede" <hdegoede@redhat.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Henrique de Moraes Holschuh" <hmh@hmh.eng.br>
Cc: "Vishnu Sankar" <vishnuocv@gmail.com>,
	"Nitin Joshi1" <njoshi1@lenovo.com>,
	ibm-acpi-devel@lists.sourceforge.net,
	"platform-driver-x86@vger.kernel.org"
	<platform-driver-x86@vger.kernel.org>
Subject: Re: [PATCH v2 22/24] platform/x86: thinkpad_acpi: Support for trackpoint doubletap
Date: Mon, 29 Apr 2024 08:40:50 -0400	[thread overview]
Message-ID: <28d295a8-7226-4222-b167-060a99134607@app.fastmail.com> (raw)
In-Reply-To: <5ea90914-16f7-4904-b7a6-e1997880e5f0@redhat.com>



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-lenovo@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-lenovo@squebb.ca>
>>> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
>>> Link: https://lore.kernel.org/r/20240417173124.9953-2-mpearson-lenovo@squebb.ca
>>> [hdegoede@redhat.com: Adjust for switch to sparse-keymap keymaps]
>>> Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>>> Signed-off-by: Hans de Goede <hdegoede@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

  reply	other threads:[~2024-04-29 12:39 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-24 12:28 [PATCH v2 00/24] platform/x86: thinkpad_acpi: Refactor hotkey handling and add support for some new hotkeys Hans de Goede
2024-04-24 12:28 ` [PATCH v2 01/24] platform/x86: thinkpad_acpi: Take hotkey_mutex during hotkey_exit() Hans de Goede
2024-04-24 12:28 ` [PATCH v2 02/24] platform/x86: thinkpad_acpi: Provide hotkey_poll_stop_sync() dummy Hans de Goede
2024-04-24 12:28 ` [PATCH v2 03/24] platform/x86: thinkpad_acpi: Drop setting send_/ignore_acpi_ev defaults twice Hans de Goede
2024-04-24 12:28 ` [PATCH v2 04/24] platform/x86: thinkpad_acpi: Drop ignore_acpi_ev Hans de Goede
2024-04-25  7:13   ` Ilpo Järvinen
2024-04-29  9:34     ` Hans de Goede
2024-04-24 12:28 ` [PATCH v2 05/24] platform/x86: thinkpad_acpi: Use tpacpi_input_send_key() in adaptive kbd code Hans de Goede
2024-04-24 12:28 ` [PATCH v2 06/24] platform/x86: thinkpad_acpi: Do hkey to scancode translation later Hans de Goede
2024-04-24 12:28 ` [PATCH v2 07/24] platform/x86: thinkpad_acpi: Make tpacpi_driver_event() return if it handled the event Hans de Goede
2024-04-24 12:28 ` [PATCH v2 08/24] platform/x86: thinkpad_acpi: Move adaptive kbd event handling to tpacpi_driver_event() Hans de Goede
2024-04-24 12:28 ` [PATCH v2 09/24] platform/x86: thinkpad_acpi: Move special original hotkeys handling out of switch-case Hans de Goede
2024-04-24 12:28 ` [PATCH v2 10/24] platform/x86: thinkpad_acpi: Move hotkey_user_mask check to tpacpi_input_send_key() Hans de Goede
2024-04-24 12:28 ` [PATCH v2 11/24] platform/x86: thinkpad_acpi: Always call tpacpi_driver_event() for hotkeys Hans de Goede
2024-04-24 12:28 ` [PATCH v2 12/24] platform/x86: thinkpad_acpi: Drop tpacpi_input_send_key_masked() and hotkey_driver_event() Hans de Goede
2024-04-24 12:28 ` [PATCH v2 13/24] platform/x86: thinkpad_acpi: Move hkey > scancode mapping to tpacpi_input_send_key() Hans de Goede
2024-04-24 12:28 ` [PATCH v2 14/24] platform/x86: thinkpad_acpi: Move tpacpi_driver_event() call " Hans de Goede
2024-04-24 12:28 ` [PATCH v2 15/24] platform/x86: thinkpad_acpi: Do not send ACPI netlink events for unknown hotkeys Hans de Goede
2024-04-25  8:51   ` Ilpo Järvinen
2024-04-24 12:28 ` [PATCH v2 16/24] platform/x86: thinkpad_acpi: Change hotkey_reserved_mask initialization Hans de Goede
2024-04-24 14:17   ` Mark Pearson
2024-04-24 14:47     ` Hans de Goede
2024-04-24 15:11       ` Mark Pearson
2024-04-25  9:14   ` Ilpo Järvinen
2024-04-29  9:52     ` Hans de Goede
2024-04-29 10:06       ` Ilpo Järvinen
2024-04-24 12:28 ` [PATCH v2 17/24] platform/x86: thinkpad_acpi: Use correct keycodes for volume and brightness keys Hans de Goede
2024-04-24 12:28 ` [PATCH v2 18/24] platform/x86: thinkpad_acpi: Drop KEY_RESERVED special handling Hans de Goede
2024-04-24 12:28 ` [PATCH v2 19/24] platform/x86: thinkpad_acpi: Switch to using sparse-keymap helpers Hans de Goede
2024-05-22 11:50   ` Vlastimil Babka
2024-05-22 13:30     ` Hans de Goede
2024-04-24 12:28 ` [PATCH v2 20/24] platform/x86: thinkpad_acpi: Add mappings for adaptive kbd clipping-tool and cloud keys Hans de Goede
2024-04-24 12:28 ` [PATCH v2 21/24] platform/x86: thinkpad_acpi: Simplify known_ev handling Hans de Goede
2024-04-24 12:28 ` [PATCH v2 22/24] platform/x86: thinkpad_acpi: Support for trackpoint doubletap Hans de Goede
2024-04-24 18:19   ` Mark Pearson
2024-04-29  9:57     ` Hans de Goede
2024-04-29 12:40       ` Mark Pearson [this message]
2024-04-24 12:28 ` [PATCH v2 23/24] platform/x86: thinkpad_acpi: Support for system debug info hotkey Hans de Goede
2024-04-24 12:28 ` [PATCH v2 24/24] platform/x86: thinkpad_acpi: Support hotkey to disable trackpoint doubletap Hans de Goede
2024-04-24 18:28 ` [PATCH v2 00/24] platform/x86: thinkpad_acpi: Refactor hotkey handling and add support for some new hotkeys Mark Pearson
2024-04-25  9:31 ` Ilpo Järvinen
2024-04-29 10:03 ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=28d295a8-7226-4222-b167-060a99134607@app.fastmail.com \
    --to=mpearson-lenovo@squebb.ca \
    --cc=andy@kernel.org \
    --cc=hdegoede@redhat.com \
    --cc=hmh@hmh.eng.br \
    --cc=ibm-acpi-devel@lists.sourceforge.net \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=njoshi1@lenovo.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=vishnuocv@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).