[PATCH 0/4] platform/x86,input: Support for new events on

[PATCH 0/4] platform/x86,input: Support for new events on

[Mark Pearson]

Hi,

This series adds support trackpoint doubletap and some new hotkey
functionality which is being added on Lenovo laptops.
 - FN+N - display system debug info. Used by customer support with
   Windows users.
 - FN+G - disable/enable trackpoint doubletap.

We combined these into a series because there was commonality between
what the different features were doing.
Please let us know if you would prefer to have them as separate commits.

Many thanks to Peter Hutterer and Benjamin Tissoires for the guidance on
what would be best for exporting the events from trackpoint doubletap to
userspace. Any mistakes are ours :)

Features have been tested on Z13 G2 (doubletap & FN+G) and X1 Carbon 
G12 (FN+N)

Mark Pearson (4):
  Input: Add trackpoint doubletap and system debug info keycodes
  platform/x86: thinkpad_acpi: Support for trackpoint doubletap
  platform/x86: thinkpad_acpi: Support for system debug info hotkey
  platform/x86: thinkpad_acpi: Support hotkey to disable trackpoint
    doubletap

 drivers/platform/x86/thinkpad_acpi.c   | 31 ++++++++++++++++++++++++++
 include/uapi/linux/input-event-codes.h |  2 ++
 2 files changed, 33 insertions(+)

-- 
2.44.0

[PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark Pearson]

Add support for new input events on Lenovo laptops that need exporting to
user space.

Lenovo trackpoints are adding the ability to generate a doubletap event.
Add a new keycode to allow this to be used by userspace.

Lenovo support is using FN+N with Windows to collect needed details for
support cases. Add a keycode so that we'll be able to provide similar
support on Linux.

Suggested-by: Peter Hutterer <peter.hutterer@redhat.com>

Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Nitin Joshi <njoshi1@lenovo.com>
Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>
---
 include/uapi/linux/input-event-codes.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 03edf2ccdf6c..bd3baca95749 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -686,6 +686,8 @@
 #define KEY_SIDEVU_SONAR               0x287
 #define KEY_NAV_INFO                   0x288
 #define KEY_BRIGHTNESS_MENU            0x289
+#define KEY_DOUBLECLICK                0x28a
+#define KEY_SYS_DEBUG_INFO             0x28b
 
 /*
  * Some keyboards have keys which do not have a defined meaning, these keys
-- 
2.44.0

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de Goede]

Hi,

On 3/24/24 10:07 PM, Mark Pearson wrote:
> Add support for new input events on Lenovo laptops that need exporting to
> user space.
> 
> Lenovo trackpoints are adding the ability to generate a doubletap event.
> Add a new keycode to allow this to be used by userspace.
> 
> Lenovo support is using FN+N with Windows to collect needed details for
> support cases. Add a keycode so that we'll be able to provide similar
> support on Linux.
> 
> Suggested-by: Peter Hutterer <peter.hutterer@redhat.com>
> 
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Nitin Joshi <njoshi1@lenovo.com>
> Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Dmitry, can I have your ack for merging this change through the pdx86
tree (since the first driver using these is a pdx86 driver) ?

Regards,

Hans




> ---
>  include/uapi/linux/input-event-codes.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 03edf2ccdf6c..bd3baca95749 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -686,6 +686,8 @@
>  #define KEY_SIDEVU_SONAR               0x287
>  #define KEY_NAV_INFO                   0x288
>  #define KEY_BRIGHTNESS_MENU            0x289
> +#define KEY_DOUBLECLICK                0x28a
> +#define KEY_SYS_DEBUG_INFO             0x28b
>  
>  /*
>   * Some keyboards have keys which do not have a defined meaning, these keys

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

Hi Mark,

On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> Add support for new input events on Lenovo laptops that need exporting to
> user space.
> 
> Lenovo trackpoints are adding the ability to generate a doubletap event.
> Add a new keycode to allow this to be used by userspace.

What is the intended meaning of this keycode? How does it differ from
the driver sending BTN_LEFT press/release twice?
> 
> Lenovo support is using FN+N with Windows to collect needed details for
> support cases. Add a keycode so that we'll be able to provide similar
> support on Linux.

Is there a userspace consumer for this?

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark Pearson]

Hi Dmitry

On Mon, Apr 8, 2024, at 7:31 PM, Dmitry Torokhov wrote:
> Hi Mark,
>
> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>> Add support for new input events on Lenovo laptops that need exporting to
>> user space.
>> 
>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>> Add a new keycode to allow this to be used by userspace.
>
> What is the intended meaning of this keycode? How does it differ from
> the driver sending BTN_LEFT press/release twice?

Double tapping on the trackpoint is a unique event - it's not the same as BTN_LEFT twice. The BIOS will send a new ACPI event for it and it's not meant to be the same as mouse button clicks.

For example, on Windows this launches a utility that let's you do various configurations on your laptop (some Lenovo specific), but that's not available on Linux (yet). We did want to make it flexible in this implementation so users could use it for whatever was useful to them.

>> 
>> Lenovo support is using FN+N with Windows to collect needed details for
>> support cases. Add a keycode so that we'll be able to provide similar
>> support on Linux.
>
> Is there a userspace consumer for this?

There isn't yet, though we would like to implement something, and do plan to. 
We still have to work through the details of the best way to do this, and if it's Lenovo specific, or (probably better) something generic.

I don't have as much knowledge on the user-space side development, and my experience is it tends to move a bit slower for getting things implemented. We thought we'd get the framework in, so it's available, and then work with user-space folk to deliver a solution in a suitable manner.
Ultimately this is something we'll need in our Linux preloads and the aim is to make it easier for Linux users to get help from our support team (not always the easiest currently).
I don't know if other vendors have something similar, but I wouldn't be surprised if this could be re-used in other cases so I hope it's not Lenovo specific. It felt like it would be useful functionality to have :)

Mark

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de Goede]

Hi Dmitry,

On 4/9/24 2:00 AM, Mark Pearson wrote:
> Hi Dmitry
> 
> On Mon, Apr 8, 2024, at 7:31 PM, Dmitry Torokhov wrote:
>> Hi Mark,
>>
>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>>> Add support for new input events on Lenovo laptops that need exporting to
>>> user space.
>>>
>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>> Add a new keycode to allow this to be used by userspace.
>>
>> What is the intended meaning of this keycode? How does it differ from
>> the driver sending BTN_LEFT press/release twice?
> 
> Double tapping on the trackpoint is a unique event - it's not the same as BTN_LEFT twice. The BIOS will send a new ACPI event for it and it's not meant to be the same as mouse button clicks.

To extend a bit on this, this double-tap event is not reported through
the PS/2 trackpoint interface at all. Instead it is reported to
the OS by the ACPI hotkey notifier, which is used to report various
multi-media hotkeys and things like that, this is handled by
the thinkpad_apci driver which sofar only reports key-presses.

So there is no BTN_LEFT to report twice and if we add a BTN_LEFT
then we end up with an input_device which has a bunch of KEYs
+ BTN_LEFT but no abs/rel axis which will just confuse userspace.

We could add a second input_device which looks like a mouse
but only ever reports BTN_LEFT double-clicks I guess, but as
Mark said the intention is for this double-tap to work more
like a hotkey then a double click. Also note that regular
taps on the trackstick do nothing. Clicking the mouse buttons
of the stick involves pressing separate physical buttons between
the trackpad and the keyboard and those are reported over
the same PS/2 port as the relative motion events from the stick.

Regards,

Hans

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Tue, Apr 09, 2024 at 12:16:04PM +0200, Hans de Goede wrote:
> Hi Dmitry,
> 
> On 4/9/24 2:00 AM, Mark Pearson wrote:
> > Hi Dmitry
> > 
> > On Mon, Apr 8, 2024, at 7:31 PM, Dmitry Torokhov wrote:
> >> Hi Mark,
> >>
> >> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> >>> Add support for new input events on Lenovo laptops that need exporting to
> >>> user space.
> >>>
> >>> Lenovo trackpoints are adding the ability to generate a doubletap event.
> >>> Add a new keycode to allow this to be used by userspace.
> >>
> >> What is the intended meaning of this keycode? How does it differ from
> >> the driver sending BTN_LEFT press/release twice?
> > 
> > Double tapping on the trackpoint is a unique event - it's not the same as BTN_LEFT twice. The BIOS will send a new ACPI event for it and it's not meant to be the same as mouse button clicks.
> 
> To extend a bit on this, this double-tap event is not reported through
> the PS/2 trackpoint interface at all. Instead it is reported to
> the OS by the ACPI hotkey notifier, which is used to report various
> multi-media hotkeys and things like that, this is handled by
> the thinkpad_apci driver which sofar only reports key-presses.

Ah, I see, so this is just an arbitrary action not connected with the
pointer handling in any way.

For such actions we typically assign keycodes based on their intended
behavior, so instead of KEY_DOUBLECLICK which conveys user gesture but
not the intent you should consider using KEY_CONFIG (with is typically
mapped to Application Launcher - Consumer Control Configuration in HID
spec) or KEY_CONTROLPANEL (Application Launcher - Control Panel).

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Peter Hutterer]

On 09/04/2024 09:31, Dmitry Torokhov wrote:
> Hi Mark,
> 
> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>> Add support for new input events on Lenovo laptops that need exporting to
>> user space.
>>
>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>> Add a new keycode to allow this to be used by userspace.
> 
> What is the intended meaning of this keycode? How does it differ from
> the driver sending BTN_LEFT press/release twice?
>>
>> Lenovo support is using FN+N with Windows to collect needed details for
>> support cases. Add a keycode so that we'll be able to provide similar
>> support on Linux.
> 
> Is there a userspace consumer for this?

Funnily enough XKB has had a keysym for this for decades but it's not hooked up anywhere due to the way it's pointer keys accessibility feature was implemented. Theory is that most of userspace just needs to patch the various pieces together for the new evdev code + keysym, it's not really any different to handling a volume key (except this one needs to be assignable).

Cheers,
   Peter

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> On 09/04/2024 09:31, Dmitry Torokhov wrote:
> > Hi Mark,
> > 
> > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> > > Add support for new input events on Lenovo laptops that need exporting to
> > > user space.
> > > 
> > > Lenovo trackpoints are adding the ability to generate a doubletap event.
> > > Add a new keycode to allow this to be used by userspace.
> > 
> > What is the intended meaning of this keycode? How does it differ from
> > the driver sending BTN_LEFT press/release twice?
> > > 
> > > Lenovo support is using FN+N with Windows to collect needed details for
> > > support cases. Add a keycode so that we'll be able to provide similar
> > > support on Linux.
> > 
> > Is there a userspace consumer for this?
> 
> Funnily enough XKB has had a keysym for this for decades but it's not
> hooked up anywhere due to the way it's pointer keys accessibility
> feature was implemented. Theory is that most of userspace just needs
> to patch the various pieces together for the new evdev code + keysym,
> it's not really any different to handling a volume key (except this
> one needs to be assignable).

What is the keysym? If we can make them relatable to each other that
would be good. Or maybe we could find a matching usage from HID usage
tables...

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> > On 09/04/2024 09:31, Dmitry Torokhov wrote:
> > > Hi Mark,
> > > 
> > > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> > > > Add support for new input events on Lenovo laptops that need exporting to
> > > > user space.
> > > > 
> > > > Lenovo trackpoints are adding the ability to generate a doubletap event.
> > > > Add a new keycode to allow this to be used by userspace.
> > > 
> > > What is the intended meaning of this keycode? How does it differ from
> > > the driver sending BTN_LEFT press/release twice?
> > > > 
> > > > Lenovo support is using FN+N with Windows to collect needed details for
> > > > support cases. Add a keycode so that we'll be able to provide similar
> > > > support on Linux.
> > > 
> > > Is there a userspace consumer for this?
> > 
> > Funnily enough XKB has had a keysym for this for decades but it's not
> > hooked up anywhere due to the way it's pointer keys accessibility
> > feature was implemented. Theory is that most of userspace just needs
> > to patch the various pieces together for the new evdev code + keysym,
> > it's not really any different to handling a volume key (except this
> > one needs to be assignable).
> 
> What is the keysym? If we can make them relatable to each other that
> would be good. Or maybe we could find a matching usage from HID usage
> tables...

I was looking through the existing codes and I see:

#define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */

