| Message ID | 20231212133148.1000761-1-me@khvoinitsky.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: lenovo: Restrict detection of patched firmware only to USB cptkbd | expand |
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
вс, 24 дек. 2023 г. в 18:51, Uli v. d. Ohe <u-v@mailbox.org>: > > > 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. Unfortunately no, the firmware is closed-source and was patched in binary form AFAIR (by replacing 1-2 instructions). Moreover, it cannot be updated in the wireless version of the keyboard (ThinkPad Compact Keyboard II). > > Best regards, > Uli
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;
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(-)