Date:   Sat, 15 Dec 2018 15:45:21 +0100
From:   Clément VUCHENER <clement.vuchener@...il.com>
To:     Harry Cutts <hcutts@...omium.org>
Cc:     Peter Hutterer <peter.hutterer@...-t.net>,
        linux-input <linux-input@...r.kernel.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Jiri Kosina <jikos@...nel.org>, torvalds@...ux-foundation.org,
        Nestor Lopez Casado <nlopezcasad@...itech.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>
Subject: Re: [PATCH v3 7/8] HID: logitech: Enable high-resolution scrolling on
 Logitech mice

Le ven. 14 déc. 2018 à 19:37, Harry Cutts <hcutts@...omium.org> a écrit :
>
> Hi Clement,
>
> On Fri, 14 Dec 2018 at 05:47, Clément VUCHENER
> <clement.vuchener@...il.com> wrote:
> > Hi, The G500s (and the G500 too, I think) does support the "scrolling
> > acceleration" bit. If I set it, I get around 8 events for each wheel
> > "click", this is what this driver expects, right? If I understood
> > correctly, I should try this patch with the
> > HIDPP_QUIRK_HI_RES_SCROLL_1P0 quirk set for my mouse.
>
> Thanks for the info! Yes, that should work.

Well, it is not that simple. I get "Device not connected" errors for
both interfaces of the mouse.

logitech-hidpp-device 0003:046D:C24E.000E: Device not connected
logitech-hidpp-device 0003:046D:C24E.000F: Device not connected

Here is the usbmon trace when connecting a G500s:

0cd42cc0 3.313741 C Ii:3:001:1 0:2048 1 =
    04
0cd42cc0 3.313750 S Ii:3:001:1 -:2048 4 <
17b49a80 3.313764 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b49a80 3.313775 C Ci:3:001:0 0 4 =
    01010100
17b49a80 3.313781 S Co:3:001:0 s 23 01 0010 0002 0000 0
17b49a80 3.313789 C Co:3:001:0 0 0
17b49a80 3.313792 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b49a80 3.313797 C Ci:3:001:0 0 4 =
    01010000
17b49a80 3.340415 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b49a80 3.340438 C Ci:3:001:0 0 4 =
    01010000
17b49a80 3.367434 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b49a80 3.367448 C Ci:3:001:0 0 4 =
    01010000
17b49a80 3.394434 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b49a80 3.394449 C Ci:3:001:0 0 4 =
    01010000
17b49a80 3.421416 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b49a80 3.421431 C Ci:3:001:0 0 4 =
    01010000
17b49a80 3.421690 S Co:3:001:0 s 23 03 0004 0002 0000 0
17b49a80 3.421707 C Co:3:001:0 0 0
17b49a80 3.483421 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b49a80 3.483436 C Ci:3:001:0 0 4 =
    11010000
17b499c0 3.545429 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b499c0 3.545442 C Ci:3:001:0 0 4 =
    03011000
17b499c0 3.545448 S Co:3:001:0 s 23 01 0014 0002 0000 0
17b499c0 3.545456 C Co:3:001:0 0 0
17b499c0 3.597659 S Ci:3:000:0 s 80 06 0100 0000 0040 64 <
17b499c0 3.597851 C Ci:3:000:0 0 8 =
    12010002 00000008
17b499c0 3.597870 S Co:3:001:0 s 23 03 0004 0002 0000 0
17b499c0 3.597884 C Co:3:001:0 0 0
17b499c0 3.659419 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b499c0 3.659434 C Ci:3:001:0 0 4 =
    11010000
17b499c0 3.721421 S Ci:3:001:0 s a3 00 0000 0002 0004 4 <
17b499c0 3.721435 C Ci:3:001:0 0 4 =
    03011000
17b499c0 3.721442 S Co:3:001:0 s 23 01 0014 0002 0000 0
17b499c0 3.721451 C Co:3:001:0 0 0
17b49600 3.788420 S Ci:3:007:0 s 80 06 0100 0000 0012 18 <
17b49600 3.788897 C Ci:3:007:0 0 18 =
    12010002 00000008 6d044ec2 01840102 0301
     . . . .  . . . .  m . N .  . . . .  . .