We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
specific debug info collection application (which I honestly doubt will
materialize).

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark Pearson]

Hi Dmitry

On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
>> > On 09/04/2024 09:31, Dmitry Torokhov wrote:
>> > > Hi Mark,
>> > > 
>> > > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>> > > > Add support for new input events on Lenovo laptops that need exporting to
>> > > > user space.
>> > > > 
>> > > > Lenovo trackpoints are adding the ability to generate a doubletap event.
>> > > > Add a new keycode to allow this to be used by userspace.
>> > > 
>> > > What is the intended meaning of this keycode? How does it differ from
>> > > the driver sending BTN_LEFT press/release twice?
>> > > > 
>> > > > Lenovo support is using FN+N with Windows to collect needed details for
>> > > > support cases. Add a keycode so that we'll be able to provide similar
>> > > > support on Linux.
>> > > 
>> > > Is there a userspace consumer for this?
>> > 
>> > Funnily enough XKB has had a keysym for this for decades but it's not
>> > hooked up anywhere due to the way it's pointer keys accessibility
>> > feature was implemented. Theory is that most of userspace just needs
>> > to patch the various pieces together for the new evdev code + keysym,
>> > it's not really any different to handling a volume key (except this
>> > one needs to be assignable).
>> 
>> What is the keysym? If we can make them relatable to each other that
>> would be good. Or maybe we could find a matching usage from HID usage
>> tables...
>
> I was looking through the existing codes and I see:
>
> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
>
> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> specific debug info collection application (which I honestly doubt will
> materialize).
>

That's a somewhat disappointing note on your doubts, is that based on anything? Just wondering what we've done to deserve that criticism.

That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I personally don't think KEY_CONFIG matches well), but I would be worried about clashing with existing functionality.

Peter - do you have any opinion from the user space side of things, or are these likely unused? KEY_VENDOR seems the safer bet to me (but I don't love it).

Dmitry - What are the downsides or concerns of introducing a new code? I'd like to evaluate that against the potential to cause conflicts by re-using existing codes. If you feel strongly about it then I'll defer to your judgement, but I'd like to understand better the context.

Thanks
Mark

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
> Hi Dmitry
> 
> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
> > On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> >> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> >> > On 09/04/2024 09:31, Dmitry Torokhov wrote:
> >> > > Hi Mark,
> >> > > 
> >> > > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> >> > > > Add support for new input events on Lenovo laptops that need exporting to
> >> > > > user space.
> >> > > > 
> >> > > > Lenovo trackpoints are adding the ability to generate a doubletap event.
> >> > > > Add a new keycode to allow this to be used by userspace.
> >> > > 
> >> > > What is the intended meaning of this keycode? How does it differ from
> >> > > the driver sending BTN_LEFT press/release twice?
> >> > > > 
> >> > > > Lenovo support is using FN+N with Windows to collect needed details for
> >> > > > support cases. Add a keycode so that we'll be able to provide similar
> >> > > > support on Linux.
> >> > > 
> >> > > Is there a userspace consumer for this?
> >> > 
> >> > Funnily enough XKB has had a keysym for this for decades but it's not
> >> > hooked up anywhere due to the way it's pointer keys accessibility
> >> > feature was implemented. Theory is that most of userspace just needs
> >> > to patch the various pieces together for the new evdev code + keysym,
> >> > it's not really any different to handling a volume key (except this
> >> > one needs to be assignable).
> >> 
> >> What is the keysym? If we can make them relatable to each other that
> >> would be good. Or maybe we could find a matching usage from HID usage
> >> tables...
> >
> > I was looking through the existing codes and I see:
> >
> > #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
> >
> > We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> > thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> > specific debug info collection application (which I honestly doubt will
> > materialize).
> >
> 
> That's a somewhat disappointing note on your doubts, is that based on
> anything? Just wondering what we've done to deserve that criticism.

Sorry, this was not meant as a criticism really, but you mentioned
yourself that there isn't anything in the works yet, you just have some
plans.

For such a project to succeed Lenovo needs to invest into selling
devices with Linux as a primary operating system, and it has to be
consumer segment (or small business, because for corporate they
typically roll their own support channels). The case of retrofitting
Linux onto a that device originally came with Windows OS rarely gets
much if any response from the normal support channels.

Is this something that is actually happening?

> 
> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
> personally don't think KEY_CONFIG matches well), but I would be
> worried about clashing with existing functionality.
> 
> Peter - do you have any opinion from the user space side of things, or
> are these likely unused? KEY_VENDOR seems the safer bet to me (but I
> don't love it).
> 
> Dmitry - What are the downsides or concerns of introducing a new code?
> I'd like to evaluate that against the potential to cause conflicts by
> re-using existing codes. If you feel strongly about it then I'll defer
> to your judgement, but I'd like to understand better the context.

The keycode space is finite and extending bitmaps leads to more memory
consumption and weird breakages (like uevent generation exceeding 4K
memory page resulting in failures). I am trying to balance need for new
keycodes with how likely they are to be used.

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark Pearson]

Hi Dmitry,

On Wed, Apr 10, 2024, at 8:02 PM, Dmitry Torokhov wrote:
> On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
>> Hi Dmitry
>> 
>> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
>> > On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
>> >> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
>> >> > On 09/04/2024 09:31, Dmitry Torokhov wrote:
>> >> > > Hi Mark,
>> >> > > 
>> >> > > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>> >> > > > Add support for new input events on Lenovo laptops that need exporting to
>> >> > > > user space.
>> >> > > > 
>> >> > > > Lenovo trackpoints are adding the ability to generate a doubletap event.
>> >> > > > Add a new keycode to allow this to be used by userspace.
>> >> > > 
>> >> > > What is the intended meaning of this keycode? How does it differ from
>> >> > > the driver sending BTN_LEFT press/release twice?
>> >> > > > 
>> >> > > > Lenovo support is using FN+N with Windows to collect needed details for
>> >> > > > support cases. Add a keycode so that we'll be able to provide similar
>> >> > > > support on Linux.
>> >> > > 
>> >> > > Is there a userspace consumer for this?
>> >> > 
>> >> > Funnily enough XKB has had a keysym for this for decades but it's not
>> >> > hooked up anywhere due to the way it's pointer keys accessibility
>> >> > feature was implemented. Theory is that most of userspace just needs
>> >> > to patch the various pieces together for the new evdev code + keysym,
>> >> > it's not really any different to handling a volume key (except this
>> >> > one needs to be assignable).
>> >> 
>> >> What is the keysym? If we can make them relatable to each other that
>> >> would be good. Or maybe we could find a matching usage from HID usage
>> >> tables...
>> >
>> > I was looking through the existing codes and I see:
>> >
>> > #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
>> >
>> > We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
>> > thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
>> > specific debug info collection application (which I honestly doubt will
>> > materialize).
>> >
>> 
>> That's a somewhat disappointing note on your doubts, is that based on
>> anything? Just wondering what we've done to deserve that criticism.
>
> Sorry, this was not meant as a criticism really, but you mentioned
> yourself that there isn't anything in the works yet, you just have some
> plans.

All good - I was just worried we'd promised something previously and not delivered. We're often slow at delivering - but I try not to fail completely. I try especially hard not to annoy hard working kernel maintainers :)

We do have something developed internally, but it's pretty basic and we'll need to have discussion with 'userspace' fol as to if we release that as a standalone application, or if we tie into gnome (which we don't have a lot of experience with)...or something else.
Kernel world is much easier :)

>
> For such a project to succeed Lenovo needs to invest into selling
> devices with Linux as a primary operating system, and it has to be
> consumer segment (or small business, because for corporate they
> typically roll their own support channels). The case of retrofitting
> Linux onto a that device originally came with Windows OS rarely gets
> much if any response from the normal support channels.
>
> Is this something that is actually happening?

This is way off topic for the patch set, but as you asked :)

I'm afraid I'm going to disagree. What you're suggesting is probably the more common and easier route vendors take, but it has some issues and I think some longer term limitations.
For me one of the things I like the most about our program using the same exact HW as Windows is it means:
 - I have a business lever to get Linux support from HW vendors. This makes a difference when you're dealing with 'minor' components - panels, fingerprint readers, touchpads etc - the smaller devices (though it helps for the CPU vendors too). We have requirements that state the upstream Linux support is needed or the chip vendor will not be considered for the platform...and that's a big deal.
 - It gets FW teams thinking about Linux support. Yeah, there are still a ton of issues there and it's far from perfect, but it means they have to think about Linux support and at least helps us shine a bit of light on what is going on in that horrible closed box.
 - It means I get Linux running at a good level on a wider range of HW then would be otherwise possible. It means that when there is new technology I get to go and ask "how about Linux" and have that discussion (including schedules). It might come later than I'd like - but at least we get to put Linux on the roadmap rather than being excluded for being so niche. Shrug.

