HID: lenovo: Restrict detection of patched firmware only to USB cptkbd

Commit 46a0a2c96f0f ("HID: lenovo: Detect quirk-free fw on cptkbd and
stop applying workaround") introduced a regression for ThinkPad
TrackPoint Keyboard II which has similar quirks to cptkbd (so it uses
the same workarounds) but slightly different so that there are
false-positives during detecting well-behaving firmware. This commit
restricts detecting well-behaving firmware to the only model which
known to have one and have stable enough quirks to not cause
false-positives.

Fixes: 46a0a2c96f0f ("HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround")
Link: https://lore.kernel.org/linux-input/ZXRiiPsBKNasioqH@jekhomev/
Link: https://bbs.archlinux.org/viewtopic.php?pid=2135468#p2135468
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
Tested-by: Yauhen Kharuzhy <jekhor@gmail.com>
---
 drivers/hid/hid-lenovo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

On Tue, 12 Dec 2023, Mikhail Khvainitski wrote:

> Commit 46a0a2c96f0f ("HID: lenovo: Detect quirk-free fw on cptkbd and
> stop applying workaround") introduced a regression for ThinkPad
> TrackPoint Keyboard II which has similar quirks to cptkbd (so it uses
> the same workarounds) but slightly different so that there are
> false-positives during detecting well-behaving firmware. This commit
> restricts detecting well-behaving firmware to the only model which
> known to have one and have stable enough quirks to not cause
> false-positives.
> 
> Fixes: 46a0a2c96f0f ("HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround")
> Link: https://lore.kernel.org/linux-input/ZXRiiPsBKNasioqH@jekhomev/
> Link: https://bbs.archlinux.org/viewtopic.php?pid=2135468#p2135468
> Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
> Tested-by: Yauhen Kharuzhy <jekhor@gmail.com>
> ---
>  drivers/hid/hid-lenovo.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 7c1b33be9d13..149a3c74346b 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -692,7 +692,8 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
>  		 * so set middlebutton_state to 3
>  		 * to never apply workaround anymore
>  		 */
> -		if (cptkbd_data->middlebutton_state == 1 &&
> +		if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD &&
> +				cptkbd_data->middlebutton_state == 1 &&
>  				usage->type == EV_REL &&
>  				(usage->code == REL_X || usage->code == REL_Y)) {
>  			cptkbd_data->middlebutton_state = 3;

Applied, thanks.

I get buggy middle button scrolling behavior on my USB Compact Keyboard 
(i.e., "1st gen") with original, unmodified firmware and the patch (of 
Sep. 23).

Sometimes the keyboard sends REL_X events while the middle button is 
pressed. Thus the old "workaround" is disabled and middle button 
scrolling henceforth exhibits the known buggy behavior.

Explicitly, I can confirm that the following values occur, leading to 
erroneous disabling of the workaround:

cptkbd_data->middlebutton_state == 1
usage->type == 2 [i.e., EV_REL]
usage->code == 0 [i.e., REL_X]

The keyboard is identified by lsusb as:
ID 17ef:6047 Lenovo ThinkPad Compact Keyboard with TrackPoint

This was tested with kernel 6.1.67 which contains the backported patch 
(commit 6e2076cad8873cc2a9f96e4becab35425c3656dc).

I didn't test the latest patch of Dec. 12. However, I don't expect it to 
fix my issue as the only added condition
hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
should be satisfied, which I understand is also the intention. 
(USB_DEVICE_ID_LENOVO_CUSBKBD == 0x6047, which is the device ID of my 
keyboard as reported by lsusb.)

Best regards,
Uli

Hello.

Thank you for the report, sorry about that. I've received one more
report of the same issue by email.
So this means that the only reliable way is to add a sysfs parameter.
I'll send a patch.


On Fri, 22 Dec 2023 at 17:32, Uli v. d. Ohe <u-v@mailbox.org> wrote:
>
> I get buggy middle button scrolling behavior on my USB Compact Keyboard
> (i.e., "1st gen") with original, unmodified firmware and the patch (of
> Sep. 23).
>
> Sometimes the keyboard sends REL_X events while the middle button is
> pressed. Thus the old "workaround" is disabled and middle button
> scrolling henceforth exhibits the known buggy behavior.
>
> Explicitly, I can confirm that the following values occur, leading to
> erroneous disabling of the workaround:
>
> cptkbd_data->middlebutton_state == 1
> usage->type == 2 [i.e., EV_REL]
> usage->code == 0 [i.e., REL_X]
>
> The keyboard is identified by lsusb as:
> ID 17ef:6047 Lenovo ThinkPad Compact Keyboard with TrackPoint
>
> This was tested with kernel 6.1.67 which contains the backported patch
> (commit 6e2076cad8873cc2a9f96e4becab35425c3656dc).
>
> I didn't test the latest patch of Dec. 12. However, I don't expect it to
> fix my issue as the only added condition
> hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
> should be satisfied, which I understand is also the intention.
> (USB_DEVICE_ID_LENOVO_CUSBKBD == 0x6047, which is the device ID of my
> keyboard as reported by lsusb.)
>
> Best regards,
> Uli

> So this means that the only reliable way is to add a sysfs parameter.
> I'll send a patch.

Thank you for the quick action!

Perhaps it would be possible to modify the firmware further in order to 
facilitate reliable detection of this modified firmware? But for now the 
solution with a sysfs parameter (and defaulting to the workaround) seems 
good.

Best regards,
Uli