Re: hidms: don't ignore mice with no x/y coordinates
joshua stein writes:
> A bug was reported where a Kensington USB trackball didn't work
> properly:
>
> uhidev4 at uhub0 port 6 configuration 1 interface 0 "Kensington Expert
> Wireless TB" rev 2.00/1.02 addr 9
> uhidev4: iclass 3/1, 3 report ids
> ums3 at uhidev4 reportid 1
> ums3: mouse has no X report
> ums4 at uhidev4 reportid 2: 0 button
> wsmouse3 at ums4 mux 0
>
> After looking at the HID report descriptor, this device is weird in
> that it puts the buttons and wheel on one report and the trackball
> X/Y coordinates an another. This causes uhidev to attach two ums
> devices, but the first one fails because there are no X/Y reports
> found.
>
> The proper fix is probably to make ums act like umt and use
> UHIDEV_CLAIM_MULTIPLE_REPORTID to find all of the necessary reports
> and attach to multiple at once if needed. I started working on this
> but all of the logic in hidms_setup gets tricky when it has to look
> at multiple reports. So an easier fix is to just not consider a
> mouse with no X/Y reports invalid.
>
> Now the device attaches to the first button/wheel report:
>
> uhidev4 at uhub4 port 4 configuration 1 interface 0 "Kensington Expert
> Wireless TB" rev 2.00/1.02 addr 3
> uhidev4: iclass 3/1, 3 report ids
> ums1 at uhidev4 reportid 1: 5 buttons, Z and W dir
> wsmouse1 at ums1 mux 0
> ums2 at uhidev4 reportid 2: 0 buttons
> wsmouse2 at ums2 mux 0
>
> Checking dmesglog for 'no X report' yields a lot of results, so this
> may help on other devices.
OK gnezdo
>
>
> diff --git sys/dev/hid/hidms.c sys/dev/hid/hidms.c
> index ab9cd9c797e..92ca89537da 100644
> --- sys/dev/hid/hidms.c
> +++ sys/dev/hid/hidms.c
> @@ -76,10 +76,9 @@ hidms_setup(struct device *self, struct hidms *ms,
> uint32_t quirks,
> ms->sc_flags = quirks;
>
> if (!hid_locate(desc, dlen, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X), id,
> - hid_input, >sc_loc_x, )) {
> - printf("\n%s: mouse has no X report\n", self->dv_xname);
> - return ENXIO;
> - }
> + hid_input, >sc_loc_x, ))
> + ms->sc_loc_x.size = 0;
> +
> switch(flags & MOUSE_FLAGS_MASK) {
> case 0:
> ms->sc_flags |= HIDMS_ABSX;
> @@ -93,10 +92,9 @@ hidms_setup(struct device *self, struct hidms *ms,
> uint32_t quirks,
> }
>
> if (!hid_locate(desc, dlen, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y), id,
> - hid_input, >sc_loc_y, )) {
> - printf("\n%s: mouse has no Y report\n", self->dv_xname);
> - return ENXIO;
> - }
> + hid_input, >sc_loc_y, ))
> + ms->sc_loc_y.size = 0;
> +
> switch(flags & MOUSE_FLAGS_MASK) {
> case 0:
> ms->sc_flags |= HIDMS_ABSY;
> @@ -292,7 +290,7 @@ hidms_attach(struct hidms *ms, const struct
> wsmouse_accessops *ops)
> #endif
>
> printf(": %d button%s",
> - ms->sc_num_buttons, ms->sc_num_buttons <= 1 ? "" : "s");
> + ms->sc_num_buttons, ms->sc_num_buttons == 1 ? "" : "s");
> switch (ms->sc_flags & (HIDMS_Z | HIDMS_W)) {
> case HIDMS_Z:
> printf(", Z dir");
Re: hidms: don't ignore mice with no x/y coordinates
On Mon, May 24, 2021 at 03:15:14PM BST, joshua stein wrote:
> A bug was reported where a Kensington USB trackball didn't work
> properly:
>
> uhidev4 at uhub0 port 6 configuration 1 interface 0 "Kensington Expert
> Wireless TB" rev 2.00/1.02 addr 9
> uhidev4: iclass 3/1, 3 report ids
> ums3 at uhidev4 reportid 1
> ums3: mouse has no X report
> ums4 at uhidev4 reportid 2: 0 button
> wsmouse3 at ums4 mux 0
>
> After looking at the HID report descriptor, this device is weird in
> that it puts the buttons and wheel on one report and the trackball
> X/Y coordinates an another. This causes uhidev to attach two ums
> devices, but the first one fails because there are no X/Y reports
> found.
>
> The proper fix is probably to make ums act like umt and use
> UHIDEV_CLAIM_MULTIPLE_REPORTID to find all of the necessary reports
> and attach to multiple at once if needed. I started working on this
> but all of the logic in hidms_setup gets tricky when it has to look
> at multiple reports. So an easier fix is to just not consider a
> mouse with no X/Y reports invalid.
>
> Now the device attaches to the first button/wheel report:
>
> uhidev4 at uhub4 port 4 configuration 1 interface 0 "Kensington Expert
> Wireless TB" rev 2.00/1.02 addr 3
> uhidev4: iclass 3/1, 3 report ids
> ums1 at uhidev4 reportid 1: 5 buttons, Z and W dir
> wsmouse1 at ums1 mux 0
> ums2 at uhidev4 reportid 2: 0 buttons
> wsmouse2 at ums2 mux 0
>
> Checking dmesglog for 'no X report' yields a lot of results, so this
> may help on other devices.
Hi all,
Just an FYI, this is the trackball in question:
https://www.kensington.com/en-gb/p/products/control/trackballs/expert-mouse-wireless-trackball-1/
In its current state, it can only be used as a paperweight as, when
plugged in, only the trackball itself (pointer) works but none of
its four buttons or scroll ring do - they aren't being recognised
at all. It works just fine on Linux, macOS, or Windows, though.
With the below patch, everything's fine - all four buttons and the
scroll ring are being recognised and work as expected.
Any chance of this getting in?
Regards,
Raf
> diff --git sys/dev/hid/hidms.c sys/dev/hid/hidms.c
> index ab9cd9c797e..92ca89537da 100644
> --- sys/dev/hid/hidms.c
> +++ sys/dev/hid/hidms.c
> @@ -76,10 +76,9 @@ hidms_setup(struct device *self, struct hidms *ms,
> uint32_t quirks,
> ms->sc_flags = quirks;
>
> if (!hid_locate(desc, dlen, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X), id,
> - hid_input, >sc_loc_x, )) {
> - printf("\n%s: mouse has no X report\n", self->dv_xname);
> - return ENXIO;
> - }
> + hid_input, >sc_loc_x, ))
> + ms->sc_loc_x.size = 0;
> +
> switch(flags & MOUSE_FLAGS_MASK) {
> case 0:
> ms->sc_flags |= HIDMS_ABSX;
> @@ -93,10 +92,9 @@ hidms_setup(struct device *self, struct hidms *ms,
> uint32_t quirks,
> }
>
> if (!hid_locate(desc, dlen, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y), id,
> - hid_input, >sc_loc_y, )) {
> - printf("\n%s: mouse has no Y report\n", self->dv_xname);
> - return ENXIO;
> - }
> + hid_input, >sc_loc_y, ))
> + ms->sc_loc_y.size = 0;
> +
> switch(flags & MOUSE_FLAGS_MASK) {
> case 0:
> ms->sc_flags |= HIDMS_ABSY;
> @@ -292,7 +290,7 @@ hidms_attach(struct hidms *ms, const struct
> wsmouse_accessops *ops)
> #endif
>
> printf(": %d button%s",
> - ms->sc_num_buttons, ms->sc_num_buttons <= 1 ? "" : "s");
> + ms->sc_num_buttons, ms->sc_num_buttons == 1 ? "" : "s");
> switch (ms->sc_flags & (HIDMS_Z | HIDMS_W)) {
> case HIDMS_Z:
> printf(", Z dir");
>
hidms: don't ignore mice with no x/y coordinates
A bug was reported where a Kensington USB trackball didn't work
properly:
uhidev4 at uhub0 port 6 configuration 1 interface 0 "Kensington Expert
Wireless TB" rev 2.00/1.02 addr 9
uhidev4: iclass 3/1, 3 report ids
ums3 at uhidev4 reportid 1
ums3: mouse has no X report
ums4 at uhidev4 reportid 2: 0 button
wsmouse3 at ums4 mux 0
After looking at the HID report descriptor, this device is weird in
that it puts the buttons and wheel on one report and the trackball
X/Y coordinates an another. This causes uhidev to attach two ums
devices, but the first one fails because there are no X/Y reports
found.
The proper fix is probably to make ums act like umt and use
UHIDEV_CLAIM_MULTIPLE_REPORTID to find all of the necessary reports
and attach to multiple at once if needed. I started working on this
but all of the logic in hidms_setup gets tricky when it has to look
at multiple reports. So an easier fix is to just not consider a
mouse with no X/Y reports invalid.
Now the device attaches to the first button/wheel report:
uhidev4 at uhub4 port 4 configuration 1 interface 0 "Kensington Expert
Wireless TB" rev 2.00/1.02 addr 3
uhidev4: iclass 3/1, 3 report ids
ums1 at uhidev4 reportid 1: 5 buttons, Z and W dir
wsmouse1 at ums1 mux 0
ums2 at uhidev4 reportid 2: 0 buttons
wsmouse2 at ums2 mux 0
Checking dmesglog for 'no X report' yields a lot of results, so this
may help on other devices.
diff --git sys/dev/hid/hidms.c sys/dev/hid/hidms.c
index ab9cd9c797e..92ca89537da 100644
--- sys/dev/hid/hidms.c
+++ sys/dev/hid/hidms.c
@@ -76,10 +76,9 @@ hidms_setup(struct device *self, struct hidms *ms, uint32_t
quirks,
ms->sc_flags = quirks;
if (!hid_locate(desc, dlen, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X), id,
- hid_input, >sc_loc_x, )) {
- printf("\n%s: mouse has no X report\n", self->dv_xname);
- return ENXIO;
- }
+ hid_input, >sc_loc_x, ))
+ ms->sc_loc_x.size = 0;
+
switch(flags & MOUSE_FLAGS_MASK) {
case 0:
ms->sc_flags |= HIDMS_ABSX;
@@ -93,10 +92,9 @@ hidms_setup(struct device *self, struct hidms *ms, uint32_t
quirks,
}
if (!hid_locate(desc, dlen, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y), id,
- hid_input, >sc_loc_y, )) {
- printf("\n%s: mouse has no Y report\n", self->dv_xname);
- return ENXIO;
- }
+ hid_input, >sc_loc_y, ))
+ ms->sc_loc_y.size = 0;
+
switch(flags & MOUSE_FLAGS_MASK) {
case 0:
ms->sc_flags |= HIDMS_ABSY;
@@ -292,7 +290,7 @@ hidms_attach(struct hidms *ms, const struct
wsmouse_accessops *ops)
#endif
printf(": %d button%s",
- ms->sc_num_buttons, ms->sc_num_buttons <= 1 ? "" : "s");
+ ms->sc_num_buttons, ms->sc_num_buttons == 1 ? "" : "s");
switch (ms->sc_flags & (HIDMS_Z | HIDMS_W)) {
case HIDMS_Z:
printf(", Z dir");