17b49600 3.788920 S Ci:3:007:0 s 80 06 0600 0000 000a 10 <
17b49600 3.789057 C Ci:3:007:0 -32 0
17b49600 3.789092 S Ci:3:007:0 s 80 06 0600 0000 000a 10 <
17b49600 3.789438 C Ci:3:007:0 -32 0
17b49600 3.789459 S Ci:3:007:0 s 80 06 0600 0000 000a 10 <
17b49600 3.789827 C Ci:3:007:0 -32 0
17b49600 3.789850 S Ci:3:007:0 s 80 06 0200 0000 0009 9 <
17b49600 3.790272 C Ci:3:007:0 0 9 =
    09023b00 020104a0 31
     . . ; .  . . . .  1
17b49600 3.790285 S Ci:3:007:0 s 80 06 0200 0000 003b 59 <
17b49600 3.791004 C Ci:3:007:0 0 59 =
    09023b00 020104a0 31090400 00010301 02000921 11010001 22430007 05810308
     . . ; .  . . . .  1 . . .  . . . .  . . . !  . . . .  " C . .  . . . .
17b49600 3.791021 S Ci:3:007:0 s 80 06 0300 0000 00ff 255 <
17b49600 3.791203 C Ci:3:007:0 0 4 =
    04030904
17b49600 3.791212 S Ci:3:007:0 s 80 06 0302 0409 00ff 255 <
17b49600 3.791829 C Ci:3:007:0 0 50 =
    32034700 35003000 30007300 20004c00 61007300 65007200 20004700 61006d00
     2 . G .  5 . 0 .  0 . s .    . L .  a . s .  e . r .    . G .  a . m .
17b49600 3.791844 S Ci:3:007:0 s 80 06 0301 0409 00ff 255 <
17b49600 3.792141 C Ci:3:007:0 0 18 =
    12034c00 6f006700 69007400 65006300 6800
     . . L .  o . g .  i . t .  e . c .  h .
17b49600 3.792154 S Ci:3:007:0 s 80 06 0303 0409 00ff 255 <
17b49600 3.792563 C Ci:3:007:0 0 30 =
    1e033300 34004500 31003300 38003500 30003800 36003000 30003000 3900
     . . 3 .  4 . E .  1 . 3 .  8 . 5 .  0 . 8 .  6 . 0 .  0 . 0 .  9 .
17b49300 3.795944 S Co:3:007:0 s 00 09 0001 0000 0000 0
17b49300 3.795998 C Co:3:007:0 0 0
17b49300 3.796025 S Ci:3:007:0 s 80 06 0304 0409 00ff 255 <
17b49300 3.796392 C Ci:3:007:0 0 26 =
    1a035500 38003400 2e003000 31005f00 42003000 30003000 3900
     . . U .  8 . 4 .  . . 0 .  1 . _ .  B . 0 .  0 . 0 .  9 .
17b49900 3.796473 S Ci:3:007:0 s 80 06 0303 0409 00ff 255 <
17b49900 3.796883 C Ci:3:007:0 0 30 =
    1e033300 34004500 31003300 38003500 30003800 36003000 30003000 3900
     . . 3 .  4 . E .  1 . 3 .  8 . 5 .  0 . 8 .  6 . 0 .  0 . 0 .  9 .
17b49900 3.796914 S Co:3:007:0 s 21 0a 0000 0000 0000 0
17b49900 3.797027 C Co:3:007:0 -32 0
17b49900 3.797056 S Ci:3:007:0 s 81 06 2200 0000 0043 67 <
17b49900 3.798055 C Ci:3:007:0 0 67 =
    05010902 a1010901 a1000509 19012910 15002501 95107501 81020501 16018026
     . . . .  . . . .  . . . .  . . ) .  . . % .  . . u .  . . . .  . . . &
17b49840 3.798350 S Co:3:007:0 s 21 09 0211 0000 0014 20 =
    11ff0011 00000000 00000000 00000000 00000000
17b49840 3.798500 C Co:3:007:0 0 20 >
17b49240 9.289560 S Ci:3:007:0 s 80 06 0303 0409 00ff 255 <
17b49240 9.289999 C Ci:3:007:0 0 30 =
    1e033300 34004500 31003300 38003500 30003800 36003000 30003000 3900
     . . 3 .  4 . E .  1 . 3 .  8 . 5 .  0 . 8 .  6 . 0 .  0 . 0 .  9 .