I think our Linux program still has a long way to go before it's even close to good - but it's in better shape than it (at least I think so, I don't love the way the industry is going with more being stuffed in FW...but that's above my paygrade)

>
>> 
>> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
>> personally don't think KEY_CONFIG matches well), but I would be
>> worried about clashing with existing functionality.
>> 
>> Peter - do you have any opinion from the user space side of things, or
>> are these likely unused? KEY_VENDOR seems the safer bet to me (but I
>> don't love it).
>> 
>> Dmitry - What are the downsides or concerns of introducing a new code?
>> I'd like to evaluate that against the potential to cause conflicts by
>> re-using existing codes. If you feel strongly about it then I'll defer
>> to your judgement, but I'd like to understand better the context.
>
> The keycode space is finite and extending bitmaps leads to more memory
> consumption and weird breakages (like uevent generation exceeding 4K
> memory page resulting in failures). I am trying to balance need for new
> keycodes with how likely they are to be used.
>
Ack.
So I've been refactoring my patches to implement the feedback from Hans for the latter patches and just need to figure out what works here.

For the SYS_DEBUG_INFO, if you'd rather we use KEY_VENDOR (I think that's my preference - as its intended for a Lenovo support role it seems the best fit), and Peter has no objections, I will make that change.

I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.

Is that OK?

Mark

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
> 
> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.

Please see my response to Peter's letter. I think it very much depends
on how it will be used (associated with the pointer or standalone as it
is now).

For standalone application, recalling your statement that on Win you
have this gesture invoke configuration utility I would argue for
KEY_CONFIG for it.

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de Goede]

Hi,

On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>
>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
> 
> Please see my response to Peter's letter. I think it very much depends
> on how it will be used (associated with the pointer or standalone as it
> is now).
> 
> For standalone application, recalling your statement that on Win you
> have this gesture invoke configuration utility I would argue for
> KEY_CONFIG for it.

KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
the GNOME/KDE control center/panel and I believe that at least GNOME
comes with a default binding to map KEY_CONFIG to the control-center.

So IMHO re-using KEY_CONFIG for the doubletap trackpoint thing is not
a good idea. But as mentioned elsewhere in the thread everyone seems
to be ok with using KEY_VENDOR for this ?

Regards,

Hans

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
> > On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
> >>
> >> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
> > 
> > Please see my response to Peter's letter. I think it very much depends
> > on how it will be used (associated with the pointer or standalone as it
> > is now).
> > 
> > For standalone application, recalling your statement that on Win you
> > have this gesture invoke configuration utility I would argue for
> > KEY_CONFIG for it.
> 
> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
> the GNOME/KDE control center/panel and I believe that at least GNOME
> comes with a default binding to map KEY_CONFIG to the control-center.

Not KEY_CONTROLPANEL?

Are there devices that use both Fn+# and the doubleclick? Would it be an
acceptable behavior for the users to have them behave the same?

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark Pearson]

Hi

On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>> Hi,
>> 
>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>> > On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>> >>
>> >> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>> > 
>> > Please see my response to Peter's letter. I think it very much depends
>> > on how it will be used (associated with the pointer or standalone as it
>> > is now).
>> > 
>> > For standalone application, recalling your statement that on Win you
>> > have this gesture invoke configuration utility I would argue for
>> > KEY_CONFIG for it.
>> 
>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>> the GNOME/KDE control center/panel and I believe that at least GNOME
>> comes with a default binding to map KEY_CONFIG to the control-center.
>
> Not KEY_CONTROLPANEL?
>
> Are there devices that use both Fn+# and the doubleclick? Would it be an
> acceptable behavior for the users to have them behave the same?
>
Catching up with the thread, thanks for all the comments.

For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use KEY_VENDOR there. My conclusion was that this is targeting vendor specific functionality, and that was the closest fit, if a new keycode was not preferred.

For the doubletap (which is a unique input event - not related to the pointer) I would like to keep it as a new unique event. 

I think it's most likely use would be for control panel, but I don't think it should be limited to that. I can see it being useful if users are able to reconfigure it to launch something different (browser or music player maybe?), hence it would be best if it did not conflict with an existing keycode function. I also can't confirm it doesn't clash on existing or future systems - it's possible.

FWIW - I wouldn't be surprised with some of the new gaming systems we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a joystick might be useful to have, if the HW supports it?

Mark

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
> Hi
> 
> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
> > On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
> >> Hi,
> >> 
> >> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
> >> > On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
> >> >>
> >> >> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
> >> > 
> >> > Please see my response to Peter's letter. I think it very much depends
> >> > on how it will be used (associated with the pointer or standalone as it
> >> > is now).
> >> > 
> >> > For standalone application, recalling your statement that on Win you
> >> > have this gesture invoke configuration utility I would argue for
> >> > KEY_CONFIG for it.
> >> 
> >> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
> >> the GNOME/KDE control center/panel and I believe that at least GNOME
> >> comes with a default binding to map KEY_CONFIG to the control-center.
> >
> > Not KEY_CONTROLPANEL?
> >
> > Are there devices that use both Fn+# and the doubleclick? Would it be an
> > acceptable behavior for the users to have them behave the same?
> >
> Catching up with the thread, thanks for all the comments.
> 
> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
> KEY_VENDOR there. My conclusion was that this is targeting vendor
> specific functionality, and that was the closest fit, if a new keycode
> was not preferred.

Fn+N -> KEY_VENDOR mapping sounds good to me.

> 
> For the doubletap (which is a unique input event - not related to the
> pointer) I would like to keep it as a new unique event. 
> 
> I think it's most likely use would be for control panel, but I don't
> think it should be limited to that. I can see it being useful if users
> are able to reconfigure it to launch something different (browser or
> music player maybe?), hence it would be best if it did not conflict
> with an existing keycode function. I also can't confirm it doesn't
> clash on existing or future systems - it's possible.

So here is the problem. Keycodes in linux input are not mere
placeholders for something that will be decided later how it is to be
used, they are supposed to communicate intent and userspace ideally does
not need to have any additional knowledge about where the event is
coming from. A keyboard either internal or external sends KEY_SCREENLOCK
and the system should lock the screen. It should not be aware that one
device was a generic USB external keyboard while another had an internal
sensor that recognized hovering palm making swiping motion to the right
because a vendor decided to make it. Otherwise you have millions of
input devices all generating unique codes and you need userspace to
decide how to interpret data coming from each device individually.

If you truly do not have a defined use case for it you have a couple
options:

- assign it KEY_RESERVED, ensure your driver allows remapping to an
  arbitrary keycode, let user or distro assign desired keycode to it

- assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
  hand of the user to define a shortcut in their DE to make it useful

> 
> FWIW - I wouldn't be surprised with some of the new gaming systems
> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
> joystick might be useful to have, if the HW supports it?

What would it do exactly? Once we have this answer we can define key or
button code (although I do agree that game controller buttons are
different from "normal" keys because they map to the geometry of the
controller which in turn defines their commonly understood function).

But in any case you would not reuse the same keycode for something that
is supposed to invoke a configuration utility and also to let's say
drop a flash grenade in a game.

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark Pearson]

Hi Dmitry,

On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>> Hi
>> 
>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>> > On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>> >> Hi,
>> >> 
>> >> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>> >> > On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>> >> >>
>> >> >> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>> >> > 
>> >> > Please see my response to Peter's letter. I think it very much depends
>> >> > on how it will be used (associated with the pointer or standalone as it
>> >> > is now).
>> >> > 
>> >> > For standalone application, recalling your statement that on Win you
>> >> > have this gesture invoke configuration utility I would argue for
>> >> > KEY_CONFIG for it.
>> >> 
>> >> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>> >> the GNOME/KDE control center/panel and I believe that at least GNOME
>> >> comes with a default binding to map KEY_CONFIG to the control-center.
>> >
>> > Not KEY_CONTROLPANEL?
>> >
>> > Are there devices that use both Fn+# and the doubleclick? Would it be an
>> > acceptable behavior for the users to have them behave the same?
>> >
>> Catching up with the thread, thanks for all the comments.
>> 
>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>> specific functionality, and that was the closest fit, if a new keycode
>> was not preferred.
>
> Fn+N -> KEY_VENDOR mapping sounds good to me.
>
>> 
>> For the doubletap (which is a unique input event - not related to the
>> pointer) I would like to keep it as a new unique event. 
>> 
>> I think it's most likely use would be for control panel, but I don't
>> think it should be limited to that. I can see it being useful if users
>> are able to reconfigure it to launch something different (browser or
>> music player maybe?), hence it would be best if it did not conflict
>> with an existing keycode function. I also can't confirm it doesn't
>> clash on existing or future systems - it's possible.
>
> So here is the problem. Keycodes in linux input are not mere
> placeholders for something that will be decided later how it is to be
> used, they are supposed to communicate intent and userspace ideally does
> not need to have any additional knowledge about where the event is
> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
> and the system should lock the screen. It should not be aware that one
> device was a generic USB external keyboard while another had an internal
> sensor that recognized hovering palm making swiping motion to the right
> because a vendor decided to make it. Otherwise you have millions of
> input devices all generating unique codes and you need userspace to
> decide how to interpret data coming from each device individually.
>
> If you truly do not have a defined use case for it you have a couple
> options:
>
> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>   arbitrary keycode, let user or distro assign desired keycode to it
>
> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>   hand of the user to define a shortcut in their DE to make it useful
>
>> 
>> FWIW - I wouldn't be surprised with some of the new gaming systems
>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>> joystick might be useful to have, if the HW supports it?
>
> What would it do exactly? Once we have this answer we can define key or
> button code (although I do agree that game controller buttons are
> different from "normal" keys because they map to the geometry of the
> controller which in turn defines their commonly understood function).
>
> But in any case you would not reuse the same keycode for something that
> is supposed to invoke a configuration utility and also to let's say
> drop a flash grenade in a game.
>

Understood.

I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.

