Hi,
In an attempt to fix a bug related to upd(4) I discovered that the
pseudo report id UHIDEV_CLAIM_MULTIPLE_REPORTID is conflicting with an
actual report id. The previous fix, reverted by now, avoided the
conflict by incrementing the pseudo report id was not a good idea
since a report id is expected to be represented using a single unsigned
byte elsewhere in the usb stack.
Here's a second attempt. Report id zero is according to the USB HID
specification reserved and should not be used. We can define
UHIDEV_CLAIM_MULTIPLE_REPORTID as zero removing the need to use a larger
integer type than the existing uint8_t for the report id. Another
correction is also needed. Some descriptors only supports a single
report and the report id is in this case optional. Internally, uhidev
uses report id zero for such devices. It's therefore of importance to
not perform a claim multiple report ids attachment on such devices as
there's only one report id to claim.
Testing would be much appreciated.
Comments? OK?
diff --git sys/dev/usb/uhidev.c sys/dev/usb/uhidev.c
index 5fe2f702e21..f5cc5984b59 100644
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -247,29 +247,36 @@ uhidev_attach(struct device *parent, struct device *self,
void *aux)
sc->sc_isize += (nrepid != 1); /* one byte for the report ID */
DPRINTF(("uhidev_attach: isize=%d\n", sc->sc_isize));
+ memset(&uha, 0, sizeof(uha));
uha.uaa = uaa;
uha.parent = sc;
- uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
- uha.nreports = nrepid;
- uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
/* Look for a driver claiming multiple report IDs first. */
- dev = config_found_sm(self, &uha, NULL, uhidevsubmatch);
- if (dev != NULL) {
- for (repid = 0; repid < nrepid; repid++) {
- /*
- * Could already be assigned by uhidev_set_report_dev().
- */
- if (sc->sc_subdevs[repid] != NULL)
- continue;
-
- if (uha.claimed[repid])
+ if (nrepid > 1) {
+ uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
+ uha.nreports = nrepid;
+ uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
+
+ dev = config_found_sm(self, &uha, NULL, uhidevsubmatch);
+ if (dev != NULL) {
+ for (repid = 0; repid < nrepid; repid++) {
+ /*
+ * Could already be assigned by
+ * uhidev_set_report_dev().
+ */
+ if (sc->sc_subdevs[repid] != NULL)
+ continue;
+
+ if (!uha.claimed[repid])
+ continue;
sc->sc_subdevs[repid] = (struct uhidev *)dev;
+ }
}
- }
- free(uha.claimed, M_TEMP, nrepid);
- uha.claimed = NULL;
+ free(uha.claimed, M_TEMP, nrepid);
+ uha.nreports = 0;
+ uha.claimed = NULL;
+ }
for (repid = 0; repid < nrepid; repid++) {
DPRINTF(("%s: try repid=%d\n", __func__, repid));
diff --git sys/dev/usb/uhidev.h sys/dev/usb/uhidev.h
index 6ce75b1f49d..9ae85e1ab9f 100644
--- sys/dev/usb/uhidev.h
+++ sys/dev/usb/uhidev.h
@@ -81,8 +81,8 @@ struct uhidev_attach_arg {
struct usb_attach_arg *uaa;
struct uhidev_softc *parent;
uint8_t reportid;
-#define UHIDEV_CLAIM_MULTIPLE_REPORTID 255
- uint8_t nreports;
+#define UHIDEV_CLAIM_MULTIPLE_REPORTID 0
+ u_int nreports;
uint8_t *claimed;
};