17b49240 9.290040 S Co:3:007:0 s 21 0a 0000 0001 0000 0
17b49240 9.290145 C Co:3:007:0 -32 0
17b49240 9.290155 S Ci:3:007:0 s 81 06 2200 0001 007a 122 <
17b49240 9.291756 C Ci:3:007:0 0 122 =
    05010906 a1018501 050719e0 29e71500 25017501 95088102 95057508 150026a4
     . . . .  . . . .  . . . .  ) . . .  % . u .  . . . .  . . u .  . . & .
17b49c00 9.292167 S Co:3:007:0 s 21 09 0211 0001 0014 20 =
    11ff0011 00000000 00000000 00000000 00000000
17b49c00 9.292628 C Co:3:007:0 0 20 >

It looks like the mouse is not responding to the
root_get_protocol_version packet. I don't know why, but even if it
did, it would not work correctly. The HID++ reports will only work
with one of the interfaces, the other should be ignored by the driver
instead of being disabled because it failed to respond to the HID++
report. The G500s and G500 HID++ interface also use the device index 0
instead of 0xff.

For comparison, if I use my distribution kernel
(4.19.7-200.fc28.x86_64) and send reports from user-space (using
hidraw) instead of for-4.21/highres-wheel kernel. I get this kind of
trace:

592193c0 0.253975 S Co:3:003:0 s 21 09 0211 0001 0014 20 =
    11ff0011 00000000 00000000 00000000 00000000
592193c0 0.254426 C Co:3:003:0 0 20 >
9ee5ff00 0.255859 C Ii:3:003:2 0:1 7 =
    10ff8f00 110800
9ee5ff00 0.255867 S Ii:3:003:2 -:1 20 <
592193c0 11.116794 S Co:3:003:0 s 21 09 0211 0001 0014 20 =
    11000011 00000000 00000000 00000000 00000000
592193c0 11.117258 C Co:3:003:0 0 20 >
9ee5ff00 11.118023 C Ii:3:003:2 0:1 7 =
    10008f00 110100
9ee5ff00 11.118033 S Ii:3:003:2 -:1 20 <

When using device index 0xff the mouse responds with a
ERR_UNKNOWN_DEVICE (0x08) error, when using device index 0 it responds
ERR_INVALID_SUBID (0x01), the expected error for a HID++1.0 device.

Here is the diff from the for-4.21/highres-wheel branch, I only added
the devices to the device list with the required quirk.

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index ed35c9a9a110..a1c431e7841c 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -724,6 +724,7 @@
 #define USB_DEVICE_ID_LOGITECH_KEYBOARD_G710_PLUS 0xc24d
 #define USB_DEVICE_ID_LOGITECH_MOUSE_C01A      0xc01a
 #define USB_DEVICE_ID_LOGITECH_MOUSE_C05A      0xc05a
+#define USB_DEVICE_ID_LOGITECH_MOUSE_G500      0xc068
 #define USB_DEVICE_ID_LOGITECH_MOUSE_C06A      0xc06a
 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD_CORD  0xc20a
 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD       0xc211
@@ -731,6 +732,7 @@
 #define USB_DEVICE_ID_LOGITECH_DUAL_ACTION     0xc216
 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2      0xc218
 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2    0xc219
+#define USB_DEVICE_ID_LOGITECH_MOUSE_G500S     0xc24e
 #define USB_DEVICE_ID_LOGITECH_G29_WHEEL       0xc24f
 #define USB_DEVICE_ID_LOGITECH_G920_WHEEL      0xc262
 #define USB_DEVICE_ID_LOGITECH_WINGMAN_F3D     0xc283
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 15ed6177a7a3..b9b34ce455a4 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3416,6 +3416,10 @@ static const struct hid_device_id hidpp_devices[] = {

        { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_G920_WHEEL),
                .driver_data = HIDPP_QUIRK_CLASS_G920 |
HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
+       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_MOUSE_G500),
+               .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
+       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_MOUSE_G500S),
+               .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
        {}
 };

Powered by blists - more mailing lists