I will drop this patch from the series and proceed using existing defined codes only.

Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.

Mark

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de Goede]

Hi Mark,

On 4/16/24 1:57 AM, Mark Pearson wrote:
> Hi Dmitry,
> 
> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>> Hi
>>>
>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>
>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>
>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>> is now).
>>>>>>
>>>>>> For standalone application, recalling your statement that on Win you
>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>> KEY_CONFIG for it.
>>>>>
>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>
>>>> Not KEY_CONTROLPANEL?
>>>>
>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>> acceptable behavior for the users to have them behave the same?
>>>>
>>> Catching up with the thread, thanks for all the comments.
>>>
>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>> specific functionality, and that was the closest fit, if a new keycode
>>> was not preferred.
>>
>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>
>>>
>>> For the doubletap (which is a unique input event - not related to the
>>> pointer) I would like to keep it as a new unique event. 
>>>
>>> I think it's most likely use would be for control panel, but I don't
>>> think it should be limited to that. I can see it being useful if users
>>> are able to reconfigure it to launch something different (browser or
>>> music player maybe?), hence it would be best if it did not conflict
>>> with an existing keycode function. I also can't confirm it doesn't
>>> clash on existing or future systems - it's possible.
>>
>> So here is the problem. Keycodes in linux input are not mere
>> placeholders for something that will be decided later how it is to be
>> used, they are supposed to communicate intent and userspace ideally does
>> not need to have any additional knowledge about where the event is
>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>> and the system should lock the screen. It should not be aware that one
>> device was a generic USB external keyboard while another had an internal
>> sensor that recognized hovering palm making swiping motion to the right
>> because a vendor decided to make it. Otherwise you have millions of
>> input devices all generating unique codes and you need userspace to
>> decide how to interpret data coming from each device individually.
>>
>> If you truly do not have a defined use case for it you have a couple
>> options:
>>
>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>>   arbitrary keycode, let user or distro assign desired keycode to it
>>
>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>>   hand of the user to define a shortcut in their DE to make it useful
>>
>>>
>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>> joystick might be useful to have, if the HW supports it?
>>
>> What would it do exactly? Once we have this answer we can define key or
>> button code (although I do agree that game controller buttons are
>> different from "normal" keys because they map to the geometry of the
>> controller which in turn defines their commonly understood function).
>>
>> But in any case you would not reuse the same keycode for something that
>> is supposed to invoke a configuration utility and also to let's say
>> drop a flash grenade in a game.
>>
> 
> Understood.
> 
> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
> 
> I will drop this patch from the series and proceed using existing defined codes only.
> 
> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.

Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
me and for the doubletap any one of KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
or some other suitable KEY_foo define works for me.

Regards,

Hans

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark Pearson]

Hi Hans

On Tue, Apr 16, 2024, at 4:33 AM, Hans de Goede wrote:
> Hi Mark,
>
> On 4/16/24 1:57 AM, Mark Pearson wrote:
>> Hi Dmitry,
>> 
>> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>>> Hi
>>>>
>>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>>
>>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>>
>>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>>> is now).
>>>>>>>
>>>>>>> For standalone application, recalling your statement that on Win you
>>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>>> KEY_CONFIG for it.
>>>>>>
>>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>>
>>>>> Not KEY_CONTROLPANEL?
>>>>>
>>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>>> acceptable behavior for the users to have them behave the same?
>>>>>
>>>> Catching up with the thread, thanks for all the comments.
>>>>
>>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>>> specific functionality, and that was the closest fit, if a new keycode
>>>> was not preferred.
>>>
>>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>>
>>>>
>>>> For the doubletap (which is a unique input event - not related to the
>>>> pointer) I would like to keep it as a new unique event. 
>>>>
>>>> I think it's most likely use would be for control panel, but I don't
>>>> think it should be limited to that. I can see it being useful if users
>>>> are able to reconfigure it to launch something different (browser or
>>>> music player maybe?), hence it would be best if it did not conflict
>>>> with an existing keycode function. I also can't confirm it doesn't
>>>> clash on existing or future systems - it's possible.
>>>
>>> So here is the problem. Keycodes in linux input are not mere
>>> placeholders for something that will be decided later how it is to be
>>> used, they are supposed to communicate intent and userspace ideally does
>>> not need to have any additional knowledge about where the event is
>>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>>> and the system should lock the screen. It should not be aware that one
>>> device was a generic USB external keyboard while another had an internal
>>> sensor that recognized hovering palm making swiping motion to the right
>>> because a vendor decided to make it. Otherwise you have millions of
>>> input devices all generating unique codes and you need userspace to
>>> decide how to interpret data coming from each device individually.
>>>
>>> If you truly do not have a defined use case for it you have a couple
>>> options:
>>>
>>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>>>   arbitrary keycode, let user or distro assign desired keycode to it
>>>
>>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>>>   hand of the user to define a shortcut in their DE to make it useful
>>>
>>>>
>>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>>> joystick might be useful to have, if the HW supports it?
>>>
>>> What would it do exactly? Once we have this answer we can define key or
>>> button code (although I do agree that game controller buttons are
>>> different from "normal" keys because they map to the geometry of the
>>> controller which in turn defines their commonly understood function).
>>>
>>> But in any case you would not reuse the same keycode for something that
>>> is supposed to invoke a configuration utility and also to let's say
>>> drop a flash grenade in a game.
>>>
>> 
>> Understood.
>> 
>> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
>> 
>> I will drop this patch from the series and proceed using existing defined codes only.
>> 
>> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.
>
> Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
> me and for the doubletap any one of 
> KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
> or some other suitable KEY_foo define works for me.
>
I think this should be a configurable input, by design. So my preference (if not allowed a new keycode, which I personally think is the better option) is for PROG1.

I discussed with Peter last night and it looks likely OK on their side. I do plan on doing some testing first, so it might take a few days to get the next set of patches out.

Mark

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de Goede]

Hi,

On 4/16/24 2:48 PM, Mark Pearson wrote:
> Hi Hans
> 
> On Tue, Apr 16, 2024, at 4:33 AM, Hans de Goede wrote:
>> Hi Mark,
>>
>> On 4/16/24 1:57 AM, Mark Pearson wrote:
>>> Hi Dmitry,
>>>
>>> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>>>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>>>> Hi
>>>>>
>>>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>>>
>>>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>>>
>>>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>>>> is now).
>>>>>>>>
>>>>>>>> For standalone application, recalling your statement that on Win you
>>>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>>>> KEY_CONFIG for it.
>>>>>>>
>>>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>>>
>>>>>> Not KEY_CONTROLPANEL?
>>>>>>
>>>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>>>> acceptable behavior for the users to have them behave the same?
>>>>>>
>>>>> Catching up with the thread, thanks for all the comments.
>>>>>
>>>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>>>> specific functionality, and that was the closest fit, if a new keycode
>>>>> was not preferred.
>>>>
>>>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>>>
>>>>>
>>>>> For the doubletap (which is a unique input event - not related to the
>>>>> pointer) I would like to keep it as a new unique event. 
>>>>>
>>>>> I think it's most likely use would be for control panel, but I don't
>>>>> think it should be limited to that. I can see it being useful if users
>>>>> are able to reconfigure it to launch something different (browser or
>>>>> music player maybe?), hence it would be best if it did not conflict
>>>>> with an existing keycode function. I also can't confirm it doesn't
>>>>> clash on existing or future systems - it's possible.
>>>>
>>>> So here is the problem. Keycodes in linux input are not mere
>>>> placeholders for something that will be decided later how it is to be
>>>> used, they are supposed to communicate intent and userspace ideally does
>>>> not need to have any additional knowledge about where the event is
>>>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>>>> and the system should lock the screen. It should not be aware that one
>>>> device was a generic USB external keyboard while another had an internal
>>>> sensor that recognized hovering palm making swiping motion to the right
>>>> because a vendor decided to make it. Otherwise you have millions of
>>>> input devices all generating unique codes and you need userspace to
>>>> decide how to interpret data coming from each device individually.
>>>>
>>>> If you truly do not have a defined use case for it you have a couple
>>>> options:
>>>>
>>>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>>>>   arbitrary keycode, let user or distro assign desired keycode to it
>>>>
>>>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>>>>   hand of the user to define a shortcut in their DE to make it useful
>>>>
>>>>>
>>>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>>>> joystick might be useful to have, if the HW supports it?
>>>>
>>>> What would it do exactly? Once we have this answer we can define key or
>>>> button code (although I do agree that game controller buttons are
>>>> different from "normal" keys because they map to the geometry of the
>>>> controller which in turn defines their commonly understood function).
>>>>
>>>> But in any case you would not reuse the same keycode for something that
>>>> is supposed to invoke a configuration utility and also to let's say
>>>> drop a flash grenade in a game.
>>>>
>>>
>>> Understood.
>>>
>>> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
>>>
>>> I will drop this patch from the series and proceed using existing defined codes only.
>>>
>>> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.
>>
>> Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
>> me and for the doubletap any one of 
>> KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
>> or some other suitable KEY_foo define works for me.
>>
> I think this should be a configurable input, by design. So my preference (if not allowed a new keycode, which I personally think is the better option) is for PROG1.
> 
> I discussed with Peter last night and it looks likely OK on their side. I do plan on doing some testing first, so it might take a few days to get the next set of patches out.

Ok, PROG1 works for me.

Regards,

Hans

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de Goede]

Hi,

On 4/15/24 9:58 PM, Dmitry Torokhov wrote:
> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>
>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>
>>> Please see my response to Peter's letter. I think it very much depends
>>> on how it will be used (associated with the pointer or standalone as it
>>> is now).
>>>
>>> For standalone application, recalling your statement that on Win you
>>> have this gesture invoke configuration utility I would argue for
>>> KEY_CONFIG for it.
>>
>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>> the GNOME/KDE control center/panel and I believe that at least GNOME
>> comes with a default binding to map KEY_CONFIG to the control-center.
> 
> Not KEY_CONTROLPANEL?

No when this was added most distros were still using X11 not Wayland so
it was preferable to use KEY_foo symbols with codes below 240.

> Are there devices that use both Fn+# and the doubleclick? Would it be an
> acceptable behavior for the users to have them behave the same?

That is a good question, at least my current ThinkPad does not have
the Fn + F## combo for the control center anymore. So I guess we could
indeed re-use KEY_CONFIG for the double-tap.

Regards,

Hans

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de Goede]

