diff mbox series

platform/x86: thinkpad_acpi: handle HKEY 0x4012, 0x4013 events

Message ID 53abdd94-8df4-cc1c-84e9-221face6b07c@a-kobel.de (mailing list archive)
State Changes Requested, archived
Headers show
Series platform/x86: thinkpad_acpi: handle HKEY 0x4012, 0x4013 events | expand

Commit Message

Alexander Kobel Feb. 7, 2021, 4:34 p.m. UTC
Those events occur when a keyboard cover is attached to a ThinkPad
Tablet device.  Typically, they are used to switch from normal to tablet
mode in userspace; e.g., to offer touch keyboard choices when focus goes
to a text box and no keyboard is attached, or to enable autorotation of
the display according to the builtin orientation sensor.

No attempt is taken to emit an EV_SW event for SW_TABLET_MODE; this is
left to userspace.  So this patch is mainly to avoid warnings about
unknown and unhandled events, which are now reported as:

* Event 0x4012: attached keyboard cover
* Event 0x4013: detached keyboard cover

Tested as working on a ThinkPad X1 Tablet Gen 2, 20JCS00C00, and as
non-interfering with a ThinkPad X1 Carbon 7th, 20QESABM02 (normal
clamshell, so it does not have a keyboard cover).

Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
---
 drivers/platform/x86/thinkpad_acpi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

--
2.30.0

Comments

Hans de Goede Feb. 7, 2021, 5:10 p.m. UTC | #1
Hi,

On 2/7/21 5:34 PM, Alexander Kobel wrote:
> Those events occur when a keyboard cover is attached to a ThinkPad
> Tablet device.  Typically, they are used to switch from normal to tablet
> mode in userspace; e.g., to offer touch keyboard choices when focus goes
> to a text box and no keyboard is attached, or to enable autorotation of
> the display according to the builtin orientation sensor.

Thank you for your patch.

> No attempt is taken to emit an EV_SW event for SW_TABLET_MODE; this is
> left to userspace.

I don't understand this part, in order for userspace to respond to these
events the thinkpad_acpi driver needs to emit events for this; and emitting
SW_TABLET_MODE seems like it is the right thing to do.

Why are you not doing this ?

Note that it is important to only advertise SW_TABLET_MODE functionality
on devices where it actually works. Which might be challenging I guess...

But we have contacts inside Lenovo now, so perhaps they can help.

Mark, Nitin, is there a way for the thinkpad_acpi code to figure out
if 0x4012 / 0x4013 events will be send by a device?

Also is there a way to get the current state of the
keyboard-cover being attached at boot or not ?

Regards,

Hans



