Support for X1 tablet keyboard (was Re: [PATCH] platform/x86: thinkpad_acpi: handle HKEY 0x4012, 0x4013 events)

Hi,

On 2/7/21 6:55 PM, Alexander Kobel wrote:

Hi,

On 2/7/21 6:10 PM, Hans de Goede wrote:

quoted

Hi,

On 2/7/21 5:34 PM, Alexander Kobel wrote:

quoted

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.

quoted

quoted

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.

Hi,

On 2/8/21 11:17 AM, Hans de Goede wrote:

On 2/7/21 6:55 PM, Alexander Kobel wrote:

quoted

<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>

quoted

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.

Hi,

On 2/9/21 4:16 PM, Alexander Kobel wrote:

Hi,

On 2/8/21 11:17 AM, Hans de Goede wrote:

quoted

On 2/7/21 6:55 PM, Alexander Kobel wrote:

quoted

<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.

quoted

<snip>

quoted

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... ;-)

quoted

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.

quoted

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.

quoted

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.)

quoted

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.

quoted

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

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:

quoted

Hi,

On 2/8/21 11:17 AM, Hans de Goede wrote:

quoted

On 2/7/21 6:55 PM, Alexander Kobel wrote:

quoted

<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.

quoted

<snip>

quoted

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... ;-)

quoted

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.

quoted

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.

quoted

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.)

quoted

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.

quoted

quoted

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

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:

quoted

Hi,

On 2/9/21 4:16 PM, Alexander Kobel wrote:

quoted

Hi,

On 2/8/21 11:17 AM, Hans de Goede wrote:

quoted

On 2/7/21 6:55 PM, Alexander Kobel wrote:

quoted

<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.

quoted

<snip>

quoted

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... ;-)

quoted

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.

quoted

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.

quoted

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.)

quoted

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.

quoted

quoted

quoted

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.

quoted

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

Hi,

On 2/21/21 5:30 PM, Hans de Goede wrote:

Hi,

On 2/21/21 2:17 PM, Alexander Kobel wrote:

quoted

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:

quoted

Hi,

On 2/9/21 4:16 PM, Alexander Kobel wrote:

quoted

Hi,

On 2/8/21 11:17 AM, Hans de Goede wrote:

quoted

On 2/7/21 6:55 PM, Alexander Kobel wrote:

quoted

<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.

quoted

<snip>

quoted

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... ;-)

quoted

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.

quoted

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.

quoted

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.)

quoted

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.

quoted

quoted

quoted

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 :)

quoted

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.

quoted

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.

quoted

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.

quoted

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.

quoted

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

quoted

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.

quoted

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")?

quoted

quoted

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

Hi,

On 2/22/21 1:31 PM, Alexander Kobel wrote:

On 2/21/21 5:30 PM, Hans de Goede wrote:

<snip>

quoted

quoted

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.

quoted

quoted

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.

quoted

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.

quoted

###

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.

quoted

quoted

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.

quoted

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.

quoted

quoted

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.

quoted

quoted

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 ?

quoted

quoted

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 [off-list ref]
Signed-off-by: Alexander Kobel <redacted>

Alternatively I could add:

Reported-and-tested-by: Alexander Kobel <redacted>

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).

quoted

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.

quoted

quoted

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

Hi,

On 2/24/21 10:00 PM, Hans de Goede wrote:

Hi,

On 2/22/21 1:31 PM, Alexander Kobel wrote:

quoted

On 2/21/21 5:30 PM, Hans de Goede wrote:

<snip>

quoted

quoted

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. ;-)

quoted

quoted

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.

quoted

quoted

###

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.

quoted

quoted

quoted

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.

quoted

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.

quoted

quoted

quoted

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.

quoted

quoted

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.

quoted

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.

quoted

quoted

quoted

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 [off-list ref]
Signed-off-by: Alexander Kobel <redacted>

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.

quoted

quoted

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.

quoted

quoted

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. ;-)

quoted

Thanks a lot,

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

Ditto!


Cheers,
Alex

Hi,

On 2/24/21 10:59 PM, Alexander Kobel wrote:

Hi,

On 2/24/21 10:00 PM, Hans de Goede wrote:

quoted

Hi,

On 2/22/21 1:31 PM, Alexander Kobel wrote:

quoted

On 2/21/21 5:30 PM, Hans de Goede wrote:

<snip>

quoted

quoted

quoted

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. ;-)

quoted

quoted

quoted

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.

quoted

quoted

###

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.

quoted

quoted

quoted

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.

quoted

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.

quoted

quoted

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

quoted

quoted

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 [off-list ref]
Signed-off-by: Alexander Kobel <redacted>

Alternatively

<snip>

quoted

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