Hi Dmitry,

On 4/11/24 2:02 AM, Dmitry Torokhov wrote:
> On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
>> Hi Dmitry
>>
>> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
>>> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
>>>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
>>>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
>>>>>> Hi Mark,
>>>>>>
>>>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>>>>>>> Add support for new input events on Lenovo laptops that need exporting to
>>>>>>> user space.
>>>>>>>
>>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>>>>>> Add a new keycode to allow this to be used by userspace.
>>>>>>
>>>>>> What is the intended meaning of this keycode? How does it differ from
>>>>>> the driver sending BTN_LEFT press/release twice?
>>>>>>>
>>>>>>> Lenovo support is using FN+N with Windows to collect needed details for
>>>>>>> support cases. Add a keycode so that we'll be able to provide similar
>>>>>>> support on Linux.
>>>>>>
>>>>>> Is there a userspace consumer for this?
>>>>>
>>>>> Funnily enough XKB has had a keysym for this for decades but it's not
>>>>> hooked up anywhere due to the way it's pointer keys accessibility
>>>>> feature was implemented. Theory is that most of userspace just needs
>>>>> to patch the various pieces together for the new evdev code + keysym,
>>>>> it's not really any different to handling a volume key (except this
>>>>> one needs to be assignable).
>>>>
>>>> What is the keysym? If we can make them relatable to each other that
>>>> would be good. Or maybe we could find a matching usage from HID usage
>>>> tables...
>>>
>>> I was looking through the existing codes and I see:
>>>
>>> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
>>>
>>> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
>>> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
>>> specific debug info collection application (which I honestly doubt will
>>> materialize).
>>>
>>
>> That's a somewhat disappointing note on your doubts, is that based on
>> anything? Just wondering what we've done to deserve that criticism.
> 
> Sorry, this was not meant as a criticism really, but you mentioned
> yourself that there isn't anything in the works yet, you just have some
> plans.
> 
> For such a project to succeed Lenovo needs to invest into selling
> devices with Linux as a primary operating system, and it has to be
> consumer segment (or small business, because for corporate they
> typically roll their own support channels). The case of retrofitting
> Linux onto a that device originally came with Windows OS rarely gets
> much if any response from the normal support channels.
> 
> Is this something that is actually happening?

Yes, Lenovo is actually offering Fedora as an OS choice when
ordering ThinkPads directly from their website in many countries
including when ordering as a consumer.

And unlike other vendor's Linux preloads which often use a kernel
with downstream laptop specific changes these laptops are running
unmodified Fedora kernels, which themselves are almost pristine
upstream kernels.

Lenovo (Mark) has been really good the last couple of years in
making sure that their hw just works with mainline kernels without
any downstream vendor specific patches.

>> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
>> personally don't think KEY_CONFIG matches well), but I would be
>> worried about clashing with existing functionality.

Using KEY_INFO / KEY_VENDOR works for me too. So maybe we should
just go with one of those 2 ?

Regards,

Hans

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Thu, Apr 11, 2024 at 02:30:35PM +0200, Hans de Goede wrote:
> Hi Dmitry,
> 
> On 4/11/24 2:02 AM, Dmitry Torokhov wrote:
> > On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
> >> Hi Dmitry
> >>
> >> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
> >>> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> >>>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> >>>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
> >>>>>> Hi Mark,
> >>>>>>
> >>>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> >>>>>>> Add support for new input events on Lenovo laptops that need exporting to
> >>>>>>> user space.
> >>>>>>>
> >>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
> >>>>>>> Add a new keycode to allow this to be used by userspace.
> >>>>>>
> >>>>>> What is the intended meaning of this keycode? How does it differ from
> >>>>>> the driver sending BTN_LEFT press/release twice?
> >>>>>>>
> >>>>>>> Lenovo support is using FN+N with Windows to collect needed details for
> >>>>>>> support cases. Add a keycode so that we'll be able to provide similar
> >>>>>>> support on Linux.
> >>>>>>
> >>>>>> Is there a userspace consumer for this?
> >>>>>
> >>>>> Funnily enough XKB has had a keysym for this for decades but it's not
> >>>>> hooked up anywhere due to the way it's pointer keys accessibility
> >>>>> feature was implemented. Theory is that most of userspace just needs
> >>>>> to patch the various pieces together for the new evdev code + keysym,
> >>>>> it's not really any different to handling a volume key (except this
> >>>>> one needs to be assignable).
> >>>>
> >>>> What is the keysym? If we can make them relatable to each other that
> >>>> would be good. Or maybe we could find a matching usage from HID usage
> >>>> tables...
> >>>
> >>> I was looking through the existing codes and I see:
> >>>
> >>> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
> >>>
> >>> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> >>> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> >>> specific debug info collection application (which I honestly doubt will
> >>> materialize).
> >>>
> >>
> >> That's a somewhat disappointing note on your doubts, is that based on
> >> anything? Just wondering what we've done to deserve that criticism.
> > 
> > Sorry, this was not meant as a criticism really, but you mentioned
> > yourself that there isn't anything in the works yet, you just have some
> > plans.
> > 
> > For such a project to succeed Lenovo needs to invest into selling
> > devices with Linux as a primary operating system, and it has to be
> > consumer segment (or small business, because for corporate they
> > typically roll their own support channels). The case of retrofitting
> > Linux onto a that device originally came with Windows OS rarely gets
> > much if any response from the normal support channels.
> > 
> > Is this something that is actually happening?
> 
> Yes, Lenovo is actually offering Fedora as an OS choice when
> ordering ThinkPads directly from their website in many countries
> including when ordering as a consumer.

Ah, very nice, I was not aware of this.

> 
> And unlike other vendor's Linux preloads which often use a kernel
> with downstream laptop specific changes these laptops are running
> unmodified Fedora kernels, which themselves are almost pristine
> upstream kernels.
> 
> Lenovo (Mark) has been really good the last couple of years in
> making sure that their hw just works with mainline kernels without
> any downstream vendor specific patches.
> 
> >> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
> >> personally don't think KEY_CONFIG matches well), but I would be
> >> worried about clashing with existing functionality.
> 
> Using KEY_INFO / KEY_VENDOR works for me too. So maybe we should
> just go with one of those 2 ?

It looks like Mark's preference is KEY_VENDOR, so let's go with it?

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de Goede]

Hi,

On 4/15/24 9:35 PM, Dmitry Torokhov wrote:
> On Thu, Apr 11, 2024 at 02:30:35PM +0200, Hans de Goede wrote:
>> Hi Dmitry,
>>
>> On 4/11/24 2:02 AM, Dmitry Torokhov wrote:
>>> On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
>>>> Hi Dmitry
>>>>
>>>> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
>>>>> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
>>>>>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
>>>>>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
>>>>>>>> Hi Mark,
>>>>>>>>
>>>>>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>>>>>>>>> Add support for new input events on Lenovo laptops that need exporting to
>>>>>>>>> user space.
>>>>>>>>>
>>>>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>>>>>>>> Add a new keycode to allow this to be used by userspace.
>>>>>>>>
>>>>>>>> What is the intended meaning of this keycode? How does it differ from
>>>>>>>> the driver sending BTN_LEFT press/release twice?
>>>>>>>>>
>>>>>>>>> Lenovo support is using FN+N with Windows to collect needed details for
>>>>>>>>> support cases. Add a keycode so that we'll be able to provide similar
>>>>>>>>> support on Linux.
>>>>>>>>
>>>>>>>> Is there a userspace consumer for this?
>>>>>>>
>>>>>>> Funnily enough XKB has had a keysym for this for decades but it's not
>>>>>>> hooked up anywhere due to the way it's pointer keys accessibility
>>>>>>> feature was implemented. Theory is that most of userspace just needs
>>>>>>> to patch the various pieces together for the new evdev code + keysym,
>>>>>>> it's not really any different to handling a volume key (except this
>>>>>>> one needs to be assignable).
>>>>>>
>>>>>> What is the keysym? If we can make them relatable to each other that
>>>>>> would be good. Or maybe we could find a matching usage from HID usage
>>>>>> tables...
>>>>>
>>>>> I was looking through the existing codes and I see:
>>>>>
>>>>> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
>>>>>
>>>>> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
>>>>> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
>>>>> specific debug info collection application (which I honestly doubt will
>>>>> materialize).
>>>>>
>>>>
>>>> That's a somewhat disappointing note on your doubts, is that based on
>>>> anything? Just wondering what we've done to deserve that criticism.
>>>
>>> Sorry, this was not meant as a criticism really, but you mentioned
>>> yourself that there isn't anything in the works yet, you just have some
>>> plans.
>>>
>>> For such a project to succeed Lenovo needs to invest into selling
>>> devices with Linux as a primary operating system, and it has to be
>>> consumer segment (or small business, because for corporate they
>>> typically roll their own support channels). The case of retrofitting
>>> Linux onto a that device originally came with Windows OS rarely gets
>>> much if any response from the normal support channels.
>>>
>>> Is this something that is actually happening?
>>
>> Yes, Lenovo is actually offering Fedora as an OS choice when
>> ordering ThinkPads directly from their website in many countries
>> including when ordering as a consumer.
> 
> Ah, very nice, I was not aware of this.
> 
>>
>> And unlike other vendor's Linux preloads which often use a kernel
>> with downstream laptop specific changes these laptops are running
>> unmodified Fedora kernels, which themselves are almost pristine
>> upstream kernels.
>>
>> Lenovo (Mark) has been really good the last couple of years in
>> making sure that their hw just works with mainline kernels without
>> any downstream vendor specific patches.
>>
>>>> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
>>>> personally don't think KEY_CONFIG matches well), but I would be
>>>> worried about clashing with existing functionality.
>>
>> Using KEY_INFO / KEY_VENDOR works for me too. So maybe we should
>> just go with one of those 2 ?
> 
> It looks like Mark's preference is KEY_VENDOR, so let's go with it?

Ack KEY_VENDOR sounds good to me for the doubletap on the trackpoint event.

What about the new Fn + N keycombo which also generates a WMI
event which we want to translate to a key code to launch a
(to be written) debug-info collecting app for when the customer
calls Lenovo support.

Mark suggested a new KEY_SYS_DEBUG_INFO for that. So do we use:

#define KEY_INFO                0x166   /* AL OEM Features/Tips/Tutorial */

for this, or do we define a new keycode ?

