Thread (11 messages) 11 messages, 6 authors, 2025-07-30

Re: [PATCH] x86/Mouse: thinkpad_acpi/Trackpoint: Trackpoint Doubletap handling

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2025-06-30 19:03:22
Also in: lkml, platform-driver-x86

On Mon, Jun 30, 2025 at 08:50:27PM +0900, Vishnu Sankar wrote:
Hi Dimitry,


On Mon, Jun 30, 2025 at 2:20 PM Dmitry Torokhov
[off-list ref] wrote:
quoted
Hi Mark,

On Mon, Jun 30, 2025 at 05:42:45AM +0900, Mark Pearson wrote:
quoted
Hi Dmitry,

On Fri, Jun 27, 2025, at 2:14 PM, Dmitry Torokhov wrote:
quoted
Hi Vishnu,

On Fri, Jun 20, 2025 at 09:42:08AM +0900, Vishnu Sankar wrote:
quoted
Newer ThinkPads have a doubletap feature that needs to be turned
ON/OFF via the trackpoint registers.
Systems released from 2023 have doubletap disabled by default and
need the feature enabling to be useful.

This patch introduces support for exposing and controlling the
trackpoint doubletap feature via a sysfs attribute.
/sys/devices/platform/thinkpad_acpi/tp_doubletap
This can be toggled by an "enable" or a "disable".

With this implemented we can remove the masking of events, and rely on
HW control instead, when the feature is disabled.

Note - Early Thinkpads (pre 2015) used the same register for hysteris
control, Check the FW IDs to make sure these are not affected.

trackpoint.h is moved to linux/input/.
No, please keep everything private to trackpoint.c and do not involve
thinkpad_acpi driver. By doing so you are introducing unwanted
dependencies (for both module loading, driver initialization, and
operation) and unsafe use of non-owned pointers/dangling pointers, etc.
Do you have recommendations on how to handle this case then?

This is a Thinkpad specific feature and hence the logic for involving
thinkpad_acpi. There are Thinkpad hotkeys that will enable/disable the
trackpoint doubletap feature - so there is some linkage. I'm not sure
how to avoid that.

Is there a cleaner way to do this that you'd recommend we look at
using? It's a feature (albeit a minor one) on the laptops that we'd
like to make available to Linux users.
I believe if you define the doubletap as an attribute (see
TRACKPOINT_INT_ATTR or TRACKPOINT_BIT_ATTR in
drivers/input/mouse/trackpoint.c) then whatever process is handling the
hot keys switching this function on or off should be able to toggle the
behavior. The difference is that it will have to locate trackpoint node
in /sys/bus/serio/devices/* (or maybe scan
/sys/devices/platform/i8042/serio*) instead of expecting the attributes
be atached to thinkpad_acpi instance.

You just don't want to have one driver directly peeking into another,
because then it starts breaking if you unbind or force use of a
different protocol, etc.

Thanks.

--
Dmitry
Thanks for the suggestion. I understand the concern about avoiding
direct driver-to-driver calls and unwanted dependencies.

Just to clarify: if we move the sysfs attribute to the trackpoint
driver itself (under /sys/bus/serio/devices/...), then thinkpad_acpi
would no longer be able to directly enable/disable the doubletap
feature in response to the Fn+G hotkey press. Don't we need userspace
to listen for the hotkey event, find the trackpoint sysfs node, and
toggle the attribute there?
Yes.
That's possible, of course, but it means the feature won't work
out-of-the-box without extra userspace integration. For example, there
would be no automatic linkage between pressing Fn+G and toggling the
feature unless a udev rule or userspace daemon is configured to do it.
Or is there an approach you'd recommend to preserve the automatic
hotkey integration while avoiding the direct dependency between
thinkpad_acpi and trackpoint?
Sorry, I missed something.
Well, I guess you can look into interacting with sysfs file from
thinkpad_acpi.c... There is kernel_read_file_from_path() and others, you
will need to implement write counterpart of it. Pretty ugly but safer
than following pointers that may go away.

Thanks.

-- 
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help