Hi,
On Tue, Feb 09, 2021 at 10:50:25AM +1300, Peter Kane wrote:
> Hello
>
> A recent change to -current has broken my mouse, which is part of a Logitech
> wireless mouse/keyboard combo. It seems to be associated in some way with
> this one:
>
> Date: Thu, 4 Feb 2021 09:25:39 -0700 (MST)
> From: Anton Lindqvist <[email protected]>
>
> Log message:
> Add uhidpp(4), a driver for Logitech HID++ devices. Currently limited to
> exposing battery sensors for HID++ 2.0 devices. Most of the code is
> derived from the hid-logitech-hidpp Linux driver.
>
> My keyboard combo now appears as uhidpp0 in the final line of the output
> below:
>
> uhidev0 at uhub2 port 3 configuration 1 interface 0 "Logitech USB
> Receiver" rev 2.00/29.01 addr 4
> uhidev0: iclass 3/1
> ukbd0 at uhidev0: 8 variable keys, 6 key codes
> wskbd1 at ukbd0 mux 1
> wskbd1: connecting to wsdisplay0
> uhidev1 at uhub2 port 3 configuration 1 interface 1 "Logitech USB
> Receiver" rev 2.00/29.01 addr 4
> uhidev1: iclass 3/1, 17 report ids
> uhidpp0 at uhidev1 device 1 keyboard " " serial 00-00-00-00, device 2
> mouse "Wireless Mouse" serial e9-87-e7-ec
>
> Previously, it looked more like this:
>
> uhidev1 at uhub2 port 3 configuration 1 interface 1 "Logitech USB
> Receiver" rev 2.00/29.01 addr 4
> uhidev1: iclass 3/1, 17 report ids
> ums0 at uhidev1 reportid 2: 16 buttons, Z and W dir
> wsmouse2 at ums0 mux 0
> uhid0 at uhidev1 reportid 3: input=4, output=0, feature=0
> uhid1 at uhidev1 reportid 4: input=1, output=0, feature=0
> uhid2 at uhidev1 reportid 16: input=6, output=6, feature=0
> uhid3 at uhidev1 reportid 17: input=19, output=19, feature=0
>
> Full -current dmesg below.
Sorry about the regression. Could you try the following diff and send me
the complete dmesg. I've also prepared an amd64 kernel with the patch
applied:
https://www.basename.se/tmp/bsd.uhidpp.claim
diff --git sys/dev/usb/uhidev.c sys/dev/usb/uhidev.c
index 9915b660d9d..1c6e9876362 100644
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -273,11 +273,13 @@ uhidev_attach(struct device *parent, struct device *self,
void *aux)
hid_report_size(desc, size, hid_feature, repid) == 0)
continue;
+ /* Could already be assigned by uhidev_set_report_dev(). */
+ if (sc->sc_subdevs[repid] != NULL)
+ continue;
+
uha.reportid = repid;
dev = config_found_sm(self, &uha, uhidevprint, uhidevsubmatch);
- /* Could already be assigned by uhidev_set_report_dev(). */
- if (sc->sc_subdevs[repid] == NULL)
- sc->sc_subdevs[repid] = (struct uhidev *)dev;
+ sc->sc_subdevs[repid] = (struct uhidev *)dev;
}
}
diff --git sys/dev/usb/uhidpp.c sys/dev/usb/uhidpp.c
index b041d86fecd..4b8eed37de3 100644
--- sys/dev/usb/uhidpp.c
+++ sys/dev/usb/uhidpp.c
@@ -29,7 +29,7 @@
#include <dev/usb/usbdevs.h>
#include <dev/usb/uhidev.h>
-/* #define UHIDPP_DEBUG */
+#define UHIDPP_DEBUG
#ifdef UHIDPP_DEBUG
#define DPRINTF(x...) do { \
@@ -281,7 +281,8 @@ uhidpp_match(struct device *parent, void *match, void *aux)
void *desc;
int descsiz, siz;
- if (uha->reportid != UHIDEV_CLAIM_ALLREPORTID)
+ if (uha->reportid != HIDPP_REPORT_ID_SHORT &&
+ uha->reportid != HIDPP_REPORT_ID_LONG)
return UMATCH_NONE;
if (usb_lookup(uhidpp_devs,
@@ -325,7 +326,7 @@ uhidpp_attach(struct device *parent, struct device *self,
void *aux)
error = uhidev_open(&sc->sc_hdev);
if (error) {
- printf(" error %d\n", error);
+ printf(" open error %d\n", error);
return;
}
@@ -338,10 +339,18 @@ uhidpp_attach(struct device *parent, struct device *self,
void *aux)
* in order to receive responses. Necessary as uhidev by default
* performs the wiring after the attach routine has returned.
*/
- uhidev_set_report_dev(sc->sc_hdev.sc_parent, &sc->sc_hdev,
+ error = uhidev_set_report_dev(sc->sc_hdev.sc_parent, &sc->sc_hdev,
HIDPP_REPORT_ID_SHORT);
- uhidev_set_report_dev(sc->sc_hdev.sc_parent, &sc->sc_hdev,
+ if (error) {
+ printf(" short report error %d\n", error);
+ return;
+ }
+ error = uhidev_set_report_dev(sc->sc_hdev.sc_parent, &sc->sc_hdev,
HIDPP_REPORT_ID_LONG);
+ if (error) {
+ printf(" long report error %d\n", error);
+ return;
+ }
/* Probe paired devices. */
for (i = 0; i < UHIDPP_NDEVICES; i++) {