Mark would using KEY_INFO for this work for you.

Dmitry any opinion on this ?

Regards,

Hans

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Mon, Apr 15, 2024 at 09:47:10PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 4/15/24 9:35 PM, Dmitry Torokhov wrote:
> > On Thu, Apr 11, 2024 at 02:30:35PM +0200, Hans de Goede wrote:
> >> Hi Dmitry,
> >>
> >> On 4/11/24 2:02 AM, Dmitry Torokhov wrote:
> >>> On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
> >>>> Hi Dmitry
> >>>>
> >>>> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
> >>>>> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> >>>>>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> >>>>>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
> >>>>>>>> Hi Mark,
> >>>>>>>>
> >>>>>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> >>>>>>>>> Add support for new input events on Lenovo laptops that need exporting to
> >>>>>>>>> user space.
> >>>>>>>>>
> >>>>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
> >>>>>>>>> Add a new keycode to allow this to be used by userspace.
> >>>>>>>>
> >>>>>>>> What is the intended meaning of this keycode? How does it differ from
> >>>>>>>> the driver sending BTN_LEFT press/release twice?
> >>>>>>>>>
> >>>>>>>>> Lenovo support is using FN+N with Windows to collect needed details for
> >>>>>>>>> support cases. Add a keycode so that we'll be able to provide similar
> >>>>>>>>> support on Linux.
> >>>>>>>>
> >>>>>>>> Is there a userspace consumer for this?
> >>>>>>>
> >>>>>>> Funnily enough XKB has had a keysym for this for decades but it's not
> >>>>>>> hooked up anywhere due to the way it's pointer keys accessibility
> >>>>>>> feature was implemented. Theory is that most of userspace just needs
> >>>>>>> to patch the various pieces together for the new evdev code + keysym,
> >>>>>>> it's not really any different to handling a volume key (except this
> >>>>>>> one needs to be assignable).
> >>>>>>
> >>>>>> What is the keysym? If we can make them relatable to each other that
> >>>>>> would be good. Or maybe we could find a matching usage from HID usage
> >>>>>> tables...
> >>>>>
> >>>>> I was looking through the existing codes and I see:
> >>>>>
> >>>>> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
> >>>>>
> >>>>> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> >>>>> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> >>>>> specific debug info collection application (which I honestly doubt will
> >>>>> materialize).
> >>>>>
> >>>>
> >>>> That's a somewhat disappointing note on your doubts, is that based on
> >>>> anything? Just wondering what we've done to deserve that criticism.
> >>>
> >>> Sorry, this was not meant as a criticism really, but you mentioned
> >>> yourself that there isn't anything in the works yet, you just have some
> >>> plans.
> >>>
> >>> For such a project to succeed Lenovo needs to invest into selling
> >>> devices with Linux as a primary operating system, and it has to be
> >>> consumer segment (or small business, because for corporate they
> >>> typically roll their own support channels). The case of retrofitting
> >>> Linux onto a that device originally came with Windows OS rarely gets
> >>> much if any response from the normal support channels.
> >>>
> >>> Is this something that is actually happening?
> >>
> >> Yes, Lenovo is actually offering Fedora as an OS choice when
> >> ordering ThinkPads directly from their website in many countries
> >> including when ordering as a consumer.
> > 
> > Ah, very nice, I was not aware of this.
> > 
> >>
> >> And unlike other vendor's Linux preloads which often use a kernel
> >> with downstream laptop specific changes these laptops are running
> >> unmodified Fedora kernels, which themselves are almost pristine
> >> upstream kernels.
> >>
> >> Lenovo (Mark) has been really good the last couple of years in
> >> making sure that their hw just works with mainline kernels without
> >> any downstream vendor specific patches.
> >>
> >>>> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
> >>>> personally don't think KEY_CONFIG matches well), but I would be
> >>>> worried about clashing with existing functionality.
> >>
> >> Using KEY_INFO / KEY_VENDOR works for me too. So maybe we should
> >> just go with one of those 2 ?
> > 
> > It looks like Mark's preference is KEY_VENDOR, so let's go with it?
> 
> Ack KEY_VENDOR sounds good to me for the doubletap on the trackpoint event.
> 
> What about the new Fn + N keycombo which also generates a WMI
> event which we want to translate to a key code to launch a
> (to be written) debug-info collecting app for when the customer
> calls Lenovo support.
> 
> Mark suggested a new KEY_SYS_DEBUG_INFO for that. So do we use:
> 
> #define KEY_INFO                0x166   /* AL OEM Features/Tips/Tutorial */
> 
> for this, or do we define a new keycode ?
> 
> Mark would using KEY_INFO for this work for you.
> 
> Dmitry any opinion on this ?

No, my understanding is that Mark was OK with using KEY_VENDOR for Fn+N
combination that is supposed to start the utility that would collect
the debug info.

For double click there is still the discussion whether to have
KEY_DOUBLECLICK (which I think will need to be tied to the pointer
device somehow), or something else, like KEY_CONFIG or a new keycode if
we continue keeping it separate from the pointer operations and match
Windows behavior which invokes Lenovo configuration utility.

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Peter Hutterer]

On 10/04/2024 11:20, Dmitry Torokhov wrote:
> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
>>>> Hi Mark,
>>>>
>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>>>>> Add support for new input events on Lenovo laptops that need exporting to
>>>>> user space.
>>>>>
>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>>>> Add a new keycode to allow this to be used by userspace.
>>>>
>>>> What is the intended meaning of this keycode? How does it differ from
>>>> the driver sending BTN_LEFT press/release twice?
>>>>>
>>>>> Lenovo support is using FN+N with Windows to collect needed details for
>>>>> support cases. Add a keycode so that we'll be able to provide similar
>>>>> support on Linux.
>>>>
>>>> Is there a userspace consumer for this?
>>>
>>> Funnily enough XKB has had a keysym for this for decades but it's not
>>> hooked up anywhere due to the way it's pointer keys accessibility
>>> feature was implemented. Theory is that most of userspace just needs
>>> to patch the various pieces together for the new evdev code + keysym,
>>> it's not really any different to handling a volume key (except this
>>> one needs to be assignable).
>>
>> What is the keysym? If we can make them relatable to each other that
>> would be good. Or maybe we could find a matching usage from HID usage
>> tables...

There's a set of  XK_Pointer_ keysyms defined in X11/keysym.h, including XK_Pointer_DblClick1 and XK_Pointer_DblClickDefault.
Unfortunately they're not hooked up to anything atm, see this draft MR:
   https://gitlab.freedesktop.org/xkeyboard-config/xkeyboard-config/-/merge_requests/659
Because they're not hooked up anywhere they'll also need to be hooked into the client space, same as the various XF86FooBar symbols we've added over the years.

If we were to add KEY_DOUBLECLICK we can patch xkeyboard-config to bind that to the existing XK_Pointer_DblClickDefault symbol (it would get XF86DoubleClick assigned by the current automated scripts), so in theory that key would work like any other key with that symbol assigned.

> I was looking through the existing codes and I see:
> 
> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
> 
> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> specific debug info collection application (which I honestly doubt will
> materialize).

fwiw, I suggested KEY_DOUBLECLICK because that is the action the user takes. Whether that starts a particular application is mostly a question of configuration, defaulting to something that emulates a double-click seems prudent though. And if someone wants to remap that to the compose key that'd be trivial too then.

Cheers,
   Peter

Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry Torokhov]

On Wed, Apr 10, 2024 at 02:32:56PM +1000, Peter Hutterer wrote:
> On 10/04/2024 11:20, Dmitry Torokhov wrote:
> > On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> > > On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> > > > On 09/04/2024 09:31, Dmitry Torokhov wrote:
> > > > > Hi Mark,
> > > > > 
> > > > > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> > > > > > Add support for new input events on Lenovo laptops that need exporting to
> > > > > > user space.
> > > > > > 
> > > > > > Lenovo trackpoints are adding the ability to generate a doubletap event.
> > > > > > Add a new keycode to allow this to be used by userspace.
> > > > > 
> > > > > What is the intended meaning of this keycode? How does it differ from
> > > > > the driver sending BTN_LEFT press/release twice?
> > > > > > 
> > > > > > Lenovo support is using FN+N with Windows to collect needed details for
> > > > > > support cases. Add a keycode so that we'll be able to provide similar
> > > > > > support on Linux.
> > > > > 
> > > > > Is there a userspace consumer for this?
> > > > 
> > > > Funnily enough XKB has had a keysym for this for decades but it's not
> > > > hooked up anywhere due to the way it's pointer keys accessibility
> > > > feature was implemented. Theory is that most of userspace just needs
> > > > to patch the various pieces together for the new evdev code + keysym,
> > > > it's not really any different to handling a volume key (except this
> > > > one needs to be assignable).
> > > 
> > > What is the keysym? If we can make them relatable to each other that
> > > would be good. Or maybe we could find a matching usage from HID usage
> > > tables...
> 
> There's a set of  XK_Pointer_ keysyms defined in X11/keysym.h,
> including XK_Pointer_DblClick1 and XK_Pointer_DblClickDefault.
> Unfortunately they're not hooked up to anything atm, see this draft
> MR:
> https://gitlab.freedesktop.org/xkeyboard-config/xkeyboard-config/-/merge_requests/659
> Because they're not hooked up anywhere they'll also need to be hooked
> into the client space, same as the various XF86FooBar symbols we've
> added over the years.
> 
> If we were to add KEY_DOUBLECLICK we can patch xkeyboard-config to
> bind that to the existing XK_Pointer_DblClickDefault symbol (it would
> get XF86DoubleClick assigned by the current automated scripts), so in
> theory that key would work like any other key with that symbol
> assigned.
> 
> > I was looking through the existing codes and I see:
> > 
> > #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
> > 
> > We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> > thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> > specific debug info collection application (which I honestly doubt will
> > materialize).
> 
> fwiw, I suggested KEY_DOUBLECLICK because that is the action the user
> takes. Whether that starts a particular application is mostly a
> question of configuration, defaulting to something that emulates a
> double-click seems prudent though. And if someone wants to remap that
> to the compose key that'd be trivial too then.

I think whether to create and use KEY_DOUBLECLICK is very much depends
if we associate this with the pointer somehow, or if we keep it as a
completely separate action.

If we continue with KEY_DOUBLECLICK then we need to try and define what
exactly it means to the applications. Actually same goes if we want
another new keycode.