> So this patch is mainly to avoid warnings about
> unknown and unhandled events, which are now reported as:
> 
> * Event 0x4012: attached keyboard cover
> * Event 0x4013: detached keyboard cover
> 
> Tested as working on a ThinkPad X1 Tablet Gen 2, 20JCS00C00, and as
> non-interfering with a ThinkPad X1 Carbon 7th, 20QESABM02 (normal
> clamshell, so it does not have a keyboard cover).
> 
> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index c404706379d9..fd5322b5bbbd 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -174,6 +174,8 @@ enum tpacpi_hkey_event_t {
>                                                      or port replicator */
>         TP_HKEY_EV_HOTPLUG_UNDOCK       = 0x4011, /* undocked from hotplug
>                                                      dock or port replicator */
> +       TP_HKEY_EV_KBD_COVER_ATTACH     = 0x4012, /* attached keyboard cover */
> +       TP_HKEY_EV_KBD_COVER_DETACH     = 0x4013, /* detached keyboard cover */
> 
>         /* User-interface events */
>         TP_HKEY_EV_LID_CLOSE            = 0x5001, /* laptop lid closed */
> @@ -3989,6 +3991,12 @@ static bool hotkey_notify_dockevent(const u32 hkey,
>         case TP_HKEY_EV_HOTPLUG_UNDOCK: /* undocked from port replicator */
>                 pr_info("undocked from hotplug port replicator\n");
>                 return true;
> +       case TP_HKEY_EV_KBD_COVER_ATTACH:
> +               pr_info("attached keyboard cover\n");
> +               return true;
> +       case TP_HKEY_EV_KBD_COVER_DETACH:
> +               pr_info("detached keyboard cover\n");
> +               return true;
> 
>         default:
>                 return false;
> --
> 2.30.0
>
Alexander Kobel Feb. 7, 2021, 5:55 p.m. UTC | #2
Hi,

On 2/7/21 6:10 PM, Hans de Goede wrote:
> Hi,
> 
> On 2/7/21 5:34 PM, Alexander Kobel wrote:
>> Those events occur when a keyboard cover is attached to a ThinkPad
>> Tablet device.  Typically, they are used to switch from normal to tablet
>> mode in userspace; e.g., to offer touch keyboard choices when focus goes
>> to a text box and no keyboard is attached, or to enable autorotation of
>> the display according to the builtin orientation sensor.
> 
> Thank you for your patch.

Thank you for your swift response.

>> No attempt is taken to emit an EV_SW event for SW_TABLET_MODE; this is
>> left to userspace.
> 
> I don't understand this part, in order for userspace to respond to these
> events the thinkpad_acpi driver needs to emit events for this; and emitting
> SW_TABLET_MODE seems like it is the right thing to do.
> 
> Why are you not doing this ?

Quite frankly, because I did not know how to, reliably.  (First ever
patch attempt to the kernel here, so I'd rather err on the safe side.)

There are a number of events (e.g., TP_HKEY_EV_HOTPLUG_DOCK) that do not
propagate to userspace, but only report a specific message.  I figured
it's better to have a meaningful entry in the log rather than just the
warning about an unknown event.
But on second thought: pretending to react, but not actually doing
something, isn't too valuable.

I'll go off and try to improve.

> Note that it is important to only advertise SW_TABLET_MODE functionality
> on devices where it actually works. Which might be challenging I guess...

Right.  I only have a two devices to test, one of them a classic laptop,
so any input is greatly appreciated.

> But we have contacts inside Lenovo now, so perhaps they can help.
> 
> Mark, Nitin, is there a way for the thinkpad_acpi code to figure out
> if 0x4012 / 0x4013 events will be send by a device?
> 
> Also is there a way to get the current state of the
> keyboard-cover being attached at boot or not ?

IIUC, tpacpi_input_send_tabletsw() will not work as-is on this device,
as it should do for TP_HKEY_EV_TABLET_TABLET on X41t-X61t; mostly,
because I'm not aware of a method to read the current state.
Nevertheless, at least emitting events on change should be doable.


Finally, I mentioned some open ends already on a post to ibm-acpi-devel
at https://sourceforge.net/p/ibm-acpi/mailman/message/37200082/; this
very question is among them.
I will start tackling the SW_TABLET_MODE event issue first, but if Mark
and Nitin can already hint about the keyboard shortcuts, it'd be highly
appreciated:


This [patch] is pretty much trivial and conservative, I assume.
However, it's not 100% clear to me whether emitting a EV_SW event for
SW_TABLET_MODE would be in order, as mentioned in the description.  If
so, I'd have a deeper look on how to do that, and perhaps require some
hand-holding.


Besides, the X1 Tablet has a few Fn+smth combos that are not working
currently, and instead report (in evtest):

Input driver version is 1.0.1
Input device ID: bus 0x3 vendor 0x17ef product 0x60a3 version 0x111
Input device name: "PRIMAX ThinkPad X1 Tablet Thin Keyboard Gen 2
Consumer Control"
type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001
type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1

Keys affected are Fn+Esc (Fn lock) and Fn+F7 - Fn+F12 (various settings
like radio/bluetooth on/off).

Also, the mute and micmute leds are not working, and Fn lock led and
function is unavailable.


I'd love to give those minor issues a shot, too, but I'm not sure where
to start.  Would this go via thinkpad_acpi or hid_lenovo [or hid_primax]?


Cheers,
Alex


> Regards,
> 
> Hans
> 
> 
> 
>> So this patch is mainly to avoid warnings about
>> unknown and unhandled events, which are now reported as:
>>
>> * Event 0x4012: attached keyboard cover
>> * Event 0x4013: detached keyboard cover
>>
>> Tested as working on a ThinkPad X1 Tablet Gen 2, 20JCS00C00, and as
>> non-interfering with a ThinkPad X1 Carbon 7th, 20QESABM02 (normal
>> clamshell, so it does not have a keyboard cover).
>>
>> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index c404706379d9..fd5322b5bbbd 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -174,6 +174,8 @@ enum tpacpi_hkey_event_t {
>>                                                      or port replicator */
>>         TP_HKEY_EV_HOTPLUG_UNDOCK       = 0x4011, /* undocked from hotplug
>>                                                      dock or port replicator */
>> +       TP_HKEY_EV_KBD_COVER_ATTACH     = 0x4012, /* attached keyboard cover */
>> +       TP_HKEY_EV_KBD_COVER_DETACH     = 0x4013, /* detached keyboard cover */
>>
>>         /* User-interface events */
>>         TP_HKEY_EV_LID_CLOSE            = 0x5001, /* laptop lid closed */
>> @@ -3989,6 +3991,12 @@ static bool hotkey_notify_dockevent(const u32 hkey,
>>         case TP_HKEY_EV_HOTPLUG_UNDOCK: /* undocked from port replicator */
>>                 pr_info("undocked from hotplug port replicator\n");
>>                 return true;
>> +       case TP_HKEY_EV_KBD_COVER_ATTACH:
>> +               pr_info("attached keyboard cover\n");
>> +               return true;
>> +       case TP_HKEY_EV_KBD_COVER_DETACH:
>> +               pr_info("detached keyboard cover\n");
>> +               return true;
>>
>>         default:
>>                 return false;
>> --
>> 2.30.0
>>
>
Nitin Joshi1 Feb. 8, 2021, 7:17 a.m. UTC | #3
Hello Hans,

>-----Original Message-----
>From: Hans de Goede <hdegoede@redhat.com>
>Sent: Monday, February 8, 2021 2:10 AM
>To: Alexander Kobel <a-kobel@a-kobel.de>; platform-driver-
>x86@vger.kernel.org; Mark Pearson <mpearson@lenovo.com>; Nitin Joshi1
><njoshi1@lenovo.com>
>Subject: [External] Re: [PATCH] platform/x86: thinkpad_acpi: handle HKEY
>0x4012, 0x4013 events
>
>Hi,
>
>On 2/7/21 5:34 PM, Alexander Kobel wrote:
>> Those events occur when a keyboard cover is attached to a ThinkPad
>> Tablet device.  Typically, they are used to switch from normal to
>> tablet mode in userspace; e.g., to offer touch keyboard choices when
>> focus goes to a text box and no keyboard is attached, or to enable
>> autorotation of the display according to the builtin orientation sensor.
>
>Thank you for your patch.
>
>> No attempt is taken to emit an EV_SW event for SW_TABLET_MODE; this is
>> left to userspace.
>
>I don't understand this part, in order for userspace to respond to these events
>the thinkpad_acpi driver needs to emit events for this; and emitting
>SW_TABLET_MODE seems like it is the right thing to do.
>
>Why are you not doing this ?
>
>Note that it is important to only advertise SW_TABLET_MODE functionality on
>devices where it actually works. Which might be challenging I guess...
>
>But we have contacts inside Lenovo now, so perhaps they can help.
>
>Mark, Nitin, is there a way for the thinkpad_acpi code to figure out if 0x4012 /
>0x4013 events will be send by a device?

It seems , these events are used for not only keyboard cover, but also other tablet options.
In attached document, Interface type 4 (Graft type) is of ThinkPad X1 Tablet Series.

>
>Also is there a way to get the current state of the keyboard-cover being
>attached at boot or not ?
It seems "GTOP" ASL method can be used to get current state.

>
>Regards,
>
>Hans

Thanks & Regards,
Nitin Joshi 
>
>
>
>> So this patch is mainly to avoid warnings about unknown and unhandled
>> events, which are now reported as:
>>
>> * Event 0x4012: attached keyboard cover
>> * Event 0x4013: detached keyboard cover
>>
>> Tested as working on a ThinkPad X1 Tablet Gen 2, 20JCS00C00, and as
>> non-interfering with a ThinkPad X1 Carbon 7th, 20QESABM02 (normal
>> clamshell, so it does not have a keyboard cover).
>>
>> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c
>> b/drivers/platform/x86/thinkpad_acpi.c
>> index c404706379d9..fd5322b5bbbd 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -174,6 +174,8 @@ enum tpacpi_hkey_event_t {
>>                                                      or port replicator */
>>         TP_HKEY_EV_HOTPLUG_UNDOCK       = 0x4011, /* undocked from
>hotplug
>>                                                      dock or port
>> replicator */
>> +       TP_HKEY_EV_KBD_COVER_ATTACH     = 0x4012, /* attached keyboard
>cover */
>> +       TP_HKEY_EV_KBD_COVER_DETACH     = 0x4013, /* detached keyboard
>cover */
>>
>>         /* User-interface events */
>>         TP_HKEY_EV_LID_CLOSE            = 0x5001, /* laptop lid closed */
>> @@ -3989,6 +3991,12 @@ static bool hotkey_notify_dockevent(const u32
>hkey,
>>         case TP_HKEY_EV_HOTPLUG_UNDOCK: /* undocked from port
>replicator */
>>                 pr_info("undocked from hotplug port replicator\n");
>>                 return true;
>> +       case TP_HKEY_EV_KBD_COVER_ATTACH:
>> +               pr_info("attached keyboard cover\n");
>> +               return true;
>> +       case TP_HKEY_EV_KBD_COVER_DETACH:
>> +               pr_info("detached keyboard cover\n");
>> +               return true;
>>
>>         default:
>>                 return false;
>> --
>> 2.30.0
>>
Hans de Goede Feb. 8, 2021, 9:04 a.m. UTC | #4
Hi Nitin, Alexander,

On 2/8/21 8:17 AM, Nitin Joshi1 wrote:
> Hello Hans,
> 
>> -----Original Message-----
>> From: Hans de Goede <hdegoede@redhat.com>
>> Sent: Monday, February 8, 2021 2:10 AM
>> To: Alexander Kobel <a-kobel@a-kobel.de>; platform-driver-
>> x86@vger.kernel.org; Mark Pearson <mpearson@lenovo.com>; Nitin Joshi1
>> <njoshi1@lenovo.com>
>> Subject: [External] Re: [PATCH] platform/x86: thinkpad_acpi: handle HKEY
>> 0x4012, 0x4013 events
>>
>> Hi,
>>
>> On 2/7/21 5:34 PM, Alexander Kobel wrote:
>>> Those events occur when a keyboard cover is attached to a ThinkPad
>>> Tablet device.  Typically, they are used to switch from normal to
>>> tablet mode in userspace; e.g., to offer touch keyboard choices when
>>> focus goes to a text box and no keyboard is attached, or to enable
>>> autorotation of the display according to the builtin orientation sensor.
>>
>> Thank you for your patch.
>>
>>> No attempt is taken to emit an EV_SW event for SW_TABLET_MODE; this is
>>> left to userspace.
>>
>> I don't understand this part, in order for userspace to respond to these events
>> the thinkpad_acpi driver needs to emit events for this; and emitting
>> SW_TABLET_MODE seems like it is the right thing to do.
>>
>> Why are you not doing this ?
>>
>> Note that it is important to only advertise SW_TABLET_MODE functionality on
>> devices where it actually works. Which might be challenging I guess...
>>
>> But we have contacts inside Lenovo now, so perhaps they can help.
>>
>> Mark, Nitin, is there a way for the thinkpad_acpi code to figure out if 0x4012 /
>> 0x4013 events will be send by a device?
> 
> It seems , these events are used for not only keyboard cover, but also other tablet options.
> In attached document, Interface type 4 (Graft type) is of ThinkPad X1 Tablet Series.

Great, Nitin thank you for the docs!

Nitin, one question below about version checks, it would be great if you can help with
this.

>> Also is there a way to get the current state of the keyboard-cover being
>> attached at boot or not ?
> It seems "GTOP" ASL method can be used to get current state.

Ack.

Alexander, so it looks like we need to do the following to support this properly:

1. At a new TP_HOTKEY_TABLET_USES_GTOP to the hotkey_tablet enum
2. At probe time in hotkey_init_tablet_mode add a new if / case with I guess
   the highest prio (so try before GMMS) which does:
   1. Call GTOP with parameter 0x0000, check return equals 0x0103 (or newer?)
      Nitin, how should the version check look like here, check that the
      upper byte == 0x01 and the lower byte >= 0x03 ?
   2. Call GTOP with parameter 0x0100, check return value equals 4
   3. Call GTOP with parameter 0x0200, set in_tablet_mode based on bit 0
      and bit 16. I think we should report SW_TABLET_MODE=1 when the thin-kbd
      is attached, but folded to the back
3. Make hotkey_get_tablet_mode support the new TP_HOTKEY_TABLET_USES_GTOP case and
   make it call GTOP with parameter 0x0200 and check bit 0 + bit 16
4. On the events which you identified call tpacpi_input_send_tabletsw()

And can you also check if there are events when folding the kbd behind the tablet?

Regards,

Hans



>>> So this patch is mainly to avoid warnings about unknown and unhandled
>>> events, which are now reported as:
>>>
>>> * Event 0x4012: attached keyboard cover
>>> * Event 0x4013: detached keyboard cover
>>>
>>> Tested as working on a ThinkPad X1 Tablet Gen 2, 20JCS00C00, and as
>>> non-interfering with a ThinkPad X1 Carbon 7th, 20QESABM02 (normal
>>> clamshell, so it does not have a keyboard cover).
>>>
>>> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
>>> ---
>>>  drivers/platform/x86/thinkpad_acpi.c | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c
>>> b/drivers/platform/x86/thinkpad_acpi.c
>>> index c404706379d9..fd5322b5bbbd 100644
>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>> @@ -174,6 +174,8 @@ enum tpacpi_hkey_event_t {
>>>                                                      or port replicator */
>>>         TP_HKEY_EV_HOTPLUG_UNDOCK       = 0x4011, /* undocked from
>> hotplug
>>>                                                      dock or port
>>> replicator */
>>> +       TP_HKEY_EV_KBD_COVER_ATTACH     = 0x4012, /* attached keyboard
>> cover */
>>> +       TP_HKEY_EV_KBD_COVER_DETACH     = 0x4013, /* detached keyboard
>> cover */
>>>
>>>         /* User-interface events */
>>>         TP_HKEY_EV_LID_CLOSE            = 0x5001, /* laptop lid closed */
>>> @@ -3989,6 +3991,12 @@ static bool hotkey_notify_dockevent(const u32
>> hkey,
>>>         case TP_HKEY_EV_HOTPLUG_UNDOCK: /* undocked from port
>> replicator */
>>>                 pr_info("undocked from hotplug port replicator\n");
>>>                 return true;
>>> +       case TP_HKEY_EV_KBD_COVER_ATTACH:
>>> +               pr_info("attached keyboard cover\n");
>>> +               return true;
>>> +       case TP_HKEY_EV_KBD_COVER_DETACH:
>>> +               pr_info("detached keyboard cover\n");
>>> +               return true;
>>>
>>>         default:
>>>                 return false;
>>> --
>>> 2.30.0
>>>
>
Hans de Goede Feb. 8, 2021, 10:17 a.m. UTC | #5
Hi,

On 2/7/21 6:55 PM, Alexander Kobel wrote:
> Hi,
> 
> On 2/7/21 6:10 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 2/7/21 5:34 PM, Alexander Kobel wrote:
>>> Those events occur when a keyboard cover is attached to a ThinkPad
>>> Tablet device.  Typically, they are used to switch from normal to tablet
>>> mode in userspace; e.g., to offer touch keyboard choices when focus goes
>>> to a text box and no keyboard is attached, or to enable autorotation of
>>> the display according to the builtin orientation sensor.
>>
>> Thank you for your patch.
> 
> Thank you for your swift response.

You're welcome.

>>> No attempt is taken to emit an EV_SW event for SW_TABLET_MODE; this is
>>> left to userspace.
>>
>> I don't understand this part, in order for userspace to respond to these
>> events the thinkpad_acpi driver needs to emit events for this; and emitting
>> SW_TABLET_MODE seems like it is the right thing to do.
>>
>> Why are you not doing this ?
> 
> Quite frankly, because I did not know how to, reliably.  (First ever
> patch attempt to the kernel here, so I'd rather err on the safe side.)
> 
> There are a number of events (e.g., TP_HKEY_EV_HOTPLUG_DOCK) that do not
> propagate to userspace, but only report a specific message.  I figured
> it's better to have a meaningful entry in the log rather than just the
> warning about an unknown event.
> But on second thought: pretending to react, but not actually doing
> something, isn't too valuable.
> 
> I'll go off and try to improve.

So Nitin has been kind enough to provide us with some docs for this,
please see me reply to Nitin's email and lets continue this part of this mail
thread there.

<snip>

> Finally, I mentioned some open ends already on a post to ibm-acpi-devel
> at https://sourceforge.net/p/ibm-acpi/mailman/message/37200082/; this
> very question is among them.
> I will start tackling the SW_TABLET_MODE event issue first, but if Mark
> and Nitin can already hint about the keyboard shortcuts, it'd be highly
> appreciated.

I think I might be able to help there, a couple of months ago I bought
a second-hand thinkpad-10 tablet which also has a USB attached keyboard.

In hindsight I guess I could have asked Mark and Nitin for some more info,
but I went on autopilot and just ran hexdump -C on the /dev/hidraw node
to see which events all the keys send.

And I fired up an usb-sniffer under Windows to figure out the audio-leds,
since I'm used to just figure these things out without help from the vendor :)

See the recent commits here for my work on this:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-lenovo.c

So on the ibm-acpi list message you said that the kbd sends the following:

type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001
type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1

For the Fn-keys, does it send the same MSC_SCAN code for *all* the
non-working Fn-keys ?

If so then it seems that this is very much like the thinkpad 10 kbd dock
which also does this, see the lenovo_input_mapping_tp10_ultrabook_kbd()
function in drivers/hid/hid-lenovo.c .

If I have that right, then I think we should be able to get the
Fn keys to work without too much trouble. You could try hacking up
drivers/hid/hid-lenovo.c a bit:

1. Add an entry to the lenovo_devices array like this:

	/*
	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard part,
	 * while letting hid-multitouch.c handle the touchpad and trackpoint.
	 */
        { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
                     USB_VENDOR_ID_LENOVO,
                     USB_DEVICE_ID_LENOVO_X1_TAB),

2. Add the following entry to the switch-case in lenovo_input_mapping() :

        case USB_DEVICE_ID_LENOVO_X1_TAB:
                return lenovo_input_mapping_tp10_ultrabook_kbd(hdev, hi, field,
                                                               usage, bit, max);

And then build hid-lenovo.c and modprobe it.

After the modprobe to:

ls -l /sys/bus/hid/devices/0003:17EF:60A3.*/driver

This should show 2 devices (I guess) with one being bound to hid-lenovo
and 1 being bound to hid-multitouch.

If this works some of your Fn + F# keys will now hopefully start doing
something, you can play around with modifying lenovo_input_mapping_tp10_ultrabook_kbd
to make it do the right thing for your kbd.

###

About LED support, just enabling the LED support bits for the
USB_DEVICE_ID_LENOVO_TP10UBKBD handling for now might work fine,
but there is a tiny chance that sending the wrong command somehow puts
the kbd in firmware update mode, I had that happen once with a Logitech
kbd which did not seem to have any kind of handshake / passcode to avoid
accidental fw updates (*).

If you can give me a dump of the hid-descriptors for your keyboard,
then I can check if that the LEDs might work the same way too (or not).

The easiest way to get a dump is to run the following command as root:

cat /sys/kernel/debug/hid/0003:17EF:60A3.*/rdesc > rdesc

And then attach rdesc to your next email.

Regards,

Hans





*) Luckily it at least had a separate firmware-bootlader mode in which it
was stuck now, so with some cmdline magic to force an upgrade the Windows
fw installer could still fix it.
Nitin Joshi1 Feb. 8, 2021, 2:12 p.m. UTC | #6
Hello Hans,

>-----Original Message-----
>From: Hans de Goede <hdegoede@redhat.com>
>Sent: Monday, February 8, 2021 6:05 PM
>To: Nitin Joshi1 <njoshi1@lenovo.com>; Alexander Kobel <a-kobel@a-
>kobel.de>; platform-driver-x86@vger.kernel.org; Mark Pearson
><mpearson@lenovo.com>
>Subject: Re: [External] Re: [PATCH] platform/x86: thinkpad_acpi: handle HKEY
>0x4012, 0x4013 events
>
>Hi Nitin, Alexander,
>
>On 2/8/21 8:17 AM, Nitin Joshi1 wrote:
>> Hello Hans,
>>
>>> -----Original Message-----
>>> From: Hans de Goede <hdegoede@redhat.com>
>>> Sent: Monday, February 8, 2021 2:10 AM
>>> To: Alexander Kobel <a-kobel@a-kobel.de>; platform-driver-
>>> x86@vger.kernel.org; Mark Pearson <mpearson@lenovo.com>; Nitin
>Joshi1
>>> <njoshi1@lenovo.com>
>>> Subject: [External] Re: [PATCH] platform/x86: thinkpad_acpi: handle
>>> HKEY 0x4012, 0x4013 events
>>>
>>> Hi,
>>>
>>> On 2/7/21 5:34 PM, Alexander Kobel wrote:
>>>> Those events occur when a keyboard cover is attached to a ThinkPad
>>>> Tablet device.  Typically, they are used to switch from normal to
>>>> tablet mode in userspace; e.g., to offer touch keyboard choices when
>>>> focus goes to a text box and no keyboard is attached, or to enable
>>>> autorotation of the display according to the builtin orientation sensor.
>>>
>>> Thank you for your patch.
>>>
>>>> No attempt is taken to emit an EV_SW event for SW_TABLET_MODE; this
>>>> is left to userspace.
>>>
>>> I don't understand this part, in order for userspace to respond to
>>> these events the thinkpad_acpi driver needs to emit events for this;
>>> and emitting SW_TABLET_MODE seems like it is the right thing to do.
>>>
>>> Why are you not doing this ?
>>>
>>> Note that it is important to only advertise SW_TABLET_MODE
>>> functionality on devices where it actually works. Which might be
>challenging I guess...
>>>
>>> But we have contacts inside Lenovo now, so perhaps they can help.
>>>
>>> Mark, Nitin, is there a way for the thinkpad_acpi code to figure out
>>> if 0x4012 /
>>> 0x4013 events will be send by a device?
>>
>> It seems , these events are used for not only keyboard cover, but also other
>tablet options.
>> In attached document, Interface type 4 (Graft type) is of ThinkPad X1 Tablet
>Series.
>
>Great, Nitin thank you for the docs!
>
>Nitin, one question below about version checks, it would be great if you can
>help with this.
>
>>> Also is there a way to get the current state of the keyboard-cover
>>> being attached at boot or not ?
>> It seems "GTOP" ASL method can be used to get current state.
>
>Ack.
>
>Alexander, so it looks like we need to do the following to support this
>properly:
>
>1. At a new TP_HOTKEY_TABLET_USES_GTOP to the hotkey_tablet enum 2. At
>probe time in hotkey_init_tablet_mode add a new if / case with I guess
>   the highest prio (so try before GMMS) which does:
>   1. Call GTOP with parameter 0x0000, check return equals 0x0103 (or newer?)
>      Nitin, how should the version check look like here, check that the
>      upper byte == 0x01 and the lower byte >= 0x03 ?

Thanks for your comments.
We should check version by checking response equal or greater than 0x0103 and not 
By checking upper byte and lower byte.

>   2. Call GTOP with parameter 0x0100, check return value equals 4
>   3. Call GTOP with parameter 0x0200, set in_tablet_mode based on bit 0
>      and bit 16. I think we should report SW_TABLET_MODE=1 when the thin-
>kbd
>      is attached, but folded to the back 3. Make hotkey_get_tablet_mode
>support the new TP_HOTKEY_TABLET_USES_GTOP case and
>   make it call GTOP with parameter 0x0200 and check bit 0 + bit 16 4. On the
>events which you identified call tpacpi_input_send_tabletsw()
>
>And can you also check if there are events when folding the kbd behind the
>tablet?
>
>Regards,
>
>Hans

Thanks & Regards,
Nitin Joshi 
>
>
>
>>>> So this patch is mainly to avoid warnings about unknown and
>>>> unhandled events, which are now reported as:
>>>>
>>>> * Event 0x4012: attached keyboard cover
>>>> * Event 0x4013: detached keyboard cover
>>>>
>>>> Tested as working on a ThinkPad X1 Tablet Gen 2, 20JCS00C00, and as
>>>> non-interfering with a ThinkPad X1 Carbon 7th, 20QESABM02 (normal
>>>> clamshell, so it does not have a keyboard cover).
>>>>
>>>> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
>>>> ---
>>>>  drivers/platform/x86/thinkpad_acpi.c | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c
>>>> b/drivers/platform/x86/thinkpad_acpi.c
>>>> index c404706379d9..fd5322b5bbbd 100644
>>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>>> @@ -174,6 +174,8 @@ enum tpacpi_hkey_event_t {
>>>>                                                      or port replicator */
>>>>         TP_HKEY_EV_HOTPLUG_UNDOCK       = 0x4011, /* undocked from
>>> hotplug
>>>>                                                      dock or port
>>>> replicator */
>>>> +       TP_HKEY_EV_KBD_COVER_ATTACH     = 0x4012, /* attached
>keyboard
>>> cover */
>>>> +       TP_HKEY_EV_KBD_COVER_DETACH     = 0x4013, /* detached
>keyboard
>>> cover */
>>>>
>>>>         /* User-interface events */
>>>>         TP_HKEY_EV_LID_CLOSE            = 0x5001, /* laptop lid closed */
>>>> @@ -3989,6 +3991,12 @@ static bool hotkey_notify_dockevent(const u32
>>> hkey,
>>>>         case TP_HKEY_EV_HOTPLUG_UNDOCK: /* undocked from port
>>> replicator */
>>>>                 pr_info("undocked from hotplug port replicator\n");
>>>>                 return true;
>>>> +       case TP_HKEY_EV_KBD_COVER_ATTACH:
>>>> +               pr_info("attached keyboard cover\n");
>>>> +               return true;
>>>> +       case TP_HKEY_EV_KBD_COVER_DETACH:
>>>> +               pr_info("detached keyboard cover\n");
>>>> +               return true;
>>>>
>>>>         default:
>>>>                 return false;
>>>> --
>>>> 2.30.0
>>>>
>>
Alexander Kobel Feb. 9, 2021, 3:16 p.m. UTC | #7
Hi,

On 2/8/21 11:17 AM, Hans de Goede wrote:
> On 2/7/21 6:55 PM, Alexander Kobel wrote:
>> <snip>
>> I'll go off and try to improve.
> 
> So Nitin has been kind enough to provide us with some docs for this,
> please see me reply to Nitin's email and lets continue this part of this mail
> thread there.

Right. I have the other patch ready, thanks to your great help. I'm
waiting for Nitin's okay whether / how much info I can copy from the
reference sheet to source code comments. Once I have that confirmation,
I will post the revised patch.

> <snip>
> 
>> Finally, I mentioned some open ends already on a post to ibm-acpi-devel
>> at https://sourceforge.net/p/ibm-acpi/mailman/message/37200082/; this
>> very question is among them.
>> I will start tackling the SW_TABLET_MODE event issue first, but if Mark
>> and Nitin can already hint about the keyboard shortcuts, it'd be highly
>> appreciated.
> 
> I think I might be able to help there, a couple of months ago I bought
> a second-hand thinkpad-10 tablet which also has a USB attached keyboard.
> 
> In hindsight I guess I could have asked Mark and Nitin for some more info,
> but I went on autopilot and just ran hexdump -C on the /dev/hidraw node
> to see which events all the keys send.
> 
> And I fired up an usb-sniffer under Windows to figure out the audio-leds,
> since I'm used to just figure these things out without help from the vendor :)

Yeah, why take the boring route if you know how to do all the work on
your own... ;-)

> See the recent commits here for my work on this:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-lenovo.c

Thanks, very helpful.

> So on the ibm-acpi list message you said that the kbd sends the following:
> 
> type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001
> type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1
> 
> For the Fn-keys, does it send the same MSC_SCAN code for *all* the
> non-working Fn-keys ?

Correct. And I can confirm that /dev/hidraw1 lets me distinguish between
the keys.

> If so then it seems that this is very much like the thinkpad 10 kbd dock
> which also does this, see the lenovo_input_mapping_tp10_ultrabook_kbd()
> function in drivers/hid/hid-lenovo.c .
> 
> If I have that right, then I think we should be able to get the
> Fn keys to work without too much trouble. You could try hacking up
> drivers/hid/hid-lenovo.c a bit:

(Not yet there, but will investigate.)

> 1. Add an entry to the lenovo_devices array like this:
> 
> 	/*
> 	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard part,
> 	 * while letting hid-multitouch.c handle the touchpad and trackpoint.
> 	 */
>         { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>                      USB_VENDOR_ID_LENOVO,
>                      USB_DEVICE_ID_LENOVO_X1_TAB),
> 
> 2. Add the following entry to the switch-case in lenovo_input_mapping() :
> 
>         case USB_DEVICE_ID_LENOVO_X1_TAB:
>                 return lenovo_input_mapping_tp10_ultrabook_kbd(hdev, hi, field,
>                                                                usage, bit, max);
> 
> And then build hid-lenovo.c and modprobe it.
> 
> After the modprobe to:
> 
> ls -l /sys/bus/hid/devices/0003:17EF:60A3.*/driver
> 
> This should show 2 devices (I guess) with one being bound to hid-lenovo
> and 1 being bound to hid-multitouch.

So far (without patching hid-lenovo), 2 bound to hid-generic and 1 to
hid-multitouch.

> If this works some of your Fn + F# keys will now hopefully start doing
> something, you can play around with modifying lenovo_input_mapping_tp10_ultrabook_kbd
> to make it do the right thing for your kbd.
> 
> ###
> 
> About LED support, just enabling the LED support bits for the
> USB_DEVICE_ID_LENOVO_TP10UBKBD handling for now might work fine,
> but there is a tiny chance that sending the wrong command somehow puts
> the kbd in firmware update mode, I had that happen once with a Logitech
> kbd which did not seem to have any kind of handshake / passcode to avoid
> accidental fw updates (*).
> 
> If you can give me a dump of the hid-descriptors for your keyboard,
> then I can check if that the LEDs might work the same way too (or not).
> 
> The easiest way to get a dump is to run the following command as root:
> 
> cat /sys/kernel/debug/hid/0003:17EF:60A3.*/rdesc > rdesc
> 
> And then attach rdesc to your next email.

Please find this one attached already now.
In case it helps, the * expands to 0057 0058 0059 on my system.


Thanks,
Alex


> Regards,
> 
> Hans
> 
> 
> 
> 
> 
> *) Luckily it at least had a separate firmware-bootlader mode in which it
> was stuck now, so with some cmdline magic to force an upgrade the Windows
> fw installer could still fix it.
>
05 01 09 06 a1 01 05 07 19 e0 29 e7 15 00 25 01 75 01 95 08 81 02 95 01 75 08 81 01 95 05 75 01 05 08 19 01 29 05 91 02 95 01 75 03 91 01 95 06 75 08 15 00 26 ff 00 05 07 19 00 2a ff 00 81 00 05 0c 09 00 15 80 25 7f 95 40 75 08 b1 02 c0 

  INPUT[INPUT]
    Field(0)
      Application(GenericDesktop.Keyboard)
      Usage(8)
        Keyboard.00e0
        Keyboard.00e1
        Keyboard.00e2
        Keyboard.00e3
        Keyboard.00e4
        Keyboard.00e5
        Keyboard.00e6
        Keyboard.00e7
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(8)
      Report Offset(0)
      Flags( Variable Absolute )
    Field(1)
      Application(GenericDesktop.Keyboard)
      Usage(256)
        Keyboard.0000
        Keyboard.0001
        Keyboard.0002
        Keyboard.0003
        Keyboard.0004
        Keyboard.0005
        Keyboard.0006
        Keyboard.0007
        Keyboard.0008
        Keyboard.0009
        Keyboard.000a
        Keyboard.000b
        Keyboard.000c
        Keyboard.000d
        Keyboard.000e
        Keyboard.000f
        Keyboard.0010
        Keyboard.0011
        Keyboard.0012
        Keyboard.0013
        Keyboard.0014
        Keyboard.0015
        Keyboard.0016
        Keyboard.0017
        Keyboard.0018
        Keyboard.0019
        Keyboard.001a
        Keyboard.001b
        Keyboard.001c
        Keyboard.001d
        Keyboard.001e
        Keyboard.001f
        Keyboard.0020
        Keyboard.0021
        Keyboard.0022
        Keyboard.0023
        Keyboard.0024
        Keyboard.0025
        Keyboard.0026
        Keyboard.0027
        Keyboard.0028
        Keyboard.0029
        Keyboard.002a
        Keyboard.002b
        Keyboard.002c
        Keyboard.002d
        Keyboard.002e
        Keyboard.002f
        Keyboard.0030
        Keyboard.0031
        Keyboard.0032
        Keyboard.0033
        Keyboard.0034
        Keyboard.0035
        Keyboard.0036
        Keyboard.0037
        Keyboard.0038
        Keyboard.0039
        Keyboard.003a
        Keyboard.003b
        Keyboard.003c
        Keyboard.003d
        Keyboard.003e
        Keyboard.003f
        Keyboard.0040
        Keyboard.0041
        Keyboard.0042
        Keyboard.0043
        Keyboard.0044
        Keyboard.0045
        Keyboard.0046
        Keyboard.0047
        Keyboard.0048
        Keyboard.0049
        Keyboard.004a
        Keyboard.004b
        Keyboard.004c
        Keyboard.004d
        Keyboard.004e
        Keyboard.004f
        Keyboard.0050
        Keyboard.0051
        Keyboard.0052
        Keyboard.0053
        Keyboard.0054
        Keyboard.0055
        Keyboard.0056
        Keyboard.0057
        Keyboard.0058
        Keyboard.0059
        Keyboard.005a
        Keyboard.005b
        Keyboard.005c
        Keyboard.005d
        Keyboard.005e
        Keyboard.005f
        Keyboard.0060
        Keyboard.0061
        Keyboard.0062
        Keyboard.0063
        Keyboard.0064
        Keyboard.0065
        Keyboard.0066
        Keyboard.0067
        Keyboard.0068
        Keyboard.0069
        Keyboard.006a
        Keyboard.006b
        Keyboard.006c
        Keyboard.006d
        Keyboard.006e
        Keyboard.006f
        Keyboard.0070
        Keyboard.0071
        Keyboard.0072
        Keyboard.0073
        Keyboard.0074
        Keyboard.0075
        Keyboard.0076
        Keyboard.0077
        Keyboard.0078
        Keyboard.0079
        Keyboard.007a
        Keyboard.007b
        Keyboard.007c
        Keyboard.007d
        Keyboard.007e
        Keyboard.007f
        Keyboard.0080
        Keyboard.0081
        Keyboard.0082
        Keyboard.0083
        Keyboard.0084
        Keyboard.0085
        Keyboard.0086
        Keyboard.0087
        Keyboard.0088
        Keyboard.0089
        Keyboard.008a
        Keyboard.008b
        Keyboard.008c
        Keyboard.008d
        Keyboard.008e
        Keyboard.008f
        Keyboard.0090
        Keyboard.0091
        Keyboard.0092
        Keyboard.0093
        Keyboard.0094
        Keyboard.0095
        Keyboard.0096
        Keyboard.0097
        Keyboard.0098
        Keyboard.0099
        Keyboard.009a
        Keyboard.009b
        Keyboard.009c
        Keyboard.009d
        Keyboard.009e
        Keyboard.009f
        Keyboard.00a0
        Keyboard.00a1
        Keyboard.00a2
        Keyboard.00a3
        Keyboard.00a4
        Keyboard.00a5
        Keyboard.00a6
        Keyboard.00a7
        Keyboard.00a8
        Keyboard.00a9
        Keyboard.00aa
        Keyboard.00ab
        Keyboard.00ac
        Keyboard.00ad
        Keyboard.00ae
        Keyboard.00af
        Keyboard.00b0
        Keyboard.00b1
        Keyboard.00b2
        Keyboard.00b3
        Keyboard.00b4
        Keyboard.00b5
        Keyboard.00b6
        Keyboard.00b7
        Keyboard.00b8
        Keyboard.00b9
        Keyboard.00ba
        Keyboard.00bb
        Keyboard.00bc
        Keyboard.00bd
        Keyboard.00be
        Keyboard.00bf
        Keyboard.00c0
        Keyboard.00c1
        Keyboard.00c2
        Keyboard.00c3
        Keyboard.00c4
        Keyboard.00c5
        Keyboard.00c6
        Keyboard.00c7
        Keyboard.00c8
        Keyboard.00c9
        Keyboard.00ca
        Keyboard.00cb
        Keyboard.00cc
        Keyboard.00cd
        Keyboard.00ce
        Keyboard.00cf
        Keyboard.00d0
        Keyboard.00d1
        Keyboard.00d2
        Keyboard.00d3
        Keyboard.00d4
        Keyboard.00d5
        Keyboard.00d6
        Keyboard.00d7
        Keyboard.00d8
        Keyboard.00d9
        Keyboard.00da
        Keyboard.00db
        Keyboard.00dc
        Keyboard.00dd
        Keyboard.00de
        Keyboard.00df
        Keyboard.00e0
        Keyboard.00e1
        Keyboard.00e2
        Keyboard.00e3
        Keyboard.00e4
        Keyboard.00e5
        Keyboard.00e6
        Keyboard.00e7
        Keyboard.00e8
        Keyboard.00e9
        Keyboard.00ea
        Keyboard.00eb
        Keyboard.00ec
        Keyboard.00ed
        Keyboard.00ee
        Keyboard.00ef
        Keyboard.00f0
        Keyboard.00f1
        Keyboard.00f2
        Keyboard.00f3
        Keyboard.00f4
        Keyboard.00f5
        Keyboard.00f6
        Keyboard.00f7
        Keyboard.00f8
        Keyboard.00f9
        Keyboard.00fa
        Keyboard.00fb
        Keyboard.00fc
        Keyboard.00fd
        Keyboard.00fe
        Keyboard.00ff
      Logical Minimum(0)
      Logical Maximum(255)
      Report Size(8)
      Report Count(6)
      Report Offset(16)
      Flags( Array Absolute )
  OUTPUT[OUTPUT]
    Field(0)
      Application(GenericDesktop.Keyboard)
      Usage(5)
        LED.NumLock
        LED.CapsLock
        LED.ScrollLock
        LED.Compose
        LED.Kana
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(5)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE[FEATURE]
    Field(0)
      Application(GenericDesktop.Keyboard)
      Usage(64)
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
        Consumer.0000
      Logical Minimum(-128)
      Logical Maximum(127)
      Report Size(8)
      Report Count(64)
      Report Offset(0)
      Flags( Variable Absolute )

Keyboard.00e0 ---> Key.LeftControl
Keyboard.00e1 ---> Key.LeftShift
Keyboard.00e2 ---> Key.LeftAlt
Keyboard.00e3 ---> Key.LeftMeta
Keyboard.00e4 ---> Key.RightCtrl
Keyboard.00e5 ---> Key.RightShift
Keyboard.00e6 ---> Key.RightAlt
Keyboard.00e7 ---> Key.RightMeta
Keyboard.0000 ---> Sync.Report
Keyboard.0001 ---> Sync.Report
Keyboard.0002 ---> Sync.Report
Keyboard.0003 ---> Sync.Report
Keyboard.0004 ---> Key.A
Keyboard.0005 ---> Key.B
Keyboard.0006 ---> Key.C
Keyboard.0007 ---> Key.D
Keyboard.0008 ---> Key.E
Keyboard.0009 ---> Key.F
Keyboard.000a ---> Key.G
Keyboard.000b ---> Key.H
Keyboard.000c ---> Key.I
Keyboard.000d ---> Key.J
Keyboard.000e ---> Key.K
Keyboard.000f ---> Key.L
Keyboard.0010 ---> Key.M
Keyboard.0011 ---> Key.N
Keyboard.0012 ---> Key.O
Keyboard.0013 ---> Key.P
Keyboard.0014 ---> Key.Q
Keyboard.0015 ---> Key.R
Keyboard.0016 ---> Key.S
Keyboard.0017 ---> Key.T
Keyboard.0018 ---> Key.U
Keyboard.0019 ---> Key.V
Keyboard.001a ---> Key.W
Keyboard.001b ---> Key.X
Keyboard.001c ---> Key.Y
Keyboard.001d ---> Key.Z
Keyboard.001e ---> Key.1
Keyboard.001f ---> Key.2
Keyboard.0020 ---> Key.3
Keyboard.0021 ---> Key.4
Keyboard.0022 ---> Key.5
Keyboard.0023 ---> Key.6
Keyboard.0024 ---> Key.7
Keyboard.0025 ---> Key.8
Keyboard.0026 ---> Key.9
Keyboard.0027 ---> Key.0
Keyboard.0028 ---> Key.Enter
Keyboard.0029 ---> Key.Esc
Keyboard.002a ---> Key.Backspace
Keyboard.002b ---> Key.Tab
Keyboard.002c ---> Key.Space
Keyboard.002d ---> Key.Minus
Keyboard.002e ---> Key.Equal
Keyboard.002f ---> Key.LeftBrace
Keyboard.0030 ---> Key.RightBrace
Keyboard.0031 ---> Key.BackSlash
Keyboard.0032 ---> Key.BackSlash
Keyboard.0033 ---> Key.Semicolon
Keyboard.0034 ---> Key.Apostrophe
Keyboard.0035 ---> Key.Grave
Keyboard.0036 ---> Key.Comma
Keyboard.0037 ---> Key.Dot
Keyboard.0038 ---> Key.Slash
Keyboard.0039 ---> Key.CapsLock
Keyboard.003a ---> Key.F1
Keyboard.003b ---> Key.F2
Keyboard.003c ---> Key.F3
Keyboard.003d ---> Key.F4
Keyboard.003e ---> Key.F5
Keyboard.003f ---> Key.F6
Keyboard.0040 ---> Key.F7
Keyboard.0041 ---> Key.F8
Keyboard.0042 ---> Key.F9
Keyboard.0043 ---> Key.F10
Keyboard.0044 ---> Key.F11
Keyboard.0045 ---> Key.F12
Keyboard.0046 ---> Key.SysRq
Keyboard.0047 ---> Key.ScrollLock
Keyboard.0048 ---> Key.Pause
Keyboard.0049 ---> Key.Insert
Keyboard.004a ---> Key.Home
Keyboard.004b ---> Key.PageUp
Keyboard.004c ---> Key.Delete
Keyboard.004d ---> Key.End
Keyboard.004e ---> Key.PageDown
Keyboard.004f ---> Key.Right
Keyboard.0050 ---> Key.Left
Keyboard.0051 ---> Key.Down
Keyboard.0052 ---> Key.Up
Keyboard.0053 ---> Key.NumLock
Keyboard.0054 ---> Key.KPSlash
Keyboard.0055 ---> Key.KPAsterisk
Keyboard.0056 ---> Key.KPMinus
Keyboard.0057 ---> Key.KPPlus
Keyboard.0058 ---> Key.KPEnter
Keyboard.0059 ---> Key.KP1
Keyboard.005a ---> Key.KP2
Keyboard.005b ---> Key.KP3
Keyboard.005c ---> Key.KP4
Keyboard.005d ---> Key.KP5
Keyboard.005e ---> Key.KP6
Keyboard.005f ---> Key.KP7
Keyboard.0060 ---> Key.KP8
Keyboard.0061 ---> Key.KP9
Keyboard.0062 ---> Key.KP0
Keyboard.0063 ---> Key.KPDot
Keyboard.0064 ---> Key.102nd
Keyboard.0065 ---> Key.Compose
Keyboard.0066 ---> Key.Power
Keyboard.0067 ---> Key.KPEqual
Keyboard.0068 ---> Key.F13
Keyboard.0069 ---> Key.F14
Keyboard.006a ---> Key.F15
Keyboard.006b ---> Key.F16
Keyboard.006c ---> Key.F17
Keyboard.006d ---> Key.F18
Keyboard.006e ---> Key.F19
Keyboard.006f ---> Key.F20
Keyboard.0070 ---> Key.F21
Keyboard.0071 ---> Key.F22
Keyboard.0072 ---> Key.F23
Keyboard.0073 ---> Key.F24
Keyboard.0074 ---> Key.Open
Keyboard.0075 ---> Key.Help
Keyboard.0076 ---> Key.Props
Keyboard.0077 ---> Key.Front
Keyboard.0078 ---> Key.Stop
Keyboard.0079 ---> Key.Again
Keyboard.007a ---> Key.Undo
Keyboard.007b ---> Key.Cut
Keyboard.007c ---> Key.Copy
Keyboard.007d ---> Key.Paste
Keyboard.007e ---> Key.Find
Keyboard.007f ---> Key.Mute
Keyboard.0080 ---> Key.VolumeUp
Keyboard.0081 ---> Key.VolumeDown
Keyboard.0082 ---> Key.Unknown
Keyboard.0083 ---> Key.Unknown
Keyboard.0084 ---> Key.Unknown
Keyboard.0085 ---> Key.KPComma
Keyboard.0086 ---> Key.Unknown
Keyboard.0087 ---> Key.RO
Keyboard.0088 ---> Key.Katakana/Hiragana
Keyboard.0089 ---> Key.Yen
Keyboard.008a ---> Key.Henkan
Keyboard.008b ---> Key.Muhenkan
Keyboard.008c ---> Key.KPJpComma
Keyboard.008d ---> Key.Unknown
Keyboard.008e ---> Key.Unknown
Keyboard.008f ---> Key.Unknown
Keyboard.0090 ---> Key.Hangeul
Keyboard.0091 ---> Key.Hanja
Keyboard.0092 ---> Key.Katakana
Keyboard.0093 ---> Key.HIRAGANA
Keyboard.0094 ---> Key.Zenkaku/Hankaku
Keyboard.0095 ---> Key.Unknown
Keyboard.0096 ---> Key.Unknown
Keyboard.0097 ---> Key.Unknown
Keyboard.0098 ---> Key.Unknown
Keyboard.0099 ---> Key.Unknown
Keyboard.009a ---> Key.Unknown
Keyboard.009b ---> Key.Unknown
Keyboard.009c ---> Key.Delete
Keyboard.009d ---> Key.Unknown
Keyboard.009e ---> Key.Unknown
Keyboard.009f ---> Key.Unknown
Keyboard.00a0 ---> Key.Unknown
Keyboard.00a1 ---> Key.Unknown
Keyboard.00a2 ---> Key.Unknown
Keyboard.00a3 ---> Key.Unknown
Keyboard.00a4 ---> Key.Unknown
Keyboard.00a5 ---> Key.Unknown
Keyboard.00a6 ---> Key.Unknown
Keyboard.00a7 ---> Key.Unknown
Keyboard.00a8 ---> Key.Unknown
Keyboard.00a9 ---> Key.Unknown
Keyboard.00aa ---> Key.Unknown
Keyboard.00ab ---> Key.Unknown
Keyboard.00ac ---> Key.Unknown
Keyboard.00ad ---> Key.Unknown
Keyboard.00ae ---> Key.Unknown
Keyboard.00af ---> Key.Unknown
Keyboard.00b0 ---> Key.Unknown
Keyboard.00b1 ---> Key.Unknown
Keyboard.00b2 ---> Key.Unknown
Keyboard.00b3 ---> Key.Unknown
Keyboard.00b4 ---> Key.Unknown
Keyboard.00b5 ---> Key.Unknown
Keyboard.00b6 ---> Key.KPLeftParenthesis
Keyboard.00b7 ---> Key.KPRightParenthesis
Keyboard.00b8 ---> Key.Unknown
Keyboard.00b9 ---> Key.Unknown
Keyboard.00ba ---> Key.Unknown
Keyboard.00bb ---> Key.Unknown
Keyboard.00bc ---> Key.Unknown
Keyboard.00bd ---> Key.Unknown
Keyboard.00be ---> Key.Unknown
Keyboard.00bf ---> Key.Unknown
Keyboard.00c0 ---> Key.Unknown
Keyboard.00c1 ---> Key.Unknown
Keyboard.00c2 ---> Key.Unknown
Keyboard.00c3 ---> Key.Unknown
Keyboard.00c4 ---> Key.Unknown
Keyboard.00c5 ---> Key.Unknown
Keyboard.00c6 ---> Key.Unknown
Keyboard.00c7 ---> Key.Unknown
Keyboard.00c8 ---> Key.Unknown
Keyboard.00c9 ---> Key.Unknown
Keyboard.00ca ---> Key.Unknown
Keyboard.00cb ---> Key.Unknown
Keyboard.00cc ---> Key.Unknown
Keyboard.00cd ---> Key.Unknown
Keyboard.00ce ---> Key.Unknown
Keyboard.00cf ---> Key.Unknown
Keyboard.00d0 ---> Key.Unknown
Keyboard.00d1 ---> Key.Unknown
Keyboard.00d2 ---> Key.Unknown
Keyboard.00d3 ---> Key.Unknown
Keyboard.00d4 ---> Key.Unknown
Keyboard.00d5 ---> Key.Unknown
Keyboard.00d6 ---> Key.Unknown
Keyboard.00d7 ---> Key.Unknown
Keyboard.00d8 ---> Key.Delete
Keyboard.00d9 ---> Key.Unknown
Keyboard.00da ---> Key.Unknown
Keyboard.00db ---> Key.Unknown
Keyboard.00dc ---> Key.Unknown
Keyboard.00dd ---> Key.Unknown
Keyboard.00de ---> Key.Unknown
Keyboard.00df ---> Key.Unknown
Keyboard.00e0 ---> Key.LeftControl
Keyboard.00e1 ---> Key.LeftShift
Keyboard.00e2 ---> Key.LeftAlt
Keyboard.00e3 ---> Key.LeftMeta
Keyboard.00e4 ---> Key.RightCtrl
Keyboard.00e5 ---> Key.RightShift
Keyboard.00e6 ---> Key.RightAlt
Keyboard.00e7 ---> Key.RightMeta
Keyboard.00e8 ---> Key.PlayPause
Keyboard.00e9 ---> Key.StopCD
Keyboard.00ea ---> Key.PreviousSong
Keyboard.00eb ---> Key.NextSong
Keyboard.00ec ---> Key.EjectCD
Keyboard.00ed ---> Key.VolumeUp
Keyboard.00ee ---> Key.VolumeDown
Keyboard.00ef ---> Key.Mute
Keyboard.00f0 ---> Key.WWW
Keyboard.00f1 ---> Key.Back
Keyboard.00f2 ---> Key.Forward
Keyboard.00f3 ---> Key.Stop
Keyboard.00f4 ---> Key.Find
Keyboard.00f5 ---> Key.ScrollUp
Keyboard.00f6 ---> Key.ScrollDown
Keyboard.00f7 ---> Key.Edit
Keyboard.00f8 ---> Key.Sleep
Keyboard.00f9 ---> Key.Coffee
Keyboard.00fa ---> Key.Refresh
Keyboard.00fb ---> Key.Calc
Keyboard.00fc ---> Key.Unknown
Keyboard.00fd ---> Key.Unknown
Keyboard.00fe ---> Key.Unknown
Keyboard.00ff ---> Key.Unknown
LED.NumLock ---> LED.NumLock
LED.CapsLock ---> LED.CapsLock
LED.ScrollLock ---> LED.ScrollLock
LED.Compose ---> LED.Compose
LED.Kana ---> LED.Kana
05 01 09 80 a1 01 85 02 05 01 19 81 29 83 15 00 25 01 95 03 75 01 81 02 95 01 75 05 81 01 c0 05 0c 09 01 a1 01 85 03 15 00 25 01 09 01 09 01 09 01 09 01 09 01 09 e2 09 01 09 01 09 01 09 01 09 01 09 b7 09 01 09 01 09 01 09 01 09 01 09 01 09 01 09 6f 09 ea 09 e9 09 70 09 01 75 01 95 18 81 02 c0 06 a0 ff 09 01 a1 01 85 09 15 00 25 ff 09 04 75 08 95 02 91 02 06 a1 ff 09 01 85 54 15 00 25 7f 0a 11 00 75 08 95 01 b1 02 06 a2 ff 09 01 85 64 15 00 25 01 0a 12 00 75 08 95 01 b1 02 06 a3 ff 09 01 85 74 15 00 25 01 0a 13 00 75 08 95 01 b1 02 06 a4 ff 09 01 85 90 09 08 b1 02 85 84 09 09 b1 02 85 20 09 0a b1 02 06 a5 ff 09 01 85 a2 09 0b 95 06 b1 02 85 a3 09 0c 95 03 b1 02 c0 09 22 a1 00 85 04 09 57 09 58 75 01 95 02 25 01 b1 02 95 06 b1 03 c0 

  INPUT(2)[INPUT]
    Field(0)
      Application(GenericDesktop.SystemControl)
      Usage(3)
        GenericDesktop.SystemPowerDown
        GenericDesktop.SystemSleep
        GenericDesktop.SystemWakeUp
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(3)
      Report Offset(0)
      Flags( Variable Absolute )
  INPUT(3)[INPUT]
    Field(0)
      Application(Consumer.0001)
      Usage(24)
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.00e2
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.00b7
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.0001
        Consumer.006f
        Consumer.00ea
        Consumer.00e9
        Consumer.0070
        Consumer.0001
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(24)
      Report Offset(0)
      Flags( Variable Absolute )
  OUTPUT(9)[OUTPUT]
    Field(0)
      Application(ffa0.0001)
      Usage(2)
        ffa0.0004
        ffa0.0004
      Logical Minimum(0)
      Logical Maximum(255)
      Report Size(8)
      Report Count(2)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(84)[FEATURE]
    Field(0)
      Application(ffa0.0001)
      Usage(2)
        ffa1.0001
        ffa1.0011
      Logical Minimum(0)
      Logical Maximum(127)
      Report Size(8)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(100)[FEATURE]
    Field(0)
      Application(ffa0.0001)
      Usage(2)
        ffa2.0001
        ffa2.0012
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(8)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(116)[FEATURE]
    Field(0)
      Application(ffa0.0001)
      Usage(2)
        ffa3.0001
        ffa3.0013
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(8)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(144)[FEATURE]
    Field(0)
      Application(ffa0.0001)
      Usage(2)
        ffa4.0001
        ffa4.0008
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(8)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(132)[FEATURE]
    Field(0)
      Application(ffa0.0001)
      Usage(1)
        ffa4.0009
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(8)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(32)[FEATURE]
    Field(0)
      Application(ffa0.0001)
      Usage(1)
        ffa4.000a
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(8)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(162)[FEATURE]
    Field(0)
      Application(ffa0.0001)
      Usage(6)
        ffa5.0001
        ffa5.000b
        ffa5.000b
        ffa5.000b
        ffa5.000b
        ffa5.000b
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(8)
      Report Count(6)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(163)[FEATURE]
    Field(0)
      Application(ffa0.0001)
      Usage(3)
        ffa5.000c
        ffa5.000c
        ffa5.000c
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(8)
      Report Count(3)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(4)[FEATURE]
    Field(0)
      Physical(ffa5.0022)
      Usage(2)
        ffa5.0057
        ffa5.0058
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(2)
      Report Offset(0)
      Flags( Variable Absolute )

GenericDesktop.SystemPowerDown ---> Key.Power
GenericDesktop.SystemSleep ---> Key.Sleep
GenericDesktop.SystemWakeUp ---> Key.WakeUp
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.00e2 ---> Key.Mute
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.00b7 ---> Key.StopCD
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.0001 ---> Key.Unknown
Consumer.006f ---> Key.BrightnessUp
Consumer.00ea ---> Key.VolumeDown
Consumer.00e9 ---> Key.VolumeUp
Consumer.0070 ---> Key.BrightnessDown
Consumer.0001 ---> Key.Unknown
ffa0.0004 ---> Sync.Report
ffa0.0004 ---> Sync.Report
05 01 09 02 a1 01 85 02 09 01 a1 00 05 09 19 01 29 02 15 00 25 01 75 01 95 02 81 02 95 06 81 01 05 01 09 30 09 31 15 81 25 7f 75 08 95 02 81 06 c0 c0 05 01 09 02 a1 01 85 10 09 01 a1 00 05 09 19 01 29 03 15 00 25 01 75 01 95 03 81 02 95 05 81 01 05 01 09 30 09 31 15 81 25 7f 75 08 95 02 81 06 c0 c0 05 0d 09 05 a1 01 85 03 05 0d 09 22 a1 02 15 00 25 01 09 47 09 42 95 02 75 01 81 02 95 01 75 03 25 05 09 51 81 02 75 01 95 03 81 03 05 01 15 00 26 16 04 75 10 55 0e 65 11 09 30 35 00 46 67 03 95 01 81 02 46 d0 01 26 2d 02 09 31 81 02 c0 05 0d 09 22 a1 02 15 00 25 01 09 47 09 42 95 02 75 01 81 02 95 01 75 03 25 05 09 51 81 02 75 01 95 03 81 03 05 01 15 00 26 16 04 75 10 55 0e 65 11 09 30 35 00 46 67 03 95 01 81 02 46 d0 01 26 2d 02 09 31 81 02 c0 05 0d 09 22 a1 02 15 00 25 01 09 47 09 42 95 02 75 01 81 02 95 01 75 03 25 05 09 51 81 02 75 01 95 03 81 03 05 01 15 00 26 16 04 75 10 55 0e 65 11 09 30 35 00 46 67 03 95 01 81 02 46 d0 01 26 2d 02 09 31 81 02 c0 05 0d 09 22 a1 02 15 00 25 01 09 47 09 42 95 02 75 01 81 02 95 01 75 03 25 05 09 51 81 02 75 01 95 03 81 03 05 01 15 00 26 16 04 75 10 55 0e 65 11 09 30 35 00 46 67 03 95 01 81 02 46 d0 01 26 2d 02 09 31 81 02 c0 05 0d 09 22 a1 02 15 00 25 01 09 47 09 42 95 02 75 01 81 02 95 01 75 03 25 05 09 51 81 02 75 01 95 03 81 03 05 01 15 00 26 16 04 75 10 55 0e 65 11 09 30 35 00 46 67 03 95 01 81 02 46 d0 01 26 2d 02 09 31 81 02 c0 05 0d 55 0c 66 01 10 47 ff ff 00 00 27 ff ff 00 00 75 10 95 01 09 56 81 02 09 54 25 7f 95 01 75 08 81 02 05 09 09 01 25 01 75 01 95 01 81 02 95 07 81 03 05 0d 85 08 09 55 09 59 75 04 95 02 25 0f b1 02 85 0d 09 60 75 01 95 01 15 00 25 01 b1 02 95 07 b1 03 85 07 06 00 ff 09 c5 15 00 26 ff 00 75 08 96 00 01 b1 02 c0 05 0d 09 0e a1 01 85 04 09 22 a1 02 09 52 15 00 25 0a 75 08 95 01 b1 02 c0 09 22 a1 00 85 06 09 57 09 58 75 01 95 02 25 01 b1 02 95 06 b1 03 c0 c0 06 00 ff 09 01 a1 01 85 09 09 02 15 00 26 ff 00 75 08 95 14 91 02 85 0a 09 03 15 00 26 ff 00 75 08 95 14 91 02 85 0b 09 04 15 00 26 ff 00 75 08 95 3d 81 02 85 0c 09 05 15 00 26 ff 00 75 08 95 3d 81 02 85 0f 09 06 15 00 26 ff 00 75 08 95 03 b1 02 85 0e 09 07 15 00 26 ff 00 75 08 95 01 b1 02 c0 

  INPUT(2)[INPUT]
    Field(0)
      Physical(GenericDesktop.Pointer)
      Application(GenericDesktop.Mouse)
      Usage(2)
        Button.0001
        Button.0002
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(2)
      Report Offset(0)
      Flags( Variable Absolute )
    Field(1)
      Physical(GenericDesktop.Pointer)
      Application(GenericDesktop.Mouse)
      Usage(2)
        GenericDesktop.X
        GenericDesktop.Y
      Logical Minimum(-127)
      Logical Maximum(127)
      Report Size(8)
      Report Count(2)
      Report Offset(8)
      Flags( Variable Relative )
  INPUT(16)[INPUT]
    Field(0)
      Physical(GenericDesktop.Pointer)
      Application(GenericDesktop.Mouse)
      Usage(3)
        Button.0001
        Button.0002
        Button.0003
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(3)
      Report Offset(0)
      Flags( Variable Absolute )
    Field(1)
      Physical(GenericDesktop.Pointer)
      Application(GenericDesktop.Mouse)
      Usage(2)
        GenericDesktop.X
        GenericDesktop.Y
      Logical Minimum(-127)
      Logical Maximum(127)
      Report Size(8)
      Report Count(2)
      Report Offset(8)
      Flags( Variable Relative )
  INPUT(3)[INPUT]
    Field(0)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(2)
        Digitizers.Confidence
        Digitizers.TipSwitch
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(2)
      Report Offset(0)
      Flags( Variable Absolute )
    Field(1)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        Digitizers.ContactID
      Logical Minimum(0)
      Logical Maximum(5)
      Report Size(3)
      Report Count(1)
      Report Offset(2)
      Flags( Variable Absolute )
    Field(2)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.X
      Logical Minimum(0)
      Logical Maximum(1046)
      Physical Minimum(0)
      Physical Maximum(871)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(8)
      Flags( Variable Absolute )
    Field(3)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.Y
      Logical Minimum(0)
      Logical Maximum(557)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(24)
      Flags( Variable Absolute )
    Field(4)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(2)
        Digitizers.Confidence
        Digitizers.TipSwitch
      Logical Minimum(0)
      Logical Maximum(1)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(1)
      Report Count(2)
      Report Offset(40)
      Flags( Variable Absolute )
    Field(5)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        Digitizers.ContactID
      Logical Minimum(0)
      Logical Maximum(5)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(3)
      Report Count(1)
      Report Offset(42)
      Flags( Variable Absolute )
    Field(6)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.X
      Logical Minimum(0)
      Logical Maximum(1046)
      Physical Minimum(0)
      Physical Maximum(871)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(48)
      Flags( Variable Absolute )
    Field(7)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.Y
      Logical Minimum(0)
      Logical Maximum(557)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(64)
      Flags( Variable Absolute )
    Field(8)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(2)
        Digitizers.Confidence
        Digitizers.TipSwitch
      Logical Minimum(0)
      Logical Maximum(1)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(1)
      Report Count(2)
      Report Offset(80)
      Flags( Variable Absolute )
    Field(9)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        Digitizers.ContactID
      Logical Minimum(0)
      Logical Maximum(5)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(3)
      Report Count(1)
      Report Offset(82)
      Flags( Variable Absolute )
    Field(10)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.X
      Logical Minimum(0)
      Logical Maximum(1046)
      Physical Minimum(0)
      Physical Maximum(871)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(88)
      Flags( Variable Absolute )
    Field(11)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.Y
      Logical Minimum(0)
      Logical Maximum(557)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(104)
      Flags( Variable Absolute )
    Field(12)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(2)
        Digitizers.Confidence
        Digitizers.TipSwitch
      Logical Minimum(0)
      Logical Maximum(1)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(1)
      Report Count(2)
      Report Offset(120)
      Flags( Variable Absolute )
    Field(13)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        Digitizers.ContactID
      Logical Minimum(0)
      Logical Maximum(5)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(3)
      Report Count(1)
      Report Offset(122)
      Flags( Variable Absolute )
    Field(14)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.X
      Logical Minimum(0)
      Logical Maximum(1046)
      Physical Minimum(0)
      Physical Maximum(871)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(128)
      Flags( Variable Absolute )
    Field(15)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.Y
      Logical Minimum(0)
      Logical Maximum(557)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(144)
      Flags( Variable Absolute )
    Field(16)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(2)
        Digitizers.Confidence
        Digitizers.TipSwitch
      Logical Minimum(0)
      Logical Maximum(1)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(1)
      Report Count(2)
      Report Offset(160)
      Flags( Variable Absolute )
    Field(17)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        Digitizers.ContactID
      Logical Minimum(0)
      Logical Maximum(5)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(3)
      Report Count(1)
      Report Offset(162)
      Flags( Variable Absolute )
    Field(18)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.X
      Logical Minimum(0)
      Logical Maximum(1046)
      Physical Minimum(0)
      Physical Maximum(871)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(168)
      Flags( Variable Absolute )
    Field(19)
      Logical(Digitizers.Finger)
      Application(Digitizers.TouchPad)
      Usage(1)
        GenericDesktop.Y
      Logical Minimum(0)
      Logical Maximum(557)
      Physical Minimum(0)
      Physical Maximum(464)
      Unit Exponent(-2)
      Unit(SI Linear : Centimeter)
      Report Size(16)
      Report Count(1)
      Report Offset(184)
      Flags( Variable Absolute )
    Field(20)
      Application(Digitizers.TouchPad)
      Usage(1)
        Digitizers.0056
      Logical Minimum(0)
      Logical Maximum(65535)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(16)
      Report Count(1)
      Report Offset(200)
      Flags( Variable Absolute )
    Field(21)
      Application(Digitizers.TouchPad)
      Usage(1)
        Digitizers.ContactCount
      Logical Minimum(0)
      Logical Maximum(127)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(1)
      Report Offset(216)
      Flags( Variable Absolute )
    Field(22)
      Application(Digitizers.TouchPad)
      Usage(1)
        Button.0001
      Logical Minimum(0)
      Logical Maximum(1)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(1)
      Report Count(1)
      Report Offset(224)
      Flags( Variable Absolute )
  INPUT(11)[INPUT]
    Field(0)
      Application(ff00.0001)
      Usage(61)
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
        ff00.0004
      Logical Minimum(0)
      Logical Maximum(255)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(61)
      Report Offset(0)
      Flags( Variable Absolute )
  INPUT(12)[INPUT]
    Field(0)
      Application(ff00.0001)
      Usage(61)
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
        ff00.0005
      Logical Minimum(0)
      Logical Maximum(255)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(61)
      Report Offset(0)
      Flags( Variable Absolute )
  OUTPUT(9)[OUTPUT]
    Field(0)
      Application(ff00.0001)
      Usage(20)
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
        ff00.0002
      Logical Minimum(0)
      Logical Maximum(255)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(20)
      Report Offset(0)
      Flags( Variable Absolute )
  OUTPUT(10)[OUTPUT]
    Field(0)
      Application(ff00.0001)
      Usage(20)
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
        ff00.0003
      Logical Minimum(0)
      Logical Maximum(255)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(20)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(8)[FEATURE]
    Field(0)
      Application(Digitizers.TouchPad)
      Usage(2)
        Digitizers.ContactMaximumNumber
        Digitizers.ButtonType
      Logical Minimum(0)
      Logical Maximum(15)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(4)
      Report Count(2)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(13)[FEATURE]
    Field(0)
      Application(Digitizers.TouchPad)
      Usage(1)
        Digitizers.0060
      Logical Minimum(0)
      Logical Maximum(1)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(1)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(7)[FEATURE]
    Field(0)
      Application(Digitizers.TouchPad)
      Usage(256)
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
        ff00.00c5
      Logical Minimum(0)
      Logical Maximum(255)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(256)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(4)[FEATURE]
    Field(0)
      Logical(Digitizers.Finger)
      Application(Digitizers.DeviceConfiguration)
      Usage(1)
        Digitizers.InputMode
      Logical Minimum(0)
      Logical Maximum(10)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(6)[FEATURE]
    Field(0)
      Physical(Digitizers.Finger)
      Application(Digitizers.DeviceConfiguration)
      Usage(2)
        Digitizers.0057
        Digitizers.0058
      Logical Minimum(0)
      Logical Maximum(1)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(1)
      Report Count(2)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(15)[FEATURE]
    Field(0)
      Application(ff00.0001)
      Usage(3)
        ff00.0006
        ff00.0006
        ff00.0006
      Logical Minimum(0)
      Logical Maximum(255)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(3)
      Report Offset(0)
      Flags( Variable Absolute )
  FEATURE(14)[FEATURE]
    Field(0)
      Application(ff00.0001)
      Usage(1)
        ff00.0007
      Logical Minimum(0)
      Logical Maximum(255)
      Physical Minimum(0)
      Physical Maximum(65535)
      Unit Exponent(-4)
      Unit(SI Linear : Seconds)
      Report Size(8)
      Report Count(1)
      Report Offset(0)
      Flags( Variable Absolute )

Button.0001 ---> Key.LeftBtn
Button.0002 ---> Key.RightBtn
GenericDesktop.X ---> Relative.X
GenericDesktop.Y ---> Relative.Y
Button.0001 ---> Key.LeftBtn
Button.0002 ---> Key.RightBtn
Button.0003 ---> Key.MiddleBtn
GenericDesktop.X ---> Relative.X
GenericDesktop.Y ---> Relative.Y
Digitizers.Confidence ---> Sync.Report
Digitizers.TipSwitch ---> Sync.Report
Digitizers.ContactID ---> Sync.Report
GenericDesktop.X ---> Sync.Report
GenericDesktop.Y ---> Sync.Report
Digitizers.Confidence ---> Sync.Report
Digitizers.TipSwitch ---> Sync.Report
Digitizers.ContactID ---> Sync.Report
GenericDesktop.X ---> Sync.Report
GenericDesktop.Y ---> Sync.Report
Digitizers.Confidence ---> Sync.Report
Digitizers.TipSwitch ---> Sync.Report
Digitizers.ContactID ---> Sync.Report
GenericDesktop.X ---> Sync.Report
GenericDesktop.Y ---> Sync.Report
Digitizers.Confidence ---> Sync.Report
Digitizers.TipSwitch ---> Sync.Report
Digitizers.ContactID ---> Sync.Report
GenericDesktop.X ---> Sync.Report
GenericDesktop.Y ---> Sync.Report
Digitizers.Confidence ---> Sync.Report
Digitizers.TipSwitch ---> Sync.Report
Digitizers.ContactID ---> Sync.Report
GenericDesktop.X ---> Sync.Report
GenericDesktop.Y ---> Sync.Report
Digitizers.0056 ---> Sync.Report
Digitizers.ContactCount ---> Sync.Report
Button.0001 ---> Key.LeftBtn
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0004 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0005 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
ff00.0003 ---> Sync.Report
Alexander Kobel Feb. 10, 2021, 5:51 p.m. UTC | #8
Hi Hans, Nitin,

thanks a lot for your help.

On 2/8/21 10:04 AM, Hans de Goede wrote:
> <snip>
> Alexander, so it looks like we need to do the following to support this properly:

Spot on. Thanks for the detailed guide; I wouldn't have been able to get there in due time without your advise.
I will send the revised patch in a separate mail, but want to highlight few points.

> 1. At a new TP_HOTKEY_TABLET_USES_GTOP to the hotkey_tablet enum
> 2. At probe time in hotkey_init_tablet_mode add a new if / case with I guess
>    the highest prio (so try before GMMS) which does:
>    1. Call GTOP with parameter 0x0000, check return equals 0x0103 (or newer?)
>       Nitin, how should the version check look like here, check that the
>       upper byte == 0x01 and the lower byte >= 0x03 ?

Exactly.

>    2. Call GTOP with parameter 0x0100, check return value equals 4

I also included the simpler "any type" interface which does not report folding to the back. I confirmed that it works on my X1 with limited functionality, but don't have a "any type" device to test available. (In fact, I don't know whether such a device even exists.) IIUC it can also easily cover type 2 "Helix" and type 3 "Thinkpad 10" with the functionality available on those types. Since I cannot test, I didn't enable that in my patch; but it should be a matter of checking for return values 2 and 3 and adding an appropriate case in hotkey_get_tablet_mode.
By the way, Nitin kindly confirmed that I can translate the GTOP reference sheet to source code comments, and provided the mapping between type 2, 3, 4 and series "Helix", "Thinkpad 10" and "X1 Tablet".

>    3. Call GTOP with parameter 0x0200, set in_tablet_mode based on bit 0
>       and bit 16. I think we should report SW_TABLET_MODE=1 when the thin-kbd
>       is attached, but folded to the back

Right.

> 3. Make hotkey_get_tablet_mode support the new TP_HOTKEY_TABLET_USES_GTOP case and
>    make it call GTOP with parameter 0x0200 and check bit 0 + bit 16
> 4. On the events which you identified call tpacpi_input_send_tabletsw()

Works like a charm.
I realized that the device also emits 0x60c0 (TP_HKEY_EV_TABLET_CHANGED) when the keyboard cover is attached or detached, yet *not* when it's folded. I don't quite get why I nevertheless receive only one notification to userspace according to acpi_listen, despite the fact that the 0x60c0 handler also calls tpacpi_input_send_tabletsw and hotkey_tablet_mode_notify_change. Is there a deduplication behind the scenes?

I also realized that intel_vbtn reports the change, too. Would it be in order to modify intel_vbtn in a next step and blacklist this device to avoid duplicates?

On the other hand, userspace should expect duplicate messages to some degree and use a hysteresis approach anyway. Every now and then, the contact of the magnetic plug is not established perfectly on the first attempt. So perhaps not really an issue.


> And can you also check if there are events when folding the kbd behind the tablet?

Works perfectly.


Cheers,
Alex
Hans de Goede Feb. 11, 2021, 4:24 p.m. UTC | #9
Hi Alexander,

On 2/10/21 6:51 PM, Alexander Kobel wrote:
> Hi Hans, Nitin,
> 
> thanks a lot for your help.
> 
> On 2/8/21 10:04 AM, Hans de Goede wrote:
>> <snip>
>> Alexander, so it looks like we need to do the following to support this properly:
> 
> Spot on. Thanks for the detailed guide; I wouldn't have been able to get there in due time without your advise.
> I will send the revised patch in a separate mail, but want to highlight few points.
> 
>> 1. At a new TP_HOTKEY_TABLET_USES_GTOP to the hotkey_tablet enum
>> 2. At probe time in hotkey_init_tablet_mode add a new if / case with I guess
>>    the highest prio (so try before GMMS) which does:
>>    1. Call GTOP with parameter 0x0000, check return equals 0x0103 (or newer?)
>>       Nitin, how should the version check look like here, check that the
>>       upper byte == 0x01 and the lower byte >= 0x03 ?
> 
> Exactly.
> 
>>    2. Call GTOP with parameter 0x0100, check return value equals 4
> 
> I also included the simpler "any type" interface which does not report folding to the back. I confirmed that it works on my X1 with limited functionality, but don't have a "any type" device to test available. (In fact, I don't know whether such a device even exists.) IIUC it can also easily cover type 2 "Helix" and type 3 "Thinkpad 10" with the functionality available on those types. Since I cannot test, I didn't enable that in my patch; but it should be a matter of checking for return values 2 and 3 and adding an appropriate case in hotkey_get_tablet_mode.
> By the way, Nitin kindly confirmed that I can translate the GTOP reference sheet to source code comments, and provided the mapping between type 2, 3, 4 and series "Helix", "Thinkpad 10" and "X1 Tablet".
> 
>>    3. Call GTOP with parameter 0x0200, set in_tablet_mode based on bit 0
>>       and bit 16. I think we should report SW_TABLET_MODE=1 when the thin-kbd
>>       is attached, but folded to the back
> 
> Right.
> 
>> 3. Make hotkey_get_tablet_mode support the new TP_HOTKEY_TABLET_USES_GTOP case and
>>    make it call GTOP with parameter 0x0200 and check bit 0 + bit 16
>> 4. On the events which you identified call tpacpi_input_send_tabletsw()
> 
> Works like a charm.
> I realized that the device also emits 0x60c0 (TP_HKEY_EV_TABLET_CHANGED) when the keyboard cover is attached or detached, yet *not* when it's folded. I don't quite get why I nevertheless receive only one notification to userspace according to acpi_listen, despite the fact that the 0x60c0 handler also calls tpacpi_input_send_tabletsw and hotkey_tablet_mode_notify_change. Is there a deduplication behind the scenes?

Yes the input subsystem layer will not send events when nothing has changed.

> I also realized that intel_vbtn reports the change, too. Would it be in order to modify intel_vbtn in a next step and blacklist this device to avoid duplicates?

Hmm, that is a bit of a problem I would prefer to avoid having to deny-list things in intel_vbtn.

So do the 2 behave exactly the same? Also wrt when the kbd is folded behind the kbd. IOW
are the 2 SW_TABLET_MODE reports fully in sync in all possible states:

1. Just the tablet
2. Tablet + keyboard attached, keyboard in use
3. Tablet + keyboard attached, keyboard folded away behind tablet 

?

> On the other hand, userspace should expect duplicate messages to some degree and use a hysteresis approach anyway. Every now and then, the contact of the magnetic plug is not established perfectly on the first attempt. So perhaps not really an issue.

The only userspace consumer of this which I know is mutter (part of gnome-shell) and it
will just take the value from the last event. So if the 2 are always in sync then
the event send by the second input-dev will essentially be a no-op since the value will
be the same as the other event.

We do need to resolve the question of how to handle this before I can merge the patch,
atm I think that just having it reported twice is fine (as long as both reports are in
sync).

Regards,

Hans
Alexander Kobel Feb. 11, 2021, 11:07 p.m. UTC | #10
Hi Hans,

and thanks also for the source code review. I will address those valid points once I know whether the patch might be accepted, see below.


On 2/11/21 5:24 PM, Hans de Goede wrote:
> Hi Alexander,
> 
> On 2/10/21 6:51 PM, Alexander Kobel wrote:
>> <snip>>> Works like a charm.
>> I realized that the device also emits 0x60c0 (TP_HKEY_EV_TABLET_CHANGED) when the keyboard cover is attached or detached, yet *not* when it's folded. I don't quite get why I nevertheless receive only one notification to userspace according to acpi_listen, despite the fact that the 0x60c0 handler also calls tpacpi_input_send_tabletsw and hotkey_tablet_mode_notify_change. Is there a deduplication behind the scenes?
> 
> Yes the input subsystem layer will not send events when nothing has changed.

I see, thanks for the confirmation.

>> I also realized that intel_vbtn reports the change, too. Would it be in order to modify intel_vbtn in a next step and blacklist this device to avoid duplicates?
> 
> Hmm, that is a bit of a problem I would prefer to avoid having to deny-list things in intel_vbtn.

Agreed.

> So do the 2 behave exactly the same? Also wrt when the kbd is folded behind the kbd. IOW
> are the 2 SW_TABLET_MODE reports fully in sync in all possible states:
> 
> 1. Just the tablet
> 2. Tablet + keyboard attached, keyboard in use
> 3. Tablet + keyboard attached, keyboard folded away behind tablet 
> 
> ?

They are in sync, at least as soon as the state changes and an event is emitted. The only functional difference seems to be that thinkpad_acpi offers the sysfs entry hotkey_tablet_mode to read the current state, that's it.
This is nice after bootup or for scripts started at random time without the chance of observing state changes. E.g., I'd like to have autorotation triggered via the orientation sensor, but only if the device is in tablet mode; so my autorotation handler would read that sysfs entry as the first thing. If there's no way to read the state, I have to resort to watching the state toggle and cache it for myself in userspace.
But perhaps intel-vbtn offers a similar interface for that purpose that I don't know?

I can almost work around that by checking for the existence of the "PRIMAX ThinkPad X1 Tablet Thin Keyboard Gen 2" input. But that's not a nice workaround, and it doesn't detect the folding away (input is still registered, although the firmware doesn't send key presses anymore).

So, indeed the benefit of my patch is rather minor. If that means it should be discarded, that's fine for me (I learned a lot while writing and refining it, always nice). If someone can give me a hint on how to read intel-vbtn state one-shot, even better, then it'd be mostly obsolete.

What might be more interesting is the potential handling different attachable devices, such as the portable dock (I guess this is the "Pico Cartridge", since it comes with another battery). For this one, I would actually expect an SW_DOCK event, and via the GTOP queries, this could be detected and distinguished from other attachment options. I assume intel-vbtn can't cover that case out-of-the-box.
Unfortunately, I don't have such a dock (yet), so that's just guessing.


Out of curiosity: is your ThinkPad 10 fully handled by intel-vbtn?


>> On the other hand, userspace should expect duplicate messages to some degree and use a hysteresis approach anyway. Every now and then, the contact of the magnetic plug is not established perfectly on the first attempt. So perhaps not really an issue.
> 
> The only userspace consumer of this which I know is mutter (part of gnome-shell) and it
> will just take the value from the last event. So if the 2 are always in sync then
> the event send by the second input-dev will essentially be a no-op since the value will
> be the same as the other event.

Well, naturally another consumer is the acpi framework, e.g., acpi_listen or acpid. There, we have the possibility to install user-defined handlers. The same holds for window manager handlers such as sway's bindswitch tablet:{on,off,toggle} or, I presume, xbindkeys.

IMHO all reasonable handlers are idempotent, but users can be arbitrarily crazy. As mentioned, even if events are emitted exactly once on the software side, non-idempotent behavior will still occasionally be buggy, because the hardware connection is dodgy at times.

> We do need to resolve the question of how to handle this before I can merge the patch,
> atm I think that just having it reported twice is fine (as long as both reports are in
> sync).

They are in sync, that much I can confirm. But as mentioned, if you refrain from integrating the patch, I'm fine with that.
In that case, we should probably just add a dummy handler for HKEYs 0x4012 and 0x4013 with comments towards intel-vbtn, to avoid the unknown HKEY warning cluttering the system log.


Cheers,
Alexander


> Regards,
> 
> Hans
Hans de Goede Feb. 11, 2021, 11:42 p.m. UTC | #11
Hi,

On 2/9/21 4:16 PM, Alexander Kobel wrote:
> Hi,
> 
> On 2/8/21 11:17 AM, Hans de Goede wrote:
>> On 2/7/21 6:55 PM, Alexander Kobel wrote:
>>> <snip>
>>> I'll go off and try to improve.
>>
>> So Nitin has been kind enough to provide us with some docs for this,
>> please see me reply to Nitin's email and lets continue this part of this mail
>> thread there.
> 
> Right. I have the other patch ready, thanks to your great help. I'm
> waiting for Nitin's okay whether / how much info I can copy from the
> reference sheet to source code comments. Once I have that confirmation,
> I will post the revised patch.
> 
>> <snip>
>>
>>> Finally, I mentioned some open ends already on a post to ibm-acpi-devel
>>> at https://sourceforge.net/p/ibm-acpi/mailman/message/37200082/; this
>>> very question is among them.
>>> I will start tackling the SW_TABLET_MODE event issue first, but if Mark
>>> and Nitin can already hint about the keyboard shortcuts, it'd be highly
>>> appreciated.
>>
>> I think I might be able to help there, a couple of months ago I bought
>> a second-hand thinkpad-10 tablet which also has a USB attached keyboard.
>>
>> In hindsight I guess I could have asked Mark and Nitin for some more info,
>> but I went on autopilot and just ran hexdump -C on the /dev/hidraw node
>> to see which events all the keys send.
>>
>> And I fired up an usb-sniffer under Windows to figure out the audio-leds,
>> since I'm used to just figure these things out without help from the vendor :)
> 
> Yeah, why take the boring route if you know how to do all the work on
> your own... ;-)
> 
>> See the recent commits here for my work on this:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-lenovo.c
> 
> Thanks, very helpful.
> 
>> So on the ibm-acpi list message you said that the kbd sends the following:
>>
>> type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001
>> type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1
>>
>> For the Fn-keys, does it send the same MSC_SCAN code for *all* the
>> non-working Fn-keys ?
> 
> Correct. And I can confirm that /dev/hidraw1 lets me distinguish between
> the keys.
> 
>> If so then it seems that this is very much like the thinkpad 10 kbd dock
>> which also does this, see the lenovo_input_mapping_tp10_ultrabook_kbd()
>> function in drivers/hid/hid-lenovo.c .
>>
>> If I have that right, then I think we should be able to get the
>> Fn keys to work without too much trouble. You could try hacking up
>> drivers/hid/hid-lenovo.c a bit:
> 
> (Not yet there, but will investigate.)
> 
>> 1. Add an entry to the lenovo_devices array like this:
>>
>> 	/*
>> 	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard part,
>> 	 * while letting hid-multitouch.c handle the touchpad and trackpoint.
>> 	 */
>>         { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>>                      USB_VENDOR_ID_LENOVO,
>>                      USB_DEVICE_ID_LENOVO_X1_TAB),
>>
>> 2. Add the following entry to the switch-case in lenovo_input_mapping() :
>>
>>         case USB_DEVICE_ID_LENOVO_X1_TAB:
>>                 return lenovo_input_mapping_tp10_ultrabook_kbd(hdev, hi, field,
>>                                                                usage, bit, max);
>>
>> And then build hid-lenovo.c and modprobe it.
>>
>> After the modprobe to:
>>
>> ls -l /sys/bus/hid/devices/0003:17EF:60A3.*/driver
>>
>> This should show 2 devices (I guess) with one being bound to hid-lenovo
>> and 1 being bound to hid-multitouch.
> 
> So far (without patching hid-lenovo), 2 bound to hid-generic and 1 to
> hid-multitouch.

Ok, so it seems that they kept the thinkpad 10 kbd bits (mostly) with
1 keyboard interface using the usb boot kbd interface (so that it will
also work inside the BIOS) and a second interface for multimedia-keys +
the mouse emulation of the thinkpad 10 touchpad, those are interfaces
1 and 2, except that they removed the mouse emulation as they added a
new proper multi-touch capable touchpad as interface 3; and that one
also handles the pointing stick I believe.

So yes 2 bound to hid-generic, 1 bound to hid-multitouch seems to be
correct.

>> If this works some of your Fn + F# keys will now hopefully start doing
>> something, you can play around with modifying lenovo_input_mapping_tp10_ultrabook_kbd
>> to make it do the right thing for your kbd.
>>z
>> ###
>>
>> About LED support, just enabling the LED support bits for the
>> USB_DEVICE_ID_LENOVO_TP10UBKBD handling for now might work fine,
>> but there is a tiny chance that sending the wrong command somehow puts
>> the kbd in firmware update mode, I had that happen once with a Logitech
>> kbd which did not seem to have any kind of handshake / passcode to avoid
>> accidental fw updates (*).
>>
>> If you can give me a dump of the hid-descriptors for your keyboard,
>> then I can check if that the LEDs might work the same way too (or not).
>>
>> The easiest way to get a dump is to run the following command as root:
>>
>> cat /sys/kernel/debug/hid/0003:17EF:60A3.*/rdesc > rdesc
>>
>> And then attach rdesc to your next email.
> 
> Please find this one attached already now.
> In case it helps, the * expands to 0057 0058 0059 on my system.

Ok, so there still is an output-report number 9 on the second interface,
which probably still controls the LEDS but its descriptors are subtly
different. Although different in a good way I guess because the thinkpad
10 dock descriptor describes the 2 bytes in the output report as being
in the range of 0-1 which is not how they are actually used.

So I think that the code for the Thinkpad 10 ultrabook keyboard as
Lenovo calls it, should also work on the X1 tablet thin keyboard.

I've prepared a set of patches which enable the tp10ubkbd code on
the X1 tablet thin keyboard. But beware as mentioned before there is a
tiny chance that sending the wrong command somehow puts the kbd in
firmware update mode. I believe that trying the tp10ubkbd code is safe,
esp. since this is using a 2 byte large output report and using that
for fw-updating would be a bit weird. Still there is a small risk
(there always is when poking hw) so I will leave it up to you if
you are willing to try this.

Here is how I test this (note you will need to adjust the paths a bit) :

Toggle the 2 mute LEDs:

[root@localhost ~]# echo 1 > /sys/class/leds/0003:17EF:6062.000E:amber:micmute/brightness
[root@localhost ~]# echo 0 > /sys/class/leds/0003:17EF:6062.000E:amber:micmute/brightness
[root@localhost ~]# echo 1 > /sys/class/leds/0003:17EF:6062.000E:amber:mute/brightness
[root@localhost ~]# echo 0 > /sys/class/leds/0003:17EF:6062.000E:amber:mute/brightness

Check Fnlock LED state (toggle on kbd by pressing Fn + Esc) :

[root@localhost ~]# cat /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
1
[root@localhost ~]# cat /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
0

Change Fnlock state from within Linux:

[root@localhost ~]# echo 1 > /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
[root@localhost ~]# echo 0 > /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock

(The Led on the kbd should update; and the F## key behavior should change)

Regards,

Hans
Hans de Goede Feb. 11, 2021, 11:51 p.m. UTC | #12
Hi,

On 2/12/21 12:07 AM, Alexander Kobel wrote:
> Hi Hans,
> 
> and thanks also for the source code review. I will address those valid points once I know whether the patch might be accepted, see below.
> 
> 
> On 2/11/21 5:24 PM, Hans de Goede wrote:
>> Hi Alexander,
>>
>> On 2/10/21 6:51 PM, Alexander Kobel wrote:
>>> <snip>>> Works like a charm.
>>> I realized that the device also emits 0x60c0 (TP_HKEY_EV_TABLET_CHANGED) when the keyboard cover is attached or detached, yet *not* when it's folded. I don't quite get why I nevertheless receive only one notification to userspace according to acpi_listen, despite the fact that the 0x60c0 handler also calls tpacpi_input_send_tabletsw and hotkey_tablet_mode_notify_change. Is there a deduplication behind the scenes?
>>
>> Yes the input subsystem layer will not send events when nothing has changed.
> 
> I see, thanks for the confirmation.
> 
>>> I also realized that intel_vbtn reports the change, too. Would it be in order to modify intel_vbtn in a next step and blacklist this device to avoid duplicates?
>>
>> Hmm, that is a bit of a problem I would prefer to avoid having to deny-list things in intel_vbtn.
> 
> Agreed.
> 
>> So do the 2 behave exactly the same? Also wrt when the kbd is folded behind the kbd. IOW
>> are the 2 SW_TABLET_MODE reports fully in sync in all possible states:
>>
>> 1. Just the tablet
>> 2. Tablet + keyboard attached, keyboard in use
>> 3. Tablet + keyboard attached, keyboard folded away behind tablet 
>>
>> ?
> 
> They are in sync, at least as soon as the state changes and an event is emitted. The only functional difference seems to be that thinkpad_acpi offers the sysfs entry hotkey_tablet_mode to read the current state, that's it.
> This is nice after bootup or for scripts started at random time without the chance of observing state changes. E.g., I'd like to have autorotation triggered via the orientation sensor, but only if the device is in tablet mode; so my autorotation handler would read that sysfs entry as the first thing. If there's no way to read the state, I have to resort to watching the state toggle and cache it for myself in userspace.
> But perhaps intel-vbtn offers a similar interface for that purpose that I don't know?

No, but you can query the switch state with the evtest util.

> I can almost work around that by checking for the existence of the "PRIMAX ThinkPad X1 Tablet Thin Keyboard Gen 2" input. But that's not a nice workaround, and it doesn't detect the folding away (input is still registered, although the firmware doesn't send key presses anymore).
> 
> So, indeed the benefit of my patch is rather minor. If that means it should be discarded, that's fine for me (I learned a lot while writing and refining it, always nice). If someone can give me a hint on how to read intel-vbtn state one-shot, even better, then it'd be mostly obsolete.

I was surprised that your device supports intel-vbtn, there might very well be other X1 tablet generations / models which support GTOP but not intel-vbtn...

> What might be more interesting is the potential handling different attachable devices, such as the portable dock (I guess this is the "Pico Cartridge", since it comes with another battery). For this one, I would actually expect an SW_DOCK event, and via the GTOP queries, this could be detected and distinguished from other attachment options. I assume intel-vbtn can't cover that case out-of-the-box.

Right, that would be another case where having GTOP support would be helpful.

> Unfortunately, I don't have such a dock (yet), so that's just guessing.
> 
> 
> Out of curiosity: is your ThinkPad 10 fully handled by intel-vbtn?


I've the first generation ThinkPad 10 with a Bay Trail SoC. Not only does it not have intel-vbtn support, it also does not have GTOP support.

I just checked and currently thinkpad_acpi does not even load on my ThinkPad 10. It does a "HKEY" device, but with a HID of LEN0168, where as thinkpad_acpi only binds to 
LEN0068 and LEN0268. I could make thinkpad_acpi bind, but it won't do anything useful. I just checked and there is nothing useful in the whole LEN0168 HKEY device.

>>> On the other hand, userspace should expect duplicate messages to some degree and use a hysteresis approach anyway. Every now and then, the contact of the magnetic plug is not established perfectly on the first attempt. So perhaps not really an issue.
>>
>> The only userspace consumer of this which I know is mutter (part of gnome-shell) and it
>> will just take the value from the last event. So if the 2 are always in sync then
>> the event send by the second input-dev will essentially be a no-op since the value will
>> be the same as the other event.
> 
> Well, naturally another consumer is the acpi framework, e.g., acpi_listen or acpid. There, we have the possibility to install user-defined handlers. The same holds for window manager handlers such as sway's bindswitch tablet:{on,off,toggle} or, I presume, xbindkeys.
> 
> IMHO all reasonable handlers are idempotent, but users can be arbitrarily crazy. As mentioned, even if events are emitted exactly once on the software side, non-idempotent behavior will still occasionally be buggy, because the hardware connection is dodgy at times.
> 
>> We do need to resolve the question of how to handle this before I can merge the patch,
>> atm I think that just having it reported twice is fine (as long as both reports are in
>> sync).
> 
> They are in sync, that much I can confirm. But as mentioned, if you refrain from integrating the patch, I'm fine with that.
> In that case, we should probably just add a dummy handler for HKEYs 0x4012 and 0x4013 with comments towards intel-vbtn, to avoid the unknown HKEY warning cluttering the system log.

Hmm, I'm honestly not sure what to do here...

I guess we can always grab your patch from the archives if it turns out to be useful on another device.

And until then a dummy handler indeed is probably best.

Can you do a v2 of your original patch with:

1. A comment about SW_TABLET_MODE being handled by intel-vbtn
2. In that comment also put a link to this mailinglist discussion:
https://lore.kernel.org/platform-driver-x86/38cb8265-1e30-d547-9e12-b4ae290be737@a-kobel.de/

Regards,

Hans
Alexander Kobel Feb. 13, 2021, 3:05 p.m. UTC | #13
Hi,

On 2/12/21 12:51 AM, Hans de Goede wrote:
> Hi,
> 
> On 2/12/21 12:07 AM, Alexander Kobel wrote:
>> Hi Hans,
>>
>> and thanks also for the source code review. I will address those valid points once I know whether the patch might be accepted, see below.
>>
>>
>> On 2/11/21 5:24 PM, Hans de Goede wrote:
>>> Hi Alexander,
>>>
>>> On 2/10/21 6:51 PM, Alexander Kobel wrote:
>>>> <snip>>> Works like a charm.
>>>> I realized that the device also emits 0x60c0 (TP_HKEY_EV_TABLET_CHANGED) when the keyboard cover is attached or detached, yet *not* when it's folded. I don't quite get why I nevertheless receive only one notification to userspace according to acpi_listen, despite the fact that the 0x60c0 handler also calls tpacpi_input_send_tabletsw and hotkey_tablet_mode_notify_change. Is there a deduplication behind the scenes?
>>>
>>> Yes the input subsystem layer will not send events when nothing has changed.
>>
>> I see, thanks for the confirmation.
>>
>>>> I also realized that intel_vbtn reports the change, too. Would it be in order to modify intel_vbtn in a next step and blacklist this device to avoid duplicates?
>>>
>>> Hmm, that is a bit of a problem I would prefer to avoid having to deny-list things in intel_vbtn.
>>
>> Agreed.
>>
>>> So do the 2 behave exactly the same? Also wrt when the kbd is folded behind the kbd. IOW
>>> are the 2 SW_TABLET_MODE reports fully in sync in all possible states:
>>>
>>> 1. Just the tablet
>>> 2. Tablet + keyboard attached, keyboard in use
>>> 3. Tablet + keyboard attached, keyboard folded away behind tablet 
>>>
>>> ?
>>
>> They are in sync, at least as soon as the state changes and an event is emitted. The only functional difference seems to be that thinkpad_acpi offers the sysfs entry hotkey_tablet_mode to read the current state, that's it.
>> This is nice after bootup or for scripts started at random time without the chance of observing state changes. E.g., I'd like to have autorotation triggered via the orientation sensor, but only if the device is in tablet mode; so my autorotation handler would read that sysfs entry as the first thing. If there's no way to read the state, I have to resort to watching the state toggle and cache it for myself in userspace.
>> But perhaps intel-vbtn offers a similar interface for that purpose that I don't know?
> 
> No, but you can query the switch state with the evtest util.

Ah! Great, thanks for the hint. This also reports the correct state with just intel-vbtn, so really no point in pursuing my patch (unless someone/me eventually bothers with other attachments).


>> I can almost work around that by checking for the existence of the "PRIMAX ThinkPad X1 Tablet Thin Keyboard Gen 2" input. But that's not a nice workaround, and it doesn't detect the folding away (input is still registered, although the firmware doesn't send key presses anymore).
>>
>> So, indeed the benefit of my patch is rather minor. If that means it should be discarded, that's fine for me (I learned a lot while writing and refining it, always nice). If someone can give me a hint on how to read intel-vbtn state one-shot, even better, then it'd be mostly obsolete.
> 
> I was surprised that your device supports intel-vbtn, there might very well be other X1 tablet generations / models which support GTOP but not intel-vbtn...

We'll have the code in the archive for easy access, no problem.


>> What might be more interesting is the potential handling different attachable devices, such as the portable dock (I guess this is the "Pico Cartridge", since it comes with another battery). For this one, I would actually expect an SW_DOCK event, and via the GTOP queries, this could be detected and distinguished from other attachment options. I assume intel-vbtn can't cover that case out-of-the-box.
> 
> Right, that would be another case where having GTOP support would be helpful.

It's on my watchlist for cheap finds on eBay, so this might happen at some point.


>> Unfortunately, I don't have such a dock (yet), so that's just guessing.
>>
>>
>> Out of curiosity: is your ThinkPad 10 fully handled by intel-vbtn?
> 
> 
> I've the first generation ThinkPad 10 with a Bay Trail SoC. Not only does it not have intel-vbtn support, it also does not have GTOP support.
> 
> I just checked and currently thinkpad_acpi does not even load on my ThinkPad 10. It does a "HKEY" device, but with a HID of LEN0168, where as thinkpad_acpi only binds to 
> LEN0068 and LEN0268. I could make thinkpad_acpi bind, but it won't do anything useful. I just checked and there is nothing useful in the whole LEN0168 HKEY device.

I see, thanks.

>>>> On the other hand, userspace should expect duplicate messages to some degree and use a hysteresis approach anyway. Every now and then, the contact of the magnetic plug is not established perfectly on the first attempt. So perhaps not really an issue.
>>>
>>> The only userspace consumer of this which I know is mutter (part of gnome-shell) and it
>>> will just take the value from the last event. So if the 2 are always in sync then
>>> the event send by the second input-dev will essentially be a no-op since the value will
>>> be the same as the other event.
>>
>> Well, naturally another consumer is the acpi framework, e.g., acpi_listen or acpid. There, we have the possibility to install user-defined handlers. The same holds for window manager handlers such as sway's bindswitch tablet:{on,off,toggle} or, I presume, xbindkeys.
>>
>> IMHO all reasonable handlers are idempotent, but users can be arbitrarily crazy. As mentioned, even if events are emitted exactly once on the software side, non-idempotent behavior will still occasionally be buggy, because the hardware connection is dodgy at times.
>>
>>> We do need to resolve the question of how to handle this before I can merge the patch,
>>> atm I think that just having it reported twice is fine (as long as both reports are in
>>> sync).
>>
>> They are in sync, that much I can confirm. But as mentioned, if you refrain from integrating the patch, I'm fine with that.
>> In that case, we should probably just add a dummy handler for HKEYs 0x4012 and 0x4013 with comments towards intel-vbtn, to avoid the unknown HKEY warning cluttering the system log.
> 
> Hmm, I'm honestly not sure what to do here...
> 
> I guess we can always grab your patch from the archives if it turns out to be useful on another device.
> 
> And until then a dummy handler indeed is probably best.

I think so, too.

> Can you do a v2 of your original patch with:
> 
> 1. A comment about SW_TABLET_MODE being handled by intel-vbtn
> 2. In that comment also put a link to this mailinglist discussion:
> https://lore.kernel.org/platform-driver-x86/38cb8265-1e30-d547-9e12-b4ae290be737@a-kobel.de/

Will be on the list in few minutes.

Note that I added send_acpi_ev=false and ignore_acpi_ev=true.  I guess that's correct?
Also, I did not bother to add the pr_info.  Mostly because, after the considerations of the week, I don't know whether the message should be "keyboard cover attached" or "something attached".  Same for the names in the enum, too - it could be that "TP_HKEY_EV_ATTACH_SOMETHING" would be more apt, but I don't know when exactly the HKEY is sent.


Cheers,
Alex
Alexander Kobel Feb. 21, 2021, 1:17 p.m. UTC | #14
Hi,

finally I got to investigate that patch. Thanks for your draft and explanations!

On 2/12/21 12:42 AM, Hans de Goede wrote:
> Hi,
> 
> On 2/9/21 4:16 PM, Alexander Kobel wrote:
>> Hi,
>>
>> On 2/8/21 11:17 AM, Hans de Goede wrote:
>>> On 2/7/21 6:55 PM, Alexander Kobel wrote:
>>>> <snip>
>>>> I'll go off and try to improve.
>>>
>>> So Nitin has been kind enough to provide us with some docs for this,
>>> please see me reply to Nitin's email and lets continue this part of this mail
>>> thread there.
>>
>> Right. I have the other patch ready, thanks to your great help. I'm
>> waiting for Nitin's okay whether / how much info I can copy from the
>> reference sheet to source code comments. Once I have that confirmation,
>> I will post the revised patch.
>>
>>> <snip>
>>>
>>>> Finally, I mentioned some open ends already on a post to ibm-acpi-devel
>>>> at https://sourceforge.net/p/ibm-acpi/mailman/message/37200082/; this
>>>> very question is among them.
>>>> I will start tackling the SW_TABLET_MODE event issue first, but if Mark
>>>> and Nitin can already hint about the keyboard shortcuts, it'd be highly
>>>> appreciated.
>>>
>>> I think I might be able to help there, a couple of months ago I bought
>>> a second-hand thinkpad-10 tablet which also has a USB attached keyboard.
>>>
>>> In hindsight I guess I could have asked Mark and Nitin for some more info,
>>> but I went on autopilot and just ran hexdump -C on the /dev/hidraw node
>>> to see which events all the keys send.
>>>
>>> And I fired up an usb-sniffer under Windows to figure out the audio-leds,
>>> since I'm used to just figure these things out without help from the vendor :)
>>
>> Yeah, why take the boring route if you know how to do all the work on
>> your own... ;-)
>>
>>> See the recent commits here for my work on this:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-lenovo.c
>>
>> Thanks, very helpful.
>>
>>> So on the ibm-acpi list message you said that the kbd sends the following:
>>>
>>> type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001
>>> type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1
>>>
>>> For the Fn-keys, does it send the same MSC_SCAN code for *all* the
>>> non-working Fn-keys ?
>>
>> Correct. And I can confirm that /dev/hidraw1 lets me distinguish between
>> the keys.
>>
>>> If so then it seems that this is very much like the thinkpad 10 kbd dock
>>> which also does this, see the lenovo_input_mapping_tp10_ultrabook_kbd()
>>> function in drivers/hid/hid-lenovo.c .
>>>
>>> If I have that right, then I think we should be able to get the
>>> Fn keys to work without too much trouble. You could try hacking up
>>> drivers/hid/hid-lenovo.c a bit:
>>
>> (Not yet there, but will investigate.)
>>
>>> 1. Add an entry to the lenovo_devices array like this:
>>>
>>> 	/*
>>> 	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard part,
>>> 	 * while letting hid-multitouch.c handle the touchpad and trackpoint.
>>> 	 */
>>>         { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>>>                      USB_VENDOR_ID_LENOVO,
>>>                      USB_DEVICE_ID_LENOVO_X1_TAB),
>>>
>>> 2. Add the following entry to the switch-case in lenovo_input_mapping() :
>>>
>>>         case USB_DEVICE_ID_LENOVO_X1_TAB:
>>>                 return lenovo_input_mapping_tp10_ultrabook_kbd(hdev, hi, field,
>>>                                                                usage, bit, max);
>>>
>>> And then build hid-lenovo.c and modprobe it.
>>>
>>> After the modprobe to:
>>>
>>> ls -l /sys/bus/hid/devices/0003:17EF:60A3.*/driver
>>>
>>> This should show 2 devices (I guess) with one being bound to hid-lenovo
>>> and 1 being bound to hid-multitouch.
>>
>> So far (without patching hid-lenovo), 2 bound to hid-generic and 1 to
>> hid-multitouch.
> 
> Ok, so it seems that they kept the thinkpad 10 kbd bits (mostly) with
> 1 keyboard interface using the usb boot kbd interface (so that it will
> also work inside the BIOS) and a second interface for multimedia-keys +
> the mouse emulation of the thinkpad 10 touchpad, those are interfaces
> 1 and 2, except that they removed the mouse emulation as they added a
> new proper multi-touch capable touchpad as interface 3; and that one
> also handles the pointing stick I believe.
> 
> So yes 2 bound to hid-generic, 1 bound to hid-multitouch seems to be
> correct.

Right, that's what I observe.

>>> If this works some of your Fn + F# keys will now hopefully start doing
>>> something, you can play around with modifying lenovo_input_mapping_tp10_ultrabook_kbd
>>> to make it do the right thing for your kbd.
>>> z
>>> ###
>>>
>>> About LED support, just enabling the LED support bits for the
>>> USB_DEVICE_ID_LENOVO_TP10UBKBD handling for now might work fine,
>>> but there is a tiny chance that sending the wrong command somehow puts
>>> the kbd in firmware update mode, I had that happen once with a Logitech
>>> kbd which did not seem to have any kind of handshake / passcode to avoid
>>> accidental fw updates (*).
>>>
>>> If you can give me a dump of the hid-descriptors for your keyboard,
>>> then I can check if that the LEDs might work the same way too (or not).
>>>
>>> The easiest way to get a dump is to run the following command as root:
>>>
>>> cat /sys/kernel/debug/hid/0003:17EF:60A3.*/rdesc > rdesc
>>>
>>> And then attach rdesc to your next email.
>>
>> Please find this one attached already now.
>> In case it helps, the * expands to 0057 0058 0059 on my system.
> 
> Ok, so there still is an output-report number 9 on the second interface,
> which probably still controls the LEDS but its descriptors are subtly
> different. Although different in a good way I guess because the thinkpad
> 10 dock descriptor describes the 2 bytes in the output report as being
> in the range of 0-1 which is not how they are actually used.
> 
> So I think that the code for the Thinkpad 10 ultrabook keyboard as
> Lenovo calls it, should also work on the X1 tablet thin keyboard.

Mostly, modulo some key mappings, as expected.

The good:

LEDs are working exactly as expected with your patch, with the appropriate triggers automatically active. Perfect!


The bad:

I could adjust some of the key mappings for the X1 Tablet 2nd keyboard. What I couldn't do is to get Fn+F10, Fn+F11, Fn+F12 and Fn+PrtSc to work.
Following the logic of /dev/hidraw1 capture (attached), those should be on usage_index 16 to 19. But apparently those are on a different usage page or something like that? Unfortunately, my RTFM skills didn't really help with figuring out how that's supposed to work.
(Is looking at the bit indices in /dev/hidraw traces how you figure out those mappings? If there's a better way, I'm eager to be told...)

Similarly - I assume - Fn+S should emit SysRq according to https://download.lenovo.com/pccbbs/mobiles_pdf/x1_tablet_gen_2_ug_en.pdf, page 51. This is not on the "consumer control" device, but the usual keyboard, so /dev/hidraw0. Again, couldn't get much further than producing a capture. But I cannot make sense of this one, because way more bits are set, so I cannot extrapolate from your code.


The ugly:

Fn+4 ("sleep") triggers the appropriate ACPI event button/sleep and emits something on /dev/hidraw0, too, but *only once*. After resuming, no reaction at all (neither on ACPI nor hidraw) until I unload and reload the hid_lenovo module.

Finally, keyboard backlight is handled in firmware, apparently; Fn+Space is visible on /dev/hidraw1 (see attached capture), but it toggles the backlight levels without any userspace code involved, as far as I can see.
Also, the keyboard backlight doesn't create an entry in /sys/class/backlight or the like, so neither read nor write access. Out-of-the-box, at least. But I'm not even sure if this is possible in Windows.


Bottom line: this is mostly usable already, modulo the adjustments for the different keys. I'd like to make F10 to F12 work before it hits testing; everything else is icing on the cake, I suppose. Do you have an hint for me how I can approach that?

Also, I'd make sure that this is about the "ThinkPad X1 Tablet Thin Keyboard **Gen 2**". The consumer functions are different for the **Gen 1** keyboard, so I would also adjust the function names. I do have an old Gen 1 keyboard lying around, but unfortunately it's either broken (it lights up shortly after attaching in Windows, but doesn't report keypresses at all, and pretends to be completely dead in Linux), or it's incompatible with my X1 Tablet 2nd Gen. So I cannot test how your patch might impact the Gen1, too... :-/


> I've prepared a set of patches which enable the tp10ubkbd code on
> the X1 tablet thin keyboard. But beware as mentioned before there is a
> tiny chance that sending the wrong command somehow puts the kbd in
> firmware update mode. I believe that trying the tp10ubkbd code is safe,
> esp. since this is using a 2 byte large output report and using that
> for fw-updating would be a bit weird. Still there is a small risk
> (there always is when poking hw) so I will leave it up to you if
> you are willing to try this.

No issue at all, and everything below works just as expected.

> Here is how I test this (note you will need to adjust the paths a bit) :
> 
> Toggle the 2 mute LEDs:
> 
> [root@localhost ~]# echo 1 > /sys/class/leds/0003:17EF:6062.000E:amber:micmute/brightness
> [root@localhost ~]# echo 0 > /sys/class/leds/0003:17EF:6062.000E:amber:micmute/brightness
> [root@localhost ~]# echo 1 > /sys/class/leds/0003:17EF:6062.000E:amber:mute/brightness
> [root@localhost ~]# echo 0 > /sys/class/leds/0003:17EF:6062.000E:amber:mute/brightness
> 
> Check Fnlock LED state (toggle on kbd by pressing Fn + Esc) :
> 
> [root@localhost ~]# cat /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
> 1
> [root@localhost ~]# cat /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
> 0
> 
> Change Fnlock state from within Linux:
> 
> [root@localhost ~]# echo 1 > /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
> [root@localhost ~]# echo 0 > /sys/bus/hid/devices/0003:17EF:6062.000E/fn_lock
> 
> (The Led on the kbd should update; and the F## key behavior should change)
> 
> Regards,
> 
> Hans


Cheers,
Alex
# echo 'Fn + [ s, 4 ]'; sleep 1; sudo xxd -b -c4 /dev/hidraw0
Fn + [ s, 4 ]

# Fn+S should be SysRq
Fn+S press:   00000000: 00000000 00000000 10011010 00000000  ....
              00000004: 00000000 00000000 00000000 00000000  ....
Fn+S release: 00000008: 00000000 00000000 00000000 00000000  ....
              0000000c: 00000000 00000000 00000000 00000000  ....

# Fn+4 should be "sleep"
Fn+4 press:   00000000: 00000000 00000000 01110010 00000000  ..r.
              00000004: 00000000 00000000 00000000 00000000  ....
Fn+4 release: 00000008: 00000000 00000000 00000000 00000000  ....
              0000000c: 00000000 00000000 00000000 00000000  ....
# subsequent presses of Fn+4 do not report anything
# until hid_lenovo is removed and re-inserted
# echo 'Fn + [ FnLock, F1, ..., F12, PrtSc, Space, Space, Space, Space ]'; xxd -b -c4 /dev/hidraw1
Fn + [ FnLock, F1, ..., F12, PrtSc, Space, Space, Space, Space ]

FnLock Press:   00000000: 00000011 00000000 00000001 00000000  ....
FnLock Release: 00000004: 00000011 00000000 00000000 00000000  ....

F1 Press:       00000008: 00000011 00100000 00000000 00000000  . ..
F1 Release:     0000000c: 00000011 00000000 00000000 00000000  ....
F2 Press:       00000010: 00000011 00000000 00000000 00010000  ....
F2 Release:     00000014: 00000011 00000000 00000000 00000000  ....
F3 Press:       00000018: 00000011 00000000 00000000 00100000  ...
F3 Release:     0000001c: 00000011 00000000 00000000 00000000  ....
F4 Press:       00000020: 00000011 00000000 00000010 00000000  ....
F4 Release:     00000024: 00000011 00000000 00000000 00000000  ....
F5 Press:       00000028: 00000011 00000000 00000000 01000000  ...@
F5 Release:     0000002c: 00000011 00000000 00000000 00000000  ....
F6 Press:       00000030: 00000011 00000000 00000000 00001000  ....
F6 Release:     00000034: 00000011 00000000 00000000 00000000  ....
F7 Press:       00000038: 00000011 00000000 00100000 00000000  .. .
F7 Release:     0000003c: 00000011 00000000 00000000 00000000  ....
F8 Press:       00000040: 00000011 00000000 01000000 00000000  ..@.
F8 Release:     00000044: 00000011 00000000 00000000 00000000  ....
F9 Press:       00000048: 00000011 00000000 00000100 00000000  ....
F9 Release:     0000004c: 00000011 00000000 00000000 00000000  ....
F10 Press:      00000050: 00000011 00000001 00000000 00000000  ....
F10 Release:    00000054: 00000011 00000000 00000000 00000000  ....
F11 Press:      00000058: 00000011 00000010 00000000 00000000  ....
F11 Release:    0000005c: 00000011 00000000 00000000 00000000  ....
F12 Press:      00000060: 00000011 00000100 00000000 00000000  ....
F12 Release:    00000064: 00000011 00000000 00000000 00000000  ....

PrtSc Press:    00000068: 00000011 00001000 00000000 00000000  ....
PrtSc Release:  0000006c: 00000011 00000000 00000000 00000000  ....

Space Press:    00000070: 00000011 00000000 10010000 00000001  ....
Space Press:    00000074: 00000011 00000000 10010000 00000001  ....
Space Press:    00000078: 00000011 00000000 10010000 00000001  ....
Space Press:    0000007c: 00000011 00000000 10010000 00000001  ....
Hans de Goede Feb. 21, 2021, 4:30 p.m. UTC | #15
Hi,

On 2/21/21 2:17 PM, Alexander Kobel wrote:
> Hi,
> 
> finally I got to investigate that patch. Thanks for your draft and explanations!
> 
> On 2/12/21 12:42 AM, Hans de Goede wrote:
>> Hi,
>>
>> On 2/9/21 4:16 PM, Alexander Kobel wrote:
>>> Hi,
>>>
>>> On 2/8/21 11:17 AM, Hans de Goede wrote:
>>>> On 2/7/21 6:55 PM, Alexander Kobel wrote:
>>>>> <snip>
>>>>> I'll go off and try to improve.
>>>>
>>>> So Nitin has been kind enough to provide us with some docs for this,
>>>> please see me reply to Nitin's email and lets continue this part of this mail
>>>> thread there.
>>>
>>> Right. I have the other patch ready, thanks to your great help. I'm
>>> waiting for Nitin's okay whether / how much info I can copy from the
>>> reference sheet to source code comments. Once I have that confirmation,
>>> I will post the revised patch.
>>>
>>>> <snip>
>>>>
>>>>> Finally, I mentioned some open ends already on a post to ibm-acpi-devel
>>>>> at https://sourceforge.net/p/ibm-acpi/mailman/message/37200082/; this
>>>>> very question is among them.
>>>>> I will start tackling the SW_TABLET_MODE event issue first, but if Mark
>>>>> and Nitin can already hint about the keyboard shortcuts, it'd be highly
>>>>> appreciated.
>>>>
>>>> I think I might be able to help there, a couple of months ago I bought
>>>> a second-hand thinkpad-10 tablet which also has a USB attached keyboard.
>>>>
>>>> In hindsight I guess I could have asked Mark and Nitin for some more info,
>>>> but I went on autopilot and just ran hexdump -C on the /dev/hidraw node
>>>> to see which events all the keys send.
>>>>
>>>> And I fired up an usb-sniffer under Windows to figure out the audio-leds,
>>>> since I'm used to just figure these things out without help from the vendor :)
>>>
>>> Yeah, why take the boring route if you know how to do all the work on
>>> your own... ;-)
>>>
>>>> See the recent commits here for my work on this:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-lenovo.c
>>>
>>> Thanks, very helpful.
>>>
>>>> So on the ibm-acpi list message you said that the kbd sends the following:
>>>>
>>>> type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001
>>>> type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1
>>>>
>>>> For the Fn-keys, does it send the same MSC_SCAN code for *all* the
>>>> non-working Fn-keys ?
>>>
>>> Correct. And I can confirm that /dev/hidraw1 lets me distinguish between
>>> the keys.
>>>
>>>> If so then it seems that this is very much like the thinkpad 10 kbd dock
>>>> which also does this, see the lenovo_input_mapping_tp10_ultrabook_kbd()
>>>> function in drivers/hid/hid-lenovo.c .
>>>>
>>>> If I have that right, then I think we should be able to get the
>>>> Fn keys to work without too much trouble. You could try hacking up
>>>> drivers/hid/hid-lenovo.c a bit:
>>>
>>> (Not yet there, but will investigate.)
>>>
>>>> 1. Add an entry to the lenovo_devices array like this:
>>>>
>>>> 	/*
>>>> 	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard part,
>>>> 	 * while letting hid-multitouch.c handle the touchpad and trackpoint.
>>>> 	 */
>>>>         { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>>>>                      USB_VENDOR_ID_LENOVO,
>>>>                      USB_DEVICE_ID_LENOVO_X1_TAB),
>>>>
>>>> 2. Add the following entry to the switch-case in lenovo_input_mapping() :
>>>>
>>>>         case USB_DEVICE_ID_LENOVO_X1_TAB:
>>>>                 return lenovo_input_mapping_tp10_ultrabook_kbd(hdev, hi, field,
>>>>                                                                usage, bit, max);
>>>>
>>>> And then build hid-lenovo.c and modprobe it.
>>>>
>>>> After the modprobe to:
>>>>
>>>> ls -l /sys/bus/hid/devices/0003:17EF:60A3.*/driver
>>>>
>>>> This should show 2 devices (I guess) with one being bound to hid-lenovo
>>>> and 1 being bound to hid-multitouch.
>>>
>>> So far (without patching hid-lenovo), 2 bound to hid-generic and 1 to
>>> hid-multitouch.
>>
>> Ok, so it seems that they kept the thinkpad 10 kbd bits (mostly) with
>> 1 keyboard interface using the usb boot kbd interface (so that it will
>> also work inside the BIOS) and a second interface for multimedia-keys +
>> the mouse emulation of the thinkpad 10 touchpad, those are interfaces
>> 1 and 2, except that they removed the mouse emulation as they added a
>> new proper multi-touch capable touchpad as interface 3; and that one
>> also handles the pointing stick I believe.
>>
>> So yes 2 bound to hid-generic, 1 bound to hid-multitouch seems to be
>> correct.
> 
> Right, that's what I observe.
> 
>>>> If this works some of your Fn + F# keys will now hopefully start doing
>>>> something, you can play around with modifying lenovo_input_mapping_tp10_ultrabook_kbd
>>>> to make it do the right thing for your kbd.
>>>> z
>>>> ###
>>>>
>>>> About LED support, just enabling the LED support bits for the
>>>> USB_DEVICE_ID_LENOVO_TP10UBKBD handling for now might work fine,
>>>> but there is a tiny chance that sending the wrong command somehow puts
>>>> the kbd in firmware update mode, I had that happen once with a Logitech
>>>> kbd which did not seem to have any kind of handshake / passcode to avoid
>>>> accidental fw updates (*).
>>>>
>>>> If you can give me a dump of the hid-descriptors for your keyboard,
>>>> then I can check if that the LEDs might work the same way too (or not).
>>>>
>>>> The easiest way to get a dump is to run the following command as root:
>>>>
>>>> cat /sys/kernel/debug/hid/0003:17EF:60A3.*/rdesc > rdesc
>>>>
>>>> And then attach rdesc to your next email.
>>>
>>> Please find this one attached already now.
>>> In case it helps, the * expands to 0057 0058 0059 on my system.
>>
>> Ok, so there still is an output-report number 9 on the second interface,
>> which probably still controls the LEDS but its descriptors are subtly
>> different. Although different in a good way I guess because the thinkpad
>> 10 dock descriptor describes the 2 bytes in the output report as being
>> in the range of 0-1 which is not how they are actually used.
>>
>> So I think that the code for the Thinkpad 10 ultrabook keyboard as
>> Lenovo calls it, should also work on the X1 tablet thin keyboard.
> 
> Mostly, modulo some key mappings, as expected.
> 
> The good:
> 
> LEDs are working exactly as expected with your patch, with the appropriate triggers automatically active. Perfect!

Good :)

> The bad:
> 
> I could adjust some of the key mappings for the X1 Tablet 2nd keyboard. What I couldn't do is to get Fn+F10, Fn+F11, Fn+F12 and Fn+PrtSc to work.
> Following the logic of /dev/hidraw1 capture (attached), those should be on usage_index 16 to 19. But apparently those are on a different usage page or something like that? Unfortunately, my RTFM skills didn't really help with figuring out how that's supposed to work.
> (Is looking at the bit indices in /dev/hidraw traces how you figure out those mappings? If there's a better way, I'm eager to be told...)

You are swapping to low and high byte of the 3 data bytes in the report. Here is an annotated part of the descriptors to explain better:

  INPUT(3)[INPUT]
    Field(0)
      Application(Consumer.0001)
      Usage(24)
 0        Consumer.0001 F10 00000001 00000000 00000000
 1        Consumer.0001 F11 00000010 00000000 00000000
 2        Consumer.0001 F12 00000100 00000000 00000000
 3        Consumer.0001 Prt 00001000 00000000 00000000
 4        Consumer.0001
 5        Consumer.00e2 F1  00100000 00000000 00000000
 6        Consumer.0001
 7        Consumer.0001
 8        Consumer.0001 ESC 00000000 00000001 00000000
 9        Consumer.0001 F4  00000000 00000010 00000000
10        Consumer.0001 F9  00000000 00000100 00000000
11        Consumer.00b7
12        Consumer.0001
13        Consumer.0001 F7  00000000 00100000 00000000
14        Consumer.0001 F8  00000000 01000000 00000000
15        Consumer.0001 
16        Consumer.0001
17        Consumer.0001
18        Consumer.0001 
19        Consumer.006f F6  00000000 00000000 00001000
20        Consumer.00ea F2  00000000 00000000 00010000
21        Consumer.00e9 F3  00000000 00000000 00100000
22        Consumer.0070 F5  00000000 00000000 01000000
23        Consumer.0001
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(24)
      Report Offset(0)
      Flags( Variable Absolute )

Notice how the keys with standard codes (which work without mapping)
F1 - F3, F5, F6 now all line up with _none_ Consumer.0001 entries.
And if you check those codes in drivers/hid/hid-input.c around
line 960 you will see the standard mappings line up too.

IOW your case 16 needs to be case 0, case 17, case 1, etc.

> Similarly - I assume - Fn+S should emit SysRq according to https://download.lenovo.com/pccbbs/mobiles_pdf/x1_tablet_gen_2_ug_en.pdf, page 51. This is not on the "consumer control" device, but the usual keyboard, so /dev/hidraw0. Again, couldn't get much further than producing a capture. But I cannot make sense of this one, because way more bits are set, so I cannot extrapolate from your code.

The input report used by the Fn + key "media keys" use a 24 bit report
with 1 bit per key. The standard keyboard interface uses 1 byte per
pressed key (with a maximum of 6 pressed keys) where the full byte
encodes the scancode of the key. Normally SysRq is 0x46 but for some
reason your keyboard is sending 0x9a you can map this by adding the following
to the mapping function:

	if (usage->hid == (HID_UP_KEYBOARD | 0x009a))
		map_key_clear(KEY_SYSRQ);

Likewise for the sleep combo:

	if (usage->hid == (HID_UP_KEYBOARD | 0x0072))
		map_key_clear(KEY_SLEEP);

###

Note chances are you have more Fn + 'letter' combinations at least on
the thinkpad10 kbd I have:

Fn + T -> SysRq
Fn + I -> Insert
Fn + P -> Pause
Fn + S -> Sysrq
Fn + K -> ScrollLock
Fn + B -> Pause

Note these do not need any special mappings on the thinkpad10 kbd and I guess
the doubles may have something to do with some non qwerty keymaps.

> The ugly:
> 
> Fn+4 ("sleep") triggers the appropriate ACPI event button/sleep and emits something on /dev/hidraw0, too, but *only once*. After resuming, no reaction at all (neither on ACPI nor hidraw) until I unload and reload the hid_lenovo module.

As for the sleep key working only once, what happens after a suspend/resume ?

I think the key may have some special handling to avoid it sending
a second KEY_SLEEP when the user uses it to wakeup the system, to
avoid the system immediately going to sleep again when the user tries
to wake the system this way.

> Finally, keyboard backlight is handled in firmware, apparently; Fn+Space is visible on /dev/hidraw1 (see attached capture), but it toggles the backlight levels without any userspace code involved, as far as I can see.

Yes I saw this in your dump, this is really weird because it sets 3 bits at once in the INPUT(3) report.
Does it always set the same 3 bits independent of the brightness level ?

> Also, the keyboard backlight doesn't create an entry in /sys/class/backlight or the like, so neither read nor write access. Out-of-the-box, at least. But I'm not even sure if this is possible in Windows.

Actuallu kbd-backlighting used the /sys/class/leds interface, but yeah that is not support by Linux ATM for this kbd.

Yeah, adding support for that (assuming the hw can do it) would definitely require making some USB dumps under Windows
(after finding sw which can change it from within the OS under Windows).

> Bottom line: this is mostly usable already, modulo the adjustments for the different keys. I'd like to make F10 to F12 work before it hits testing; everything else is icing on the cake, I suppose. Do you have an hint for me how I can approach that?

See above, I think we are pretty close to solving this.

Note in the mean time I've posted a hid-lenovo patch-series with various improvements related to
the LED handling. I'll send you an offlist mail with the latest patches so that you can base any work you do on top of those.

> Also, I'd make sure that this is about the "ThinkPad X1 Tablet Thin Keyboard **Gen 2**". The consumer functions are different for the **Gen 1** keyboard, so I would also adjust the function names. I do have an old Gen 1 keyboard lying around, but unfortunately it's either broken (it lights up shortly after attaching in Windows, but doesn't report keypresses at all, and pretends to be completely dead in Linux), or it's incompatible with my X1 Tablet 2nd Gen. So I cannot test how your patch might impact the Gen1, too... :-/

I would expect the Gen1 to have a different product-id, so my patch shouldn't do anything.

>> I've prepared a set of patches which enable the tp10ubkbd code on
>> the X1 tablet thin keyboard. But beware as mentioned before there is a
>> tiny chance that sending the wrong command somehow puts the kbd in
>> firmware update mode. I believe that trying the tp10ubkbd code is safe,
>> esp. since this is using a 2 byte large output report and using that
>> for fw-updating would be a bit weird. Still there is a small risk
>> (there always is when poking hw) so I will leave it up to you if
>> you are willing to try this.
> 
> No issue at all, and everything below works just as expected.

Good.

Regards,

Hans
Alexander Kobel Feb. 22, 2021, 12:31 p.m. UTC | #16
Hi,

On 2/21/21 5:30 PM, Hans de Goede wrote:
> Hi,
> 
> On 2/21/21 2:17 PM, Alexander Kobel wrote:
>> Hi,
>>
>> finally I got to investigate that patch. Thanks for your draft and explanations!
>>
>> On 2/12/21 12:42 AM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 2/9/21 4:16 PM, Alexander Kobel wrote:
>>>> Hi,
>>>>
>>>> On 2/8/21 11:17 AM, Hans de Goede wrote:
>>>>> On 2/7/21 6:55 PM, Alexander Kobel wrote:
>>>>>> <snip>
>>>>>> I'll go off and try to improve.
>>>>>
>>>>> So Nitin has been kind enough to provide us with some docs for this,
>>>>> please see me reply to Nitin's email and lets continue this part of this mail
>>>>> thread there.
>>>>
>>>> Right. I have the other patch ready, thanks to your great help. I'm
>>>> waiting for Nitin's okay whether / how much info I can copy from the
>>>> reference sheet to source code comments. Once I have that confirmation,
>>>> I will post the revised patch.
>>>>
>>>>> <snip>
>>>>>
>>>>>> Finally, I mentioned some open ends already on a post to ibm-acpi-devel
>>>>>> at https://sourceforge.net/p/ibm-acpi/mailman/message/37200082/; this
>>>>>> very question is among them.
>>>>>> I will start tackling the SW_TABLET_MODE event issue first, but if Mark
>>>>>> and Nitin can already hint about the keyboard shortcuts, it'd be highly
>>>>>> appreciated.
>>>>>
>>>>> I think I might be able to help there, a couple of months ago I bought
>>>>> a second-hand thinkpad-10 tablet which also has a USB attached keyboard.
>>>>>
>>>>> In hindsight I guess I could have asked Mark and Nitin for some more info,
>>>>> but I went on autopilot and just ran hexdump -C on the /dev/hidraw node
>>>>> to see which events all the keys send.
>>>>>
>>>>> And I fired up an usb-sniffer under Windows to figure out the audio-leds,
>>>>> since I'm used to just figure these things out without help from the vendor :)
>>>>
>>>> Yeah, why take the boring route if you know how to do all the work on
>>>> your own... ;-)
>>>>
>>>>> See the recent commits here for my work on this:
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-lenovo.c
>>>>
>>>> Thanks, very helpful.
>>>>
>>>>> So on the ibm-acpi list message you said that the kbd sends the following:
>>>>>
>>>>> type 4 (EV_MSC), code 4 (MSC_SCAN), value c0001
>>>>> type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 1
>>>>>
>>>>> For the Fn-keys, does it send the same MSC_SCAN code for *all* the
>>>>> non-working Fn-keys ?
>>>>
>>>> Correct. And I can confirm that /dev/hidraw1 lets me distinguish between
>>>> the keys.
>>>>
>>>>> If so then it seems that this is very much like the thinkpad 10 kbd dock
>>>>> which also does this, see the lenovo_input_mapping_tp10_ultrabook_kbd()
>>>>> function in drivers/hid/hid-lenovo.c .
>>>>>
>>>>> If I have that right, then I think we should be able to get the
>>>>> Fn keys to work without too much trouble. You could try hacking up
>>>>> drivers/hid/hid-lenovo.c a bit:
>>>>
>>>> (Not yet there, but will investigate.)
>>>>
>>>>> 1. Add an entry to the lenovo_devices array like this:
>>>>>
>>>>> 	/*
>>>>> 	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard part,
>>>>> 	 * while letting hid-multitouch.c handle the touchpad and trackpoint.
>>>>> 	 */
>>>>>         { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>>>>>                      USB_VENDOR_ID_LENOVO,
>>>>>                      USB_DEVICE_ID_LENOVO_X1_TAB),
>>>>>
>>>>> 2. Add the following entry to the switch-case in lenovo_input_mapping() :
>>>>>
>>>>>         case USB_DEVICE_ID_LENOVO_X1_TAB:
>>>>>                 return lenovo_input_mapping_tp10_ultrabook_kbd(hdev, hi, field,
>>>>>                                                                usage, bit, max);
>>>>>
>>>>> And then build hid-lenovo.c and modprobe it.
>>>>>
>>>>> After the modprobe to:
>>>>>
>>>>> ls -l /sys/bus/hid/devices/0003:17EF:60A3.*/driver
>>>>>
>>>>> This should show 2 devices (I guess) with one being bound to hid-lenovo
>>>>> and 1 being bound to hid-multitouch.
>>>>
>>>> So far (without patching hid-lenovo), 2 bound to hid-generic and 1 to
>>>> hid-multitouch.
>>>
>>> Ok, so it seems that they kept the thinkpad 10 kbd bits (mostly) with
>>> 1 keyboard interface using the usb boot kbd interface (so that it will
>>> also work inside the BIOS) and a second interface for multimedia-keys +
>>> the mouse emulation of the thinkpad 10 touchpad, those are interfaces
>>> 1 and 2, except that they removed the mouse emulation as they added a
>>> new proper multi-touch capable touchpad as interface 3; and that one
>>> also handles the pointing stick I believe.
>>>
>>> So yes 2 bound to hid-generic, 1 bound to hid-multitouch seems to be
>>> correct.
>>
>> Right, that's what I observe.
>>
>>>>> If this works some of your Fn + F# keys will now hopefully start doing
>>>>> something, you can play around with modifying lenovo_input_mapping_tp10_ultrabook_kbd
>>>>> to make it do the right thing for your kbd.
>>>>> z
>>>>> ###
>>>>>
>>>>> About LED support, just enabling the LED support bits for the
>>>>> USB_DEVICE_ID_LENOVO_TP10UBKBD handling for now might work fine,
>>>>> but there is a tiny chance that sending the wrong command somehow puts
>>>>> the kbd in firmware update mode, I had that happen once with a Logitech
>>>>> kbd which did not seem to have any kind of handshake / passcode to avoid
>>>>> accidental fw updates (*).
>>>>>
>>>>> If you can give me a dump of the hid-descriptors for your keyboard,
>>>>> then I can check if that the LEDs might work the same way too (or not).
>>>>>
>>>>> The easiest way to get a dump is to run the following command as root:
>>>>>
>>>>> cat /sys/kernel/debug/hid/0003:17EF:60A3.*/rdesc > rdesc
>>>>>
>>>>> And then attach rdesc to your next email.
>>>>
>>>> Please find this one attached already now.
>>>> In case it helps, the * expands to 0057 0058 0059 on my system.
>>>
>>> Ok, so there still is an output-report number 9 on the second interface,
>>> which probably still controls the LEDS but its descriptors are subtly
>>> different. Although different in a good way I guess because the thinkpad
>>> 10 dock descriptor describes the 2 bytes in the output report as being
>>> in the range of 0-1 which is not how they are actually used.
>>>
>>> So I think that the code for the Thinkpad 10 ultrabook keyboard as
>>> Lenovo calls it, should also work on the X1 tablet thin keyboard.
>>
>> Mostly, modulo some key mappings, as expected.
>>
>> The good:
>>
>> LEDs are working exactly as expected with your patch, with the appropriate triggers automatically active. Perfect!
> 
> Good :)
> 
>> The bad:
>>
>> I could adjust some of the key mappings for the X1 Tablet 2nd keyboard. What I couldn't do is to get Fn+F10, Fn+F11, Fn+F12 and Fn+PrtSc to work.
>> Following the logic of /dev/hidraw1 capture (attached), those should be on usage_index 16 to 19. But apparently those are on a different usage page or something like that? Unfortunately, my RTFM skills didn't really help with figuring out how that's supposed to work.
>> (Is looking at the bit indices in /dev/hidraw traces how you figure out those mappings? If there's a better way, I'm eager to be told...)
> 
> You are swapping to low and high byte of the 3 data bytes in the report. Here is an annotated part of the descriptors to explain better:
> 
>   INPUT(3)[INPUT]
>     Field(0)
>       Application(Consumer.0001)
>       Usage(24)
>  0        Consumer.0001 F10 00000001 00000000 00000000
>  1        Consumer.0001 F11 00000010 00000000 00000000
>  2        Consumer.0001 F12 00000100 00000000 00000000
>  3        Consumer.0001 Prt 00001000 00000000 00000000
>  4        Consumer.0001
>  5        Consumer.00e2 F1  00100000 00000000 00000000
>  6        Consumer.0001
>  7        Consumer.0001
>  8        Consumer.0001 ESC 00000000 00000001 00000000
>  9        Consumer.0001 F4  00000000 00000010 00000000
> 10        Consumer.0001 F9  00000000 00000100 00000000
> 11        Consumer.00b7
> 12        Consumer.0001
> 13        Consumer.0001 F7  00000000 00100000 00000000
> 14        Consumer.0001 F8  00000000 01000000 00000000
> 15        Consumer.0001 
> 16        Consumer.0001
> 17        Consumer.0001
> 18        Consumer.0001 
> 19        Consumer.006f F6  00000000 00000000 00001000
> 20        Consumer.00ea F2  00000000 00000000 00010000
> 21        Consumer.00e9 F3  00000000 00000000 00100000
> 22        Consumer.0070 F5  00000000 00000000 01000000
> 23        Consumer.0001
>       Logical Minimum(0)
>       Logical Maximum(1)
>       Report Size(1)
>       Report Count(24)
>       Report Offset(0)
>       Flags( Variable Absolute )
> 
> Notice how the keys with standard codes (which work without mapping)
> F1 - F3, F5, F6 now all line up with _none_ Consumer.0001 entries.
> And if you check those codes in drivers/hid/hid-input.c around
> line 960 you will see the standard mappings line up too.
> 
> IOW your case 16 needs to be case 0, case 17, case 1, etc.

Gotcha, I suppose. At least on a shallow level... Thanks!
And, yes: that works perfectly.

>> Similarly - I assume - Fn+S should emit SysRq according to https://download.lenovo.com/pccbbs/mobiles_pdf/x1_tablet_gen_2_ug_en.pdf, page 51. This is not on the "consumer control" device, but the usual keyboard, so /dev/hidraw0. Again, couldn't get much further than producing a capture. But I cannot make sense of this one, because way more bits are set, so I cannot extrapolate from your code.
> 
> The input report used by the Fn + key "media keys" use a 24 bit report
> with 1 bit per key. The standard keyboard interface uses 1 byte per
> pressed key (with a maximum of 6 pressed keys) where the full byte
> encodes the scancode of the key. Normally SysRq is 0x46 but for some
> reason your keyboard is sending 0x9a you can map this by adding the following
> to the mapping function:
> 
> 	if (usage->hid == (HID_UP_KEYBOARD | 0x009a))
> 		map_key_clear(KEY_SYSRQ);

And the same here, I think. Works with return 1; after the map_key_clear, see the attached function.

> Likewise for the sleep combo:
> 
> 	if (usage->hid == (HID_UP_KEYBOARD | 0x0072))
> 		map_key_clear(KEY_SLEEP);

This seems unnecessary, as the sleep combo already emits KEY_SLEEP. Which I don't quite get, cause - if I learned correctly how to read the rdesc - sleep should be on 0x0072 should emit F23 (and 0x0078 sleep), but the key produces the 0x0072 pattern according to hidraw.
By the way, the Fn+4 for sleep also works in hid-generic, and also just once, see below.

> ###
> 
> Note chances are you have more Fn + 'letter' combinations at least on
> the thinkpad10 kbd I have:
> 
> Fn + T -> SysRq
> Fn + I -> Insert
> Fn + P -> Pause
> Fn + S -> Sysrq
> Fn + K -> ScrollLock
> Fn + B -> Pause
> 
> Note these do not need any special mappings on the thinkpad10 kbd and I guess
> the doubles may have something to do with some non qwerty keymaps.

Not the same on this keyboard. I have Fn + {B,K,P,S,End,4} for {Break,ScrLk,Pause,SysRq,Insert,Sleep}, but only SysRq was missing; the others are available on the "default" device, both with hid-lenovo and hid-generic.

>> The ugly:
>>
>> Fn+4 ("sleep") triggers the appropriate ACPI event button/sleep and emits something on /dev/hidraw0, too, but *only once*. After resuming, no reaction at all (neither on ACPI nor hidraw) until I unload and reload the hid_lenovo module.
> 
> As for the sleep key working only once, what happens after a suspend/resume ?

Nothing. At least nothing I can measure (via evtest, libinput debug-events, cat /dev/hidraw*, dmesg, journal, acpi_listen). The key simply doesn't react anymore.
Irrespective on how I wake the device. Even irrespective of whether I actually suspend the device or disable the sleep handler altogether, e.g. by systemd-inhibit. In only see the KEY_SLEEP press (no release!) event once until I reload the module or unplug and reattach the keyboard.
Very strange. And, by the way: the same for hid-generic.

> I think the key may have some special handling to avoid it sending
> a second KEY_SLEEP when the user uses it to wakeup the system, to
> avoid the system immediately going to sleep again when the user tries
> to wake the system this way.

Yes, that'd make sense. So probably the handler should restore something; apparently, that part is initialized when the module is loaded, so it's not just the keyboard firmware.

>> Finally, keyboard backlight is handled in firmware, apparently; Fn+Space is visible on /dev/hidraw1 (see attached capture), but it toggles the backlight levels without any userspace code involved, as far as I can see.
> 
> Yes I saw this in your dump, this is really weird because it sets 3 bits at once in the INPUT(3) report.
> Does it always set the same 3 bits independent of the brightness level ?

Yes; the four key presses are actually one full cycle over off, auto, dim, and bright. hidraw dumps are identical.

>> Also, the keyboard backlight doesn't create an entry in /sys/class/backlight or the like, so neither read nor write access. Out-of-the-box, at least. But I'm not even sure if this is possible in Windows.
> 
> Actuallu kbd-backlighting used the /sys/class/leds interface, but yeah that is not support by Linux ATM for this kbd.
> 
> Yeah, adding support for that (assuming the hw can do it) would definitely require making some USB dumps under Windows
> (after finding sw which can change it from within the OS under Windows).

I double-checked; somewhat unsurprisingly, there is at least a notification client on Windows that displays the new configuration after Fn+Space. The client only reports the setting; not sure if it would be technically feasible to also set the brightness level in software.


But anyways: I feel that I exploited your generosity in helping me far enough. I hope I wasn't just fed, but learned a bit how to fish for myself in the future. So, unless you're really committed to walk me through this further, I won't beg any further.

And in case you're looking for problems to tackle on the Tablet 2nd Gen, this one about the power button could be way more significant - but not sure if it's in an area that you are familiar in:
https://bbs.archlinux.org/viewtopic.php?id=248857
https://bugzilla.kernel.org/show_bug.cgi?id=204763


>> Bottom line: this is mostly usable already, modulo the adjustments for the different keys. I'd like to make F10 to F12 work before it hits testing; everything else is icing on the cake, I suppose. Do you have an hint for me how I can approach that?
> 
> See above, I think we are pretty close to solving this.

Attached is the modified version of the input mapping for this keyboard, working subject to the above mentioned restrictions. I think this is fine.
As you did all the real work, I feel this should be your contribution. But of course I can also prepare a patch on top of yours.

> Note in the mean time I've posted a hid-lenovo patch-series with various improvements related to
> the LED handling. I'll send you an offlist mail with the latest patches so that you can base any work you do on top of those.

Didn't have a look yet, but will do.

>> Also, I'd make sure that this is about the "ThinkPad X1 Tablet Thin Keyboard **Gen 2**". The consumer functions are different for the **Gen 1** keyboard, so I would also adjust the function names. I do have an old Gen 1 keyboard lying around, but unfortunately it's either broken (it lights up shortly after attaching in Windows, but doesn't report keypresses at all, and pretends to be completely dead in Linux), or it's incompatible with my X1 Tablet 2nd Gen. So I cannot test how your patch might impact the Gen1, too... :-/
> 
> I would expect the Gen1 to have a different product-id, so my patch shouldn't do anything.

Right. In this light, perhaps the function should still be called lenovo_input_mapping_x1_tab2_kbd (note the "2")?

>>> I've prepared a set of patches which enable the tp10ubkbd code on
>>> the X1 tablet thin keyboard. But beware as mentioned before there is a
>>> tiny chance that sending the wrong command somehow puts the kbd in
>>> firmware update mode. I believe that trying the tp10ubkbd code is safe,
>>> esp. since this is using a 2 byte large output report and using that
>>> for fw-updating would be a bit weird. Still there is a small risk
>>> (there always is when poking hw) so I will leave it up to you if
>>> you are willing to try this.
>>
>> No issue at all, and everything below works just as expected.
> 
> Good.
> 
> Regards,
> 
> Hans


Thanks a lot,
Alex
Hans de Goede Feb. 24, 2021, 9 p.m. UTC | #17
Hi,

On 2/22/21 1:31 PM, Alexander Kobel wrote:
> On 2/21/21 5:30 PM, Hans de Goede wrote:

<snip>

>>> I could adjust some of the key mappings for the X1 Tablet 2nd keyboard. What I couldn't do is to get Fn+F10, Fn+F11, Fn+F12 and Fn+PrtSc to work.
>>> Following the logic of /dev/hidraw1 capture (attached), those should be on usage_index 16 to 19. But apparently those are on a different usage page or something like that? Unfortunately, my RTFM skills didn't really help with figuring out how that's supposed to work.
>>> (Is looking at the bit indices in /dev/hidraw traces how you figure out those mappings? If there's a better way, I'm eager to be told...)
>>
>> You are swapping to low and high byte of the 3 data bytes in the report. Here is an annotated part of the descriptors to explain better:
>>
>>   INPUT(3)[INPUT]
>>     Field(0)
>>       Application(Consumer.0001)
>>       Usage(24)
>>  0        Consumer.0001 F10 00000001 00000000 00000000
>>  1        Consumer.0001 F11 00000010 00000000 00000000
>>  2        Consumer.0001 F12 00000100 00000000 00000000
>>  3        Consumer.0001 Prt 00001000 00000000 00000000
>>  4        Consumer.0001
>>  5        Consumer.00e2 F1  00100000 00000000 00000000
>>  6        Consumer.0001
>>  7        Consumer.0001
>>  8        Consumer.0001 ESC 00000000 00000001 00000000
>>  9        Consumer.0001 F4  00000000 00000010 00000000
>> 10        Consumer.0001 F9  00000000 00000100 00000000
>> 11        Consumer.00b7
>> 12        Consumer.0001
>> 13        Consumer.0001 F7  00000000 00100000 00000000
>> 14        Consumer.0001 F8  00000000 01000000 00000000
>> 15        Consumer.0001 
>> 16        Consumer.0001
>> 17        Consumer.0001
>> 18        Consumer.0001 
>> 19        Consumer.006f F6  00000000 00000000 00001000
>> 20        Consumer.00ea F2  00000000 00000000 00010000
>> 21        Consumer.00e9 F3  00000000 00000000 00100000
>> 22        Consumer.0070 F5  00000000 00000000 01000000
>> 23        Consumer.0001
>>       Logical Minimum(0)
>>       Logical Maximum(1)
>>       Report Size(1)
>>       Report Count(24)
>>       Report Offset(0)
>>       Flags( Variable Absolute )
>>
>> Notice how the keys with standard codes (which work without mapping)
>> F1 - F3, F5, F6 now all line up with _none_ Consumer.0001 entries.
>> And if you check those codes in drivers/hid/hid-input.c around
>> line 960 you will see the standard mappings line up too.
>>
>> IOW your case 16 needs to be case 0, case 17, case 1, etc.
> 
> Gotcha, I suppose. At least on a shallow level... Thanks!
> And, yes: that works perfectly.
> 
>>> Similarly - I assume - Fn+S should emit SysRq according to https://download.lenovo.com/pccbbs/mobiles_pdf/x1_tablet_gen_2_ug_en.pdf, page 51. This is not on the "consumer control" device, but the usual keyboard, so /dev/hidraw0. Again, couldn't get much further than producing a capture. But I cannot make sense of this one, because way more bits are set, so I cannot extrapolate from your code.
>>
>> The input report used by the Fn + key "media keys" use a 24 bit report
>> with 1 bit per key. The standard keyboard interface uses 1 byte per
>> pressed key (with a maximum of 6 pressed keys) where the full byte
>> encodes the scancode of the key. Normally SysRq is 0x46 but for some
>> reason your keyboard is sending 0x9a you can map this by adding the following
>> to the mapping function:
>>
>> 	if (usage->hid == (HID_UP_KEYBOARD | 0x009a))
>> 		map_key_clear(KEY_SYSRQ);
> 
> And the same here, I think. Works with return 1; after the map_key_clear, see the attached function.

Ah yes, I forgot the return 1, sorry about that.

> 
>> Likewise for the sleep combo:
>>
>> 	if (usage->hid == (HID_UP_KEYBOARD | 0x0072))
>> 		map_key_clear(KEY_SLEEP);
> 
> This seems unnecessary, as the sleep combo already emits KEY_SLEEP. Which I don't quite get, cause - if I learned correctly how to read the rdesc - sleep should be on 0x0072 should emit F23 (and 0x0078 sleep), but the key produces the 0x0072 pattern according to hidraw.
> By the way, the Fn+4 for sleep also works in hid-generic, and also just once, see below.

Ok.

>> ###
>>
>> Note chances are you have more Fn + 'letter' combinations at least on
>> the thinkpad10 kbd I have:
>>
>> Fn + T -> SysRq
>> Fn + I -> Insert
>> Fn + P -> Pause
>> Fn + S -> Sysrq
>> Fn + K -> ScrollLock
>> Fn + B -> Pause
>>
>> Note these do not need any special mappings on the thinkpad10 kbd and I guess
>> the doubles may have something to do with some non qwerty keymaps.
> 
> Not the same on this keyboard. I have Fn + {B,K,P,S,End,4} for {Break,ScrLk,Pause,SysRq,Insert,Sleep}, but only SysRq was missing; the others are available on the "default" device, both with hid-lenovo and hid-generic.

Ok.

>>> The ugly:
>>>
>>> Fn+4 ("sleep") triggers the appropriate ACPI event button/sleep and emits something on /dev/hidraw0, too, but *only once*. After resuming, no reaction at all (neither on ACPI nor hidraw) until I unload and reload the hid_lenovo module.
>>
>> As for the sleep key working only once, what happens after a suspend/resume ?
> 
> Nothing. At least nothing I can measure (via evtest, libinput debug-events, cat /dev/hidraw*, dmesg, journal, acpi_listen). The key simply doesn't react anymore.
> Irrespective on how I wake the device. Even irrespective of whether I actually suspend the device or disable the sleep handler altogether, e.g. by systemd-inhibit. In only see the KEY_SLEEP press (no release!) event once until I reload the module or unplug and reattach the keyboard.
> Very strange. And, by the way: the same for hid-generic.
> 
>> I think the key may have some special handling to avoid it sending
>> a second KEY_SLEEP when the user uses it to wakeup the system, to
>> avoid the system immediately going to sleep again when the user tries
>> to wake the system this way.
> 
> Yes, that'd make sense. So probably the handler should restore something; apparently, that part is initialized when the module is loaded, so it's not just the keyboard firmware.

Maybe echo-ing to the fnlock attribute resets the key ? The driver does always force the fnlock LED on when it is loaded.

>>> Finally, keyboard backlight is handled in firmware, apparently; Fn+Space is visible on /dev/hidraw1 (see attached capture), but it toggles the backlight levels without any userspace code involved, as far as I can see.
>>
>> Yes I saw this in your dump, this is really weird because it sets 3 bits at once in the INPUT(3) report.
>> Does it always set the same 3 bits independent of the brightness level ?
> 
> Yes; the four key presses are actually one full cycle over off, auto, dim, and bright. hidraw dumps are identical.
> 
>>> Also, the keyboard backlight doesn't create an entry in /sys/class/backlight or the like, so neither read nor write access. Out-of-the-box, at least. But I'm not even sure if this is possible in Windows.
>>
>> Actuallu kbd-backlighting used the /sys/class/leds interface, but yeah that is not support by Linux ATM for this kbd.
>>
>> Yeah, adding support for that (assuming the hw can do it) would definitely require making some USB dumps under Windows
>> (after finding sw which can change it from within the OS under Windows).
> 
> I double-checked; somewhat unsurprisingly, there is at least a notification client on Windows that displays the new configuration after Fn+Space. The client only reports the setting; not sure if it would be technically feasible to also set the brightness level in software.

We could try asking Nitin if he has any info about this, but I agree that this is a low priority item.

> But anyways: I feel that I exploited your generosity in helping me far enough. I hope I wasn't just fed, but learned a bit how to fish for myself in the future. So, unless you're really committed to walk me through this further, I won't beg any further.
> 
> And in case you're looking for problems to tackle on the Tablet 2nd Gen, this one about the power button could be way more significant - but not sure if it's in an area that you are familiar in:
> https://bbs.archlinux.org/viewtopic.php?id=248857
> https://bugzilla.kernel.org/show_bug.cgi?id=204763

I see that you have already tested the patch which was posted for this, so I assume that this is resolved now ?

>>> Bottom line: this is mostly usable already, modulo the adjustments for the different keys. I'd like to make F10 to F12 work before it hits testing; everything else is icing on the cake, I suppose. Do you have an hint for me how I can approach that?
>>
>> See above, I think we are pretty close to solving this.
> 
> Attached is the modified version of the input mapping for this keyboard, working subject to the above mentioned restrictions. I think this is fine.
> As you did all the real work, I feel this should be your contribution. But of course I can also prepare a patch on top of yours.

I think you're underestimating your own contribution here...

For cases like this we usually add a co-authored tag. Since this applies on top of another hid-lenovo series which I recently send out it is probably easier if I upstream this, that I agree on.

I would like to attribute your work though, so I would like to suggest adding the following 2 tags to the commit msg for
the "HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard" patch:

Co-authored-by: Alexander Kobel <a-kobel@a-kobel.de>
Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>

Alternatively I could add:

Reported-and-tested-by: Alexander Kobel <a-kobel@a-kobel.de>

But I believe that co-authored-by + s-o-b are more appropriate.

If you can let me know which one you prefer, then I will drop in your fixed lenovo_input_mapping_x1_tab_kbd() function, remove the WIP from the commit subject and submit the last 2 patches of the set which I send you upstream (the rest was already submitted earlier).

>> Note in the mean time I've posted a hid-lenovo patch-series with various improvements related to
>> the LED handling. I'll send you an offlist mail with the latest patches so that you can base any work you do on top of those.
> 
> Didn't have a look yet, but will do.
> 
>>> Also, I'd make sure that this is about the "ThinkPad X1 Tablet Thin Keyboard **Gen 2**". The consumer functions are different for the **Gen 1** keyboard, so I would also adjust the function names. I do have an old Gen 1 keyboard lying around, but unfortunately it's either broken (it lights up shortly after attaching in Windows, but doesn't report keypresses at all, and pretends to be completely dead in Linux), or it's incompatible with my X1 Tablet 2nd Gen. So I cannot test how your patch might impact the Gen1, too... :-/
>>
>> I would expect the Gen1 to have a different product-id, so my patch shouldn't do anything.
> 
> Right. In this light, perhaps the function should still be called lenovo_input_mapping_x1_tab2_kbd (note the "2")?

Well drivers/hid/hid-ids.h has this:

#define USB_DEVICE_ID_LENOVO_X1_COVER   0x6085
#define USB_DEVICE_ID_LENOVO_X1_TAB     0x60a3
#define USB_DEVICE_ID_LENOVO_X1_TAB3    0x60b5

And I guess that the COVER might be the X1 gen1 product-id ?

Your working kbd is using the USB_DEVICE_ID_LENOVO_X1_TAB id. note not TAB2 just tab.

I don't know, but it is not all that important really, we can always rename both the #define USB_DEVICE_ID_LENOVO_X1_TAB and the function later if there is a reason to do so.

> Thanks a lot,

You're welcome and thank you for helping with improving support for Linux on these devices.

Regards,

Hans
Alexander Kobel Feb. 24, 2021, 9:59 p.m. UTC | #18
Hi,

On 2/24/21 10:00 PM, Hans de Goede wrote:
> Hi,
> 
> On 2/22/21 1:31 PM, Alexander Kobel wrote:
>> On 2/21/21 5:30 PM, Hans de Goede wrote:

<snip>

>>> The input report used by the Fn + key "media keys" use a 24 bit report
>>> with 1 bit per key. The standard keyboard interface uses 1 byte per
>>> pressed key (with a maximum of 6 pressed keys) where the full byte
>>> encodes the scancode of the key. Normally SysRq is 0x46 but for some
>>> reason your keyboard is sending 0x9a you can map this by adding the following
>>> to the mapping function:
>>>
>>> 	if (usage->hid == (HID_UP_KEYBOARD | 0x009a))
>>> 		map_key_clear(KEY_SYSRQ);
>>
>> And the same here, I think. Works with return 1; after the map_key_clear, see the attached function.
> 
> Ah yes, I forgot the return 1, sorry about that.

No problem, nearby pattern matching and copy-paste is a great way to learn sometimes. ;-)

>>> Likewise for the sleep combo:
>>>
>>> 	if (usage->hid == (HID_UP_KEYBOARD | 0x0072))
>>> 		map_key_clear(KEY_SLEEP);
>>
>> This seems unnecessary, as the sleep combo already emits KEY_SLEEP. Which I don't quite get, cause - if I learned correctly how to read the rdesc - sleep should be on 0x0072 should emit F23 (and 0x0078 sleep), but the key produces the 0x0072 pattern according to hidraw.
>> By the way, the Fn+4 for sleep also works in hid-generic, and also just once, see below.
> 
> Ok.
> 
>>> ###
>>>
>>> Note chances are you have more Fn + 'letter' combinations at least on
>>> the thinkpad10 kbd I have:
>>>
>>> Fn + T -> SysRq
>>> Fn + I -> Insert
>>> Fn + P -> Pause
>>> Fn + S -> Sysrq
>>> Fn + K -> ScrollLock
>>> Fn + B -> Pause
>>>
>>> Note these do not need any special mappings on the thinkpad10 kbd and I guess
>>> the doubles may have something to do with some non qwerty keymaps.
>>
>> Not the same on this keyboard. I have Fn + {B,K,P,S,End,4} for {Break,ScrLk,Pause,SysRq,Insert,Sleep}, but only SysRq was missing; the others are available on the "default" device, both with hid-lenovo and hid-generic.
> 
> Ok.
> 
>>>> The ugly:
>>>>
>>>> Fn+4 ("sleep") triggers the appropriate ACPI event button/sleep and emits something on /dev/hidraw0, too, but *only once*. After resuming, no reaction at all (neither on ACPI nor hidraw) until I unload and reload the hid_lenovo module.
>>>
>>> As for the sleep key working only once, what happens after a suspend/resume ?
>>
>> Nothing. At least nothing I can measure (via evtest, libinput debug-events, cat /dev/hidraw*, dmesg, journal, acpi_listen). The key simply doesn't react anymore.
>> Irrespective on how I wake the device. Even irrespective of whether I actually suspend the device or disable the sleep handler altogether, e.g. by systemd-inhibit. In only see the KEY_SLEEP press (no release!) event once until I reload the module or unplug and reattach the keyboard.
>> Very strange. And, by the way: the same for hid-generic.
>>
>>> I think the key may have some special handling to avoid it sending
>>> a second KEY_SLEEP when the user uses it to wakeup the system, to
>>> avoid the system immediately going to sleep again when the user tries
>>> to wake the system this way.
>>
>> Yes, that'd make sense. So probably the handler should restore something; apparently, that part is initialized when the module is loaded, so it's not just the keyboard firmware.
> 
> Maybe echo-ing to the fnlock attribute resets the key ? The driver does always force the fnlock LED on when it is loaded.

Good catch, but that doesn't help, either.

It's only a minor nuisance, though; in any case, it can be worked around by reloading the driver in a resume hook. If one actually wants to use that button; I personally won't.

>>>> Finally, keyboard backlight is handled in firmware, apparently; Fn+Space is visible on /dev/hidraw1 (see attached capture), but it toggles the backlight levels without any userspace code involved, as far as I can see.
>>>
>>> Yes I saw this in your dump, this is really weird because it sets 3 bits at once in the INPUT(3) report.
>>> Does it always set the same 3 bits independent of the brightness level ?
>>
>> Yes; the four key presses are actually one full cycle over off, auto, dim, and bright. hidraw dumps are identical.
>>
>>>> Also, the keyboard backlight doesn't create an entry in /sys/class/backlight or the like, so neither read nor write access. Out-of-the-box, at least. But I'm not even sure if this is possible in Windows.
>>>
>>> Actuallu kbd-backlighting used the /sys/class/leds interface, but yeah that is not support by Linux ATM for this kbd.
>>>
>>> Yeah, adding support for that (assuming the hw can do it) would definitely require making some USB dumps under Windows
>>> (after finding sw which can change it from within the OS under Windows).
>>
>> I double-checked; somewhat unsurprisingly, there is at least a notification client on Windows that displays the new configuration after Fn+Space. The client only reports the setting; not sure if it would be technically feasible to also set the brightness level in software.
> 
> We could try asking Nitin if he has any info about this, but I agree that this is a low priority item.

Ack.

>> But anyways: I feel that I exploited your generosity in helping me far enough. I hope I wasn't just fed, but learned a bit how to fish for myself in the future. So, unless you're really committed to walk me through this further, I won't beg any further.
>>
>> And in case you're looking for problems to tackle on the Tablet 2nd Gen, this one about the power button could be way more significant - but not sure if it's in an area that you are familiar in:
>> https://bbs.archlinux.org/viewtopic.php?id=248857
>> https://bugzilla.kernel.org/show_bug.cgi?id=204763
> 
> I see that you have already tested the patch which was posted for this, so I assume that this is resolved now ?

Correct. I resurrected the bugzilla task shortly after my last mail, and Alban cranked out a patch with you in CC within few hours. Didn't want to add more noise here.

>>>> Bottom line: this is mostly usable already, modulo the adjustments for the different keys. I'd like to make F10 to F12 work before it hits testing; everything else is icing on the cake, I suppose. Do you have an hint for me how I can approach that?
>>>
>>> See above, I think we are pretty close to solving this.
>>
>> Attached is the modified version of the input mapping for this keyboard, working subject to the above mentioned restrictions. I think this is fine.
>> As you did all the real work, I feel this should be your contribution. But of course I can also prepare a patch on top of yours.
> 
> I think you're underestimating your own contribution here...
> 
> For cases like this we usually add a co-authored tag. Since this applies on top of another hid-lenovo series which I recently send out it is probably easier if I upstream this, that I agree on.
> 
> I would like to attribute your work though, so I would like to suggest adding the following 2 tags to the commit msg for
> the "HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard" patch:
> 
> Co-authored-by: Alexander Kobel <a-kobel@a-kobel.de>
> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
> 
> Alternatively

<snip>

> But I believe that co-authored-by + s-o-b are more appropriate.

Okay, so be it. I trust your opinion here.


> If you can let me know which one you prefer, then I will drop in your fixed lenovo_input_mapping_x1_tab_kbd() function, remove the WIP from the commit subject and submit the last 2 patches of the set which I send you upstream (the rest was already submitted earlier).

co-authored-by + s-o-b it is then. Ack for the rest.

>>> Note in the mean time I've posted a hid-lenovo patch-series with various improvements related to
>>> the LED handling. I'll send you an offlist mail with the latest patches so that you can base any work you do on top of those.
>>
>> Didn't have a look yet, but will do.
>>
>>>> Also, I'd make sure that this is about the "ThinkPad X1 Tablet Thin Keyboard **Gen 2**". The consumer functions are different for the **Gen 1** keyboard, so I would also adjust the function names. I do have an old Gen 1 keyboard lying around, but unfortunately it's either broken (it lights up shortly after attaching in Windows, but doesn't report keypresses at all, and pretends to be completely dead in Linux), or it's incompatible with my X1 Tablet 2nd Gen. So I cannot test how your patch might impact the Gen1, too... :-/
>>>
>>> I would expect the Gen1 to have a different product-id, so my patch shouldn't do anything.
>>
>> Right. In this light, perhaps the function should still be called lenovo_input_mapping_x1_tab2_kbd (note the "2")?
> 
> Well drivers/hid/hid-ids.h has this:
> 
> #define USB_DEVICE_ID_LENOVO_X1_COVER   0x6085
> #define USB_DEVICE_ID_LENOVO_X1_TAB     0x60a3
> #define USB_DEVICE_ID_LENOVO_X1_TAB3    0x60b5
> 
> And I guess that the COVER might be the X1 gen1 product-id ?

AFAICS, yes; confirmed by a quick web search. So it'd be more apt to

#define USB_DEVICE_ID_LENOVO_X1_TAB     0x6085

#define USB_DEVICE_ID_LENOVO_X1_TAB2    0x60a3

#define USB_DEVICE_ID_LENOVO_X1_TAB3    0x60b5


but I have no way to really confirm right now, so I'd also be in favor to leave it as-is.

> Your working kbd is using the USB_DEVICE_ID_LENOVO_X1_TAB id. note not TAB2 just tab.
> 
> I don't know, but it is not all that important really, we can always rename both the #define USB_DEVICE_ID_LENOVO_X1_TAB and the function later if there is a reason to do so.

Ack. Better have #defines and function names in sync and think about more interesting stuff. ;-)


>> Thanks a lot,
> 
> You're welcome and thank you for helping with improving support for Linux on these devices.

Ditto!


Cheers,
Alex
Hans de Goede March 4, 2021, 7:23 p.m. UTC | #19
Hi,

On 2/24/21 10:59 PM, Alexander Kobel wrote:
> Hi,
> 
> On 2/24/21 10:00 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 2/22/21 1:31 PM, Alexander Kobel wrote:
>>> On 2/21/21 5:30 PM, Hans de Goede wrote:
> 
> <snip>
> 
>>>> The input report used by the Fn + key "media keys" use a 24 bit report
>>>> with 1 bit per key. The standard keyboard interface uses 1 byte per
>>>> pressed key (with a maximum of 6 pressed keys) where the full byte
>>>> encodes the scancode of the key. Normally SysRq is 0x46 but for some
>>>> reason your keyboard is sending 0x9a you can map this by adding the following
>>>> to the mapping function:
>>>>
>>>> 	if (usage->hid == (HID_UP_KEYBOARD | 0x009a))
>>>> 		map_key_clear(KEY_SYSRQ);
>>>
>>> And the same here, I think. Works with return 1; after the map_key_clear, see the attached function.
>>
>> Ah yes, I forgot the return 1, sorry about that.
> 
> No problem, nearby pattern matching and copy-paste is a great way to learn sometimes. ;-)
> 
>>>> Likewise for the sleep combo:
>>>>
>>>> 	if (usage->hid == (HID_UP_KEYBOARD | 0x0072))
>>>> 		map_key_clear(KEY_SLEEP);
>>>
>>> This seems unnecessary, as the sleep combo already emits KEY_SLEEP. Which I don't quite get, cause - if I learned correctly how to read the rdesc - sleep should be on 0x0072 should emit F23 (and 0x0078 sleep), but the key produces the 0x0072 pattern according to hidraw.
>>> By the way, the Fn+4 for sleep also works in hid-generic, and also just once, see below.
>>
>> Ok.
>>
>>>> ###
>>>>
>>>> Note chances are you have more Fn + 'letter' combinations at least on
>>>> the thinkpad10 kbd I have:
>>>>
>>>> Fn + T -> SysRq
>>>> Fn + I -> Insert
>>>> Fn + P -> Pause
>>>> Fn + S -> Sysrq
>>>> Fn + K -> ScrollLock
>>>> Fn + B -> Pause
>>>>
>>>> Note these do not need any special mappings on the thinkpad10 kbd and I guess
>>>> the doubles may have something to do with some non qwerty keymaps.
>>>
>>> Not the same on this keyboard. I have Fn + {B,K,P,S,End,4} for {Break,ScrLk,Pause,SysRq,Insert,Sleep}, but only SysRq was missing; the others are available on the "default" device, both with hid-lenovo and hid-generic.
>>
>> Ok.
>>
>>>>> The ugly:
>>>>>
>>>>> Fn+4 ("sleep") triggers the appropriate ACPI event button/sleep and emits something on /dev/hidraw0, too, but *only once*. After resuming, no reaction at all (neither on ACPI nor hidraw) until I unload and reload the hid_lenovo module.
>>>>
>>>> As for the sleep key working only once, what happens after a suspend/resume ?
>>>
>>> Nothing. At least nothing I can measure (via evtest, libinput debug-events, cat /dev/hidraw*, dmesg, journal, acpi_listen). The key simply doesn't react anymore.
>>> Irrespective on how I wake the device. Even irrespective of whether I actually suspend the device or disable the sleep handler altogether, e.g. by systemd-inhibit. In only see the KEY_SLEEP press (no release!) event once until I reload the module or unplug and reattach the keyboard.
>>> Very strange. And, by the way: the same for hid-generic.
>>>
>>>> I think the key may have some special handling to avoid it sending
>>>> a second KEY_SLEEP when the user uses it to wakeup the system, to
>>>> avoid the system immediately going to sleep again when the user tries
>>>> to wake the system this way.
>>>
>>> Yes, that'd make sense. So probably the handler should restore something; apparently, that part is initialized when the module is loaded, so it's not just the keyboard firmware.
>>
>> Maybe echo-ing to the fnlock attribute resets the key ? The driver does always force the fnlock LED on when it is loaded.
> 
> Good catch, but that doesn't help, either.
> 
> It's only a minor nuisance, though; in any case, it can be worked around by reloading the driver in a resume hook. If one actually wants to use that button; I personally won't.
> 
>>>>> Finally, keyboard backlight is handled in firmware, apparently; Fn+Space is visible on /dev/hidraw1 (see attached capture), but it toggles the backlight levels without any userspace code involved, as far as I can see.
>>>>
>>>> Yes I saw this in your dump, this is really weird because it sets 3 bits at once in the INPUT(3) report.
>>>> Does it always set the same 3 bits independent of the brightness level ?
>>>
>>> Yes; the four key presses are actually one full cycle over off, auto, dim, and bright. hidraw dumps are identical.
>>>
>>>>> Also, the keyboard backlight doesn't create an entry in /sys/class/backlight or the like, so neither read nor write access. Out-of-the-box, at least. But I'm not even sure if this is possible in Windows.
>>>>
>>>> Actuallu kbd-backlighting used the /sys/class/leds interface, but yeah that is not support by Linux ATM for this kbd.
>>>>
>>>> Yeah, adding support for that (assuming the hw can do it) would definitely require making some USB dumps under Windows
>>>> (after finding sw which can change it from within the OS under Windows).
>>>
>>> I double-checked; somewhat unsurprisingly, there is at least a notification client on Windows that displays the new configuration after Fn+Space. The client only reports the setting; not sure if it would be technically feasible to also set the brightness level in software.
>>
>> We could try asking Nitin if he has any info about this, but I agree that this is a low priority item.
> 
> Ack.
> 
>>> But anyways: I feel that I exploited your generosity in helping me far enough. I hope I wasn't just fed, but learned a bit how to fish for myself in the future. So, unless you're really committed to walk me through this further, I won't beg any further.
>>>
>>> And in case you're looking for problems to tackle on the Tablet 2nd Gen, this one about the power button could be way more significant - but not sure if it's in an area that you are familiar in:
>>> https://bbs.archlinux.org/viewtopic.php?id=248857
>>> https://bugzilla.kernel.org/show_bug.cgi?id=204763
>>
>> I see that you have already tested the patch which was posted for this, so I assume that this is resolved now ?
> 
> Correct. I resurrected the bugzilla task shortly after my last mail, and Alban cranked out a patch with you in CC within few hours. Didn't want to add more noise here.
> 
>>>>> Bottom line: this is mostly usable already, modulo the adjustments for the different keys. I'd like to make F10 to F12 work before it hits testing; everything else is icing on the cake, I suppose. Do you have an hint for me how I can approach that?
>>>>
>>>> See above, I think we are pretty close to solving this.
>>>
>>> Attached is the modified version of the input mapping for this keyboard, working subject to the above mentioned restrictions. I think this is fine.
>>> As you did all the real work, I feel this should be your contribution. But of course I can also prepare a patch on top of yours.
>>
>> I think you're underestimating your own contribution here...
>>
>> For cases like this we usually add a co-authored tag. Since this applies on top of another hid-lenovo series which I recently send out it is probably easier if I upstream this, that I agree on.
>>
>> I would like to attribute your work though, so I would like to suggest adding the following 2 tags to the commit msg for
>> the "HID: lenovo: Add support for Thinkpad X1 Tablet Thin keyboard" patch:
>>
>> Co-authored-by: Alexander Kobel <a-kobel@a-kobel.de>
>> Signed-off-by: Alexander Kobel <a-kobel@a-kobel.de>
>>
>> Alternatively
> 
> <snip>
> 
>> But I believe that co-authored-by + s-o-b are more appropriate.
> 
> Okay, so be it. I trust your opinion here.

Great, I've submitted a new version of my previous hid-lenovo series upstream now, with the 2 patches to add support for the X1 tablet keyboard added to the series.

Regards,

Hans
diff mbox series

Patch

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index c404706379d9..fd5322b5bbbd 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -174,6 +174,8 @@  enum tpacpi_hkey_event_t {
                                                     or port replicator */
        TP_HKEY_EV_HOTPLUG_UNDOCK       = 0x4011, /* undocked from hotplug
                                                     dock or port replicator */
+       TP_HKEY_EV_KBD_COVER_ATTACH     = 0x4012, /* attached keyboard cover */
+       TP_HKEY_EV_KBD_COVER_DETACH     = 0x4013, /* detached keyboard cover */

        /* User-interface events */
        TP_HKEY_EV_LID_CLOSE            = 0x5001, /* laptop lid closed */
@@ -3989,6 +3991,12 @@  static bool hotkey_notify_dockevent(const u32 hkey,
        case TP_HKEY_EV_HOTPLUG_UNDOCK: /* undocked from port replicator */
                pr_info("undocked from hotplug port replicator\n");
                return true;
+       case TP_HKEY_EV_KBD_COVER_ATTACH:
+               pr_info("attached keyboard cover\n");
+               return true;
+       case TP_HKEY_EV_KBD_COVER_DETACH:
+               pr_info("detached keyboard cover\n");
+               return true;

        default:
                return false;