As far as easy remapping, I think one can map this to KEY_RESERVED and
then remap to whatever they want, you do not need to have a new keycode
for that.

Thanks.

-- 
Dmitry

[PATCH 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Mark Pearson]

Lenovo trackpoints are adding the ability to generate a doubletap event.
This handles the doubletap event and sends the KEY_DOUBLECLICK event to
userspace.

Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 82429e59999d..2bbb32c898e9 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -232,6 +232,7 @@ enum tpacpi_hkey_event_t {
 
 	/* Misc */
 	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
+	TP_HKEY_EV_TRACKPOINT_DOUBLETAP = 0x8036, /* doubletap on Trackpoint*/
 };
 
 /****************************************************************************
@@ -4081,6 +4082,22 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 				break;
 			}
 			fallthrough;	/* to default */
+		case 8:
+			/* 0x8036: Trackpoint doubletaps */
+			if (hkey == TP_HKEY_EV_TRACKPOINT_DOUBLETAP) {
+				send_acpi_ev = true;
+				ignore_acpi_ev = false;
+				known_ev = true;
+				/* Send to user space */
+				mutex_lock(&tpacpi_inputdev_send_mutex);
+				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 1);
+				input_sync(tpacpi_inputdev);
+				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 0);
+				input_sync(tpacpi_inputdev);
+				mutex_unlock(&tpacpi_inputdev_send_mutex);
+				break;
+			}
+			fallthrough;	/* to default */
 		default:
 			known_ev = false;
 		}
-- 
2.44.0

Re: [PATCH 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Hans de Goede]

Hi Mark,

On 3/24/24 10:07 PM, Mark Pearson wrote:
> Lenovo trackpoints are adding the ability to generate a doubletap event.
> This handles the doubletap event and sends the KEY_DOUBLECLICK event to
> userspace.
> 
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 82429e59999d..2bbb32c898e9 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -232,6 +232,7 @@ enum tpacpi_hkey_event_t {
>  
>  	/* Misc */
>  	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
> +	TP_HKEY_EV_TRACKPOINT_DOUBLETAP = 0x8036, /* doubletap on Trackpoint*/
>  };
>  
>  /****************************************************************************
> @@ -4081,6 +4082,22 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
>  				break;
>  			}
>  			fallthrough;	/* to default */

This now no longer fallsthrough to default. IMHO the best thing to do
here is add a new preparation patch which initializes known_ev to false
inside the while before the switch-case (together with the send_acpi_ev
and ignore_acpi_ev init). and then change this fallthrough to a break
in the preparation patch. You can then also remove the default case
altogether in this prep patch.

> +		case 8:
> +			/* 0x8036: Trackpoint doubletaps */
> +			if (hkey == TP_HKEY_EV_TRACKPOINT_DOUBLETAP) {
> +				send_acpi_ev = true;
> +				ignore_acpi_ev = false;

These 2 values are set as the default above the switch-case, please
drop these 2 lines.

> +				known_ev = true;
> +				/* Send to user space */
> +				mutex_lock(&tpacpi_inputdev_send_mutex);
> +				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 1);
> +				input_sync(tpacpi_inputdev);
> +				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 0);
> +				input_sync(tpacpi_inputdev);
> +				mutex_unlock(&tpacpi_inputdev_send_mutex);

This code duplicates tpacpi_input_send_key(), what you want to do here
is define a hotkey_keycode_map scancode range for new 0x8xxx codes like how this
was done when extended scancodes where added to deal with the new 0x13xx hotkey
event codes for the 2017+ models.

See commit 696c6523ec8f ("platform/x86: thinkpad_acpi: add mapping for new hotkeys")

Despite re-using tpacpi_input_send_key() there are 2 reasons why we want
scancodes for these new "keys".

1. By adding the keys to the hotkey_keycode_map they automatically
also get input_set_capability(tpacpi_inputdev, EV_KEY, hotkey_keycode_map[i]);
called on them advertising to userspace that tpacpi_inputdev can actually
generate these keypresses. Something which is currently lacking from your
patch. Related to this did you test this with evtest? I think that the input
core will suppress the events when you do not set the capability ?

2. This allows remapping scancodes to different KEY_foo values with hwdb
entries.

Regards,

Hans








> +				break;
> +			}
> +			fallthrough;	/* to default */
>  		default:
>  			known_ev = false;
>  		}

Re: [ibm-acpi-devel] [PATCH 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Mark Pearson]

Hi Hans,

Many thanks for the review.

On Mon, Apr 8, 2024, at 9:04 AM, Hans de Goede wrote:
> Hi Mark,
>
> On 3/24/24 10:07 PM, Mark Pearson wrote:
>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>> This handles the doubletap event and sends the KEY_DOUBLECLICK event to
>> userspace.
>> 
>> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>> 
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index 82429e59999d..2bbb32c898e9 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -232,6 +232,7 @@ enum tpacpi_hkey_event_t {
>>  
>>  	/* Misc */
>>  	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
>> +	TP_HKEY_EV_TRACKPOINT_DOUBLETAP = 0x8036, /* doubletap on Trackpoint*/
>>  };
>>  
>>  /****************************************************************************
>> @@ -4081,6 +4082,22 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
>>  				break;
>>  			}
>>  			fallthrough;	/* to default */
>
> This now no longer fallsthrough to default. IMHO the best thing to do
> here is add a new preparation patch which initializes known_ev to false
> inside the while before the switch-case (together with the send_acpi_ev
> and ignore_acpi_ev init). and then change this fallthrough to a break
> in the preparation patch. You can then also remove the default case
> altogether in this prep patch.
>
Ack - that makes sense. I'll look at doing that.

>> +		case 8:
>> +			/* 0x8036: Trackpoint doubletaps */
>> +			if (hkey == TP_HKEY_EV_TRACKPOINT_DOUBLETAP) {
>> +				send_acpi_ev = true;
>> +				ignore_acpi_ev = false;
>
> These 2 values are set as the default above the switch-case, please
> drop these 2 lines.

Agreed. Will change.

>
>> +				known_ev = true;
>> +				/* Send to user space */
>> +				mutex_lock(&tpacpi_inputdev_send_mutex);
>> +				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 1);
>> +				input_sync(tpacpi_inputdev);
>> +				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 0);
>> +				input_sync(tpacpi_inputdev);
>> +				mutex_unlock(&tpacpi_inputdev_send_mutex);
>
> This code duplicates tpacpi_input_send_key(), what you want to do here
> is define a hotkey_keycode_map scancode range for new 0x8xxx codes like how this
> was done when extended scancodes where added to deal with the new 0x13xx hotkey
> event codes for the 2017+ models.
>
> See commit 696c6523ec8f ("platform/x86: thinkpad_acpi: add mapping for 
> new hotkeys")
>
> Despite re-using tpacpi_input_send_key() there are 2 reasons why we want
> scancodes for these new "keys".
>
> 1. By adding the keys to the hotkey_keycode_map they automatically
> also get input_set_capability(tpacpi_inputdev, EV_KEY, hotkey_keycode_map[i]);
> called on them advertising to userspace that tpacpi_inputdev can actually
> generate these keypresses. Something which is currently lacking from your
> patch. Related to this did you test this with evtest? I think that the input
> core will suppress the events when you do not set the capability ?
>
> 2. This allows remapping scancodes to different KEY_foo values with hwdb
> entries.
>
Will look into doing this.
There was a reason originally I did it like this, but I can't remember what it was. I'll revisit it.

I did test with evtest but I ended up having to cheat as there's quite a few layers in userspace and I got a bit bogged down chewing my way through those (building them against the right headers etc). 
I ended up using an already existing code to make sure it was doing the right thing in the driver - and then assumed that once the keycode was 'released', and the different user space projects updated per normal procedure, it would work. It's possible it meant I bypassed/missed this issue so I'll retry once I've made the updates.

Mark

[PATCH 3/4] platform/x86: thinkpad_acpi: Support for system debug info hotkey

[Mark Pearson]

New Lenovo platforms are adding the FN+N key to generate system debug
details that support can use for collecting important details on any
customer cases for Windows.
Add the infrastructure so we can do the same on Linux by generating a
SYS_DEBUG_INFO keycode to userspace.

Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Nitin Joshi <njoshi1@lenovo.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 2bbb32c898e9..854ce971bde2 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -1787,6 +1787,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
 	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
 	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
 	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
+	TP_ACPI_HOTKEYSCAN_SYS_DEBUG_INFO = 81,
 
 	/* Hotkey keymap size */
 	TPACPI_HOTKEY_MAP_LEN
@@ -3337,6 +3338,9 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 		KEY_NOTIFICATION_CENTER,	/* Notification Center */
 		KEY_PICKUP_PHONE,		/* Answer incoming call */
 		KEY_HANGUP_PHONE,		/* Decline incoming call */
+		KEY_UNKNOWN,			/* AMT Toggle (event), 0x31A */
+		KEY_UNKNOWN, KEY_UNKNOWN,
+		KEY_SYS_DEBUG_INFO,             /* System debug info, 0x31D */
 		},
 	};
 
-- 
2.44.0

Re: [PATCH 3/4] platform/x86: thinkpad_acpi: Support for system debug info hotkey

[Hans de Goede]

Hi,

On 3/24/24 10:08 PM, Mark Pearson wrote:
> New Lenovo platforms are adding the FN+N key to generate system debug
> details that support can use for collecting important details on any
> customer cases for Windows.
> Add the infrastructure so we can do the same on Linux by generating a
> SYS_DEBUG_INFO keycode to userspace.
> 
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Nitin Joshi <njoshi1@lenovo.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 2bbb32c898e9..854ce971bde2 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -1787,6 +1787,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>  	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
>  	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
>  	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
> +	TP_ACPI_HOTKEYSCAN_SYS_DEBUG_INFO = 81,
>  
>  	/* Hotkey keymap size */
>  	TPACPI_HOTKEY_MAP_LEN
> @@ -3337,6 +3338,9 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
>  		KEY_NOTIFICATION_CENTER,	/* Notification Center */
>  		KEY_PICKUP_PHONE,		/* Answer incoming call */
>  		KEY_HANGUP_PHONE,		/* Decline incoming call */
> +		KEY_UNKNOWN,			/* AMT Toggle (event), 0x31A */
> +		KEY_UNKNOWN, KEY_UNKNOWN,
> +		KEY_SYS_DEBUG_INFO,             /* System debug info, 0x31D */
>  		},
>  	};
>  

Looking at the next patch 0x131c is TP_HKEY_EV_DOUBLETAP_TOGGLE and 0x131a is
TP_HKEY_EV_AMT_TOGGLE based on this please change this to:

  		KEY_NOTIFICATION_CENTER,	/* Notification Center */
  		KEY_PICKUP_PHONE,		/* Answer incoming call */
  		KEY_HANGUP_PHONE,		/* Decline incoming call */
		KEY_UNKNOWN,			/* TP_HKEY_EV_AMT_TOGGLE handled in driver, 0x31a */
		KEY_UNKNOWN,			/* ?, 0X31b */
		KEY_UNKNOWN,			/* TP_HKEY_EV_DOUBLETAP_TOGGLE handled in driver, 0x31c */
		KEY_SYS_DEBUG_INFO,             /* System debug info, 0x31d */
		},

Regards,

Hans

Re: [PATCH 3/4] platform/x86: thinkpad_acpi: Support for system debug info hotkey

[Mark Pearson]

Thanks Hans

On Mon, Apr 8, 2024, at 9:11 AM, Hans de Goede wrote:
> Hi,
>
> On 3/24/24 10:08 PM, Mark Pearson wrote:
>> New Lenovo platforms are adding the FN+N key to generate system debug
>> details that support can use for collecting important details on any
>> customer cases for Windows.
>> Add the infrastructure so we can do the same on Linux by generating a
>> SYS_DEBUG_INFO keycode to userspace.
>> 
>> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Nitin Joshi <njoshi1@lenovo.com>
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index 2bbb32c898e9..854ce971bde2 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -1787,6 +1787,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>>  	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
>>  	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
>>  	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
>> +	TP_ACPI_HOTKEYSCAN_SYS_DEBUG_INFO = 81,
>>  
>>  	/* Hotkey keymap size */
>>  	TPACPI_HOTKEY_MAP_LEN
>> @@ -3337,6 +3338,9 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
>>  		KEY_NOTIFICATION_CENTER,	/* Notification Center */
>>  		KEY_PICKUP_PHONE,		/* Answer incoming call */
>>  		KEY_HANGUP_PHONE,		/* Decline incoming call */
>> +		KEY_UNKNOWN,			/* AMT Toggle (event), 0x31A */
>> +		KEY_UNKNOWN, KEY_UNKNOWN,
>> +		KEY_SYS_DEBUG_INFO,             /* System debug info, 0x31D */
>>  		},
>>  	};
>>  
>
> Looking at the next patch 0x131c is TP_HKEY_EV_DOUBLETAP_TOGGLE and 0x131a is
> TP_HKEY_EV_AMT_TOGGLE based on this please change this to:
>
>   		KEY_NOTIFICATION_CENTER,	/* Notification Center */
>   		KEY_PICKUP_PHONE,		/* Answer incoming call */
>   		KEY_HANGUP_PHONE,		/* Decline incoming call */
> 		KEY_UNKNOWN,			/* TP_HKEY_EV_AMT_TOGGLE handled in driver, 0x31a */
> 		KEY_UNKNOWN,			/* ?, 0X31b */
> 		KEY_UNKNOWN,			/* TP_HKEY_EV_DOUBLETAP_TOGGLE handled in driver, 0x31c */
> 		KEY_SYS_DEBUG_INFO,             /* System debug info, 0x31d */
> 		},
>
Will do

Mark

[PATCH 4/4] platform/x86: thinkpad_acpi: Support hotkey to disable trackpoint doubletap

[Mark Pearson]

The hotkey combination FN+G can be used to disable the trackpoint
doubletap feature on Windows.
Add matching functionality for Linux.

Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 854ce971bde2..21756aa3d28d 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -167,6 +167,7 @@ enum tpacpi_hkey_event_t {
 	TP_HKEY_EV_VOL_MUTE		= 0x1017, /* Mixer output mute */
 	TP_HKEY_EV_PRIVACYGUARD_TOGGLE	= 0x130f, /* Toggle priv.guard on/off */
 	TP_HKEY_EV_AMT_TOGGLE		= 0x131a, /* Toggle AMT on/off */
+	TP_HKEY_EV_DOUBLETAP_TOGGLE	= 0x131c, /* Toggle trackpoint doubletap on/off */
 	TP_HKEY_EV_PROFILE_TOGGLE	= 0x131f, /* Toggle platform profile */
 
 	/* Reasons for waking up from S3/S4 */
@@ -354,6 +355,7 @@ static struct {
 	u32 hotkey_poll_active:1;
 	u32 has_adaptive_kbd:1;
 	u32 kbd_lang:1;
+	u32 trackpoint_doubletap:1;
 	struct quirk_entry *quirks;
 } tp_features;
 
@@ -3598,6 +3600,9 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 
 	hotkey_poll_setup_safe(true);
 
+	/* Enable doubletap by default */
+	tp_features.trackpoint_doubletap = 1;
+
 	return 0;
 }
 
@@ -3739,6 +3744,7 @@ static bool hotkey_notify_extended_hotkey(const u32 hkey)
 	case TP_HKEY_EV_PRIVACYGUARD_TOGGLE:
 	case TP_HKEY_EV_AMT_TOGGLE:
 	case TP_HKEY_EV_PROFILE_TOGGLE:
+	case TP_HKEY_EV_DOUBLETAP_TOGGLE:
 		tpacpi_driver_event(hkey);
 		return true;
 	}
@@ -4092,13 +4098,15 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 				send_acpi_ev = true;
 				ignore_acpi_ev = false;
 				known_ev = true;
-				/* Send to user space */
-				mutex_lock(&tpacpi_inputdev_send_mutex);
-				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 1);
-				input_sync(tpacpi_inputdev);
-				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 0);
-				input_sync(tpacpi_inputdev);
-				mutex_unlock(&tpacpi_inputdev_send_mutex);
+				if (tp_features.trackpoint_doubletap) {
+					/* Send to user space */
+					mutex_lock(&tpacpi_inputdev_send_mutex);
+					input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 1);
+					input_sync(tpacpi_inputdev);
+					input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 0);
+					input_sync(tpacpi_inputdev);
+					mutex_unlock(&tpacpi_inputdev_send_mutex);
+				}
 				break;
 			}
 			fallthrough;	/* to default */
@@ -11228,6 +11236,8 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
 		/* Notify user space the profile changed */
 		platform_profile_notify();
 	}
+	if (hkey_event == TP_HKEY_EV_DOUBLETAP_TOGGLE)
+		tp_features.trackpoint_doubletap = !tp_features.trackpoint_doubletap;
 }
 
 static void hotkey_driver_event(const unsigned int scancode)
-- 
2.44.0

Re: [PATCH 4/4] platform/x86: thinkpad_acpi: Support hotkey to disable trackpoint doubletap

[Hans de Goede]

Hi,

On 3/24/24 10:08 PM, Mark Pearson wrote:
> The hotkey combination FN+G can be used to disable the trackpoint
> doubletap feature on Windows.
> Add matching functionality for Linux.
> 
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 854ce971bde2..21756aa3d28d 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -167,6 +167,7 @@ enum tpacpi_hkey_event_t {
>  	TP_HKEY_EV_VOL_MUTE		= 0x1017, /* Mixer output mute */
>  	TP_HKEY_EV_PRIVACYGUARD_TOGGLE	= 0x130f, /* Toggle priv.guard on/off */
>  	TP_HKEY_EV_AMT_TOGGLE		= 0x131a, /* Toggle AMT on/off */
> +	TP_HKEY_EV_DOUBLETAP_TOGGLE	= 0x131c, /* Toggle trackpoint doubletap on/off */
>  	TP_HKEY_EV_PROFILE_TOGGLE	= 0x131f, /* Toggle platform profile */
>  
>  	/* Reasons for waking up from S3/S4 */
> @@ -354,6 +355,7 @@ static struct {
>  	u32 hotkey_poll_active:1;
>  	u32 has_adaptive_kbd:1;
>  	u32 kbd_lang:1;
> +	u32 trackpoint_doubletap:1;
>  	struct quirk_entry *quirks;
>  } tp_features;
>  
> @@ -3598,6 +3600,9 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
>  
>  	hotkey_poll_setup_safe(true);
>  
> +	/* Enable doubletap by default */
> +	tp_features.trackpoint_doubletap = 1;
> +
>  	return 0;
>  }
>  
> @@ -3739,6 +3744,7 @@ static bool hotkey_notify_extended_hotkey(const u32 hkey)
>  	case TP_HKEY_EV_PRIVACYGUARD_TOGGLE:
>  	case TP_HKEY_EV_AMT_TOGGLE:
>  	case TP_HKEY_EV_PROFILE_TOGGLE:
> +	case TP_HKEY_EV_DOUBLETAP_TOGGLE:
>  		tpacpi_driver_event(hkey);
>  		return true;
>  	}
> @@ -4092,13 +4098,15 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
>  				send_acpi_ev = true;
>  				ignore_acpi_ev = false;
>  				known_ev = true;
> -				/* Send to user space */
> -				mutex_lock(&tpacpi_inputdev_send_mutex);
> -				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 1);
> -				input_sync(tpacpi_inputdev);
> -				input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 0);
> -				input_sync(tpacpi_inputdev);
> -				mutex_unlock(&tpacpi_inputdev_send_mutex);
> +				if (tp_features.trackpoint_doubletap) {
> +					/* Send to user space */
> +					mutex_lock(&tpacpi_inputdev_send_mutex);
> +					input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 1);
> +					input_sync(tpacpi_inputdev);
> +					input_report_key(tpacpi_inputdev, KEY_DOUBLECLICK, 0);
> +					input_sync(tpacpi_inputdev);
> +					mutex_unlock(&tpacpi_inputdev_send_mutex);
> +				}
>  				break;
>  			}
>  			fallthrough;	/* to default */

This chunk will need to change after incorporating my review comments into
patch 2/4. With that said this looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> @@ -11228,6 +11236,8 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
>  		/* Notify user space the profile changed */
>  		platform_profile_notify();
>  	}
> +	if (hkey_event == TP_HKEY_EV_DOUBLETAP_TOGGLE)
> +		tp_features.trackpoint_doubletap = !tp_features.trackpoint_doubletap;
>  }
>  
>  static void hotkey_driver_event(const unsigned int scancode)