MFp4 //depot/projects/usb; 157814, 157863, 157868

- The software computed HID size is not always correct, because the algoritm
  does not handle unsorted HID descriptors.
- Change the way we obtain the report ID.
- Use the X/Y/Z+button locations instead for report ID source for ums.
- Add more range checks.
- Remove Microsoft Mouse quirks. If the positions are moduloed the report
  length multiplied by 8, the values seem correct.
- Some minor style changes.

Submitted by:	Hans Petter Selasky

4 files changed, 149 insertions, 148 deletions

diff --git a/sys/dev/usb/input/ums.c b/sys/dev/usb/input/ums.c
index b064aa9..04e2580 100644
--- a/sys/dev/usb/input/ums.c
+++ b/sys/dev/usb/input/ums.c
@@ -87,8 +87,7 @@ SYSCTL_INT(_hw_usb2_ums, OID_AUTO, debug, CTLFLAG_RW,
 
 enum {
 	UMS_INTR_DT,
-	UMS_INTR_CS,
-	UMS_N_TRANSFER = 2,
+	UMS_N_TRANSFER,
 };
 
 struct ums_softc {
@@ -113,9 +112,8 @@ struct ums_softc {
 #define	UMS_FLAG_Z_AXIS     0x0004
 #define	UMS_FLAG_T_AXIS     0x0008
 #define	UMS_FLAG_SBU        0x0010	/* spurious button up events */
-#define	UMS_FLAG_INTR_STALL 0x0020	/* set if transfer error */
-#define	UMS_FLAG_REVZ	    0x0040	/* Z-axis is reversed */
-#define	UMS_FLAG_W_AXIS     0x0080
+#define	UMS_FLAG_REVZ	    0x0020	/* Z-axis is reversed */
+#define	UMS_FLAG_W_AXIS     0x0040
 
 	uint8_t	sc_buttons;
 	uint8_t	sc_iid;
@@ -124,7 +122,6 @@ struct ums_softc {
 
 static void ums_put_queue_timeout(void *__sc);
 
-static usb2_callback_t ums_clear_stall_callback;
 static usb2_callback_t ums_intr_callback;
 
 static device_probe_t ums_probe;
@@ -159,19 +156,6 @@ ums_put_queue_timeout(void *__sc)
 }
 
 static void
-ums_clear_stall_callback(struct usb2_xfer *xfer)
-{
-	struct ums_softc *sc = xfer->priv_sc;
-	struct usb2_xfer *xfer_other = sc->sc_xfer[UMS_INTR_DT];
-
-	if (usb2_clear_stall_callback(xfer, xfer_other)) {
-		DPRINTF("stall cleared\n");
-		sc->sc_flags &= ~UMS_FLAG_INTR_STALL;
-		usb2_transfer_start(xfer_other);
-	}
-}
-
-static void
 ums_intr_callback(struct usb2_xfer *xfer)
 {
 	struct ums_softc *sc = xfer->priv_sc;
@@ -194,9 +178,9 @@ ums_intr_callback(struct usb2_xfer *xfer)
 			    sizeof(sc->sc_temp));
 			len = sizeof(sc->sc_temp);
 		}
-		if (len == 0) {
+		if (len == 0)
 			goto tr_setup;
-		}
+
 		usb2_copy_out(xfer->frbuffers, 0, buf, len);
 
 		DPRINTFN(6, "data = %02x %02x %02x %02x "
@@ -207,21 +191,23 @@ ums_intr_callback(struct usb2_xfer *xfer)
 		    (len > 6) ? buf[6] : 0, (len > 7) ? buf[7] : 0);
 
 		/*
-		 * The M$ Wireless Intellimouse 2.0 sends 1 extra leading byte
-		 * of data compared to most USB mice. This byte frequently
-		 * switches from 0x01 (usual state) to 0x02. I assume it is to
-		 * allow extra, non-standard, reporting (say battery-life).
+		 * The M$ Wireless Intellimouse 2.0 sends 1 extra
+		 * leading byte of data compared to most USB
+		 * mice. This byte frequently switches from 0x01
+		 * (usual state) to 0x02. I assume it is to allow
+		 * extra, non-standard, reporting (say battery-life).
 		 *
-		 * However at the same time it generates a left-click message
-		 * on the button byte which causes spurious left-click's where
-		 * there shouldn't be.  This should sort that.  Currently it's
-		 * the only user of UMS_FLAG_T_AXIS so use it as an
-		 * identifier.
+		 * However at the same time it generates a left-click
+		 * message on the button byte which causes spurious
+		 * left-click's where there shouldn't be.  This should
+		 * sort that.  Currently it's the only user of
+		 * UMS_FLAG_T_AXIS so use it as an identifier.
 		 *
 		 *
-		 * UPDATE: This problem affects the M$ Wireless Notebook Optical Mouse,
-		 * too. However, the leading byte for this mouse is normally 0x11,
-		 * and the phantom mouse click occurs when its 0x14.
+		 * UPDATE: This problem affects the M$ Wireless
+		 * Notebook Optical Mouse, too. However, the leading
+		 * byte for this mouse is normally 0x11, and the
+		 * phantom mouse click occurs when its 0x14.
 		 *
 		 * We probably should switch to some more official quirk.
 		 */
@@ -287,12 +273,14 @@ ums_intr_callback(struct usb2_xfer *xfer)
 			 */
 
 			/*
-		         * The Qtronix keyboard has a built in PS/2 port for a mouse.
-		         * The firmware once in a while posts a spurious button up
-		         * event. This event we ignore by doing a timeout for 50 msecs.
-		         * If we receive dx=dy=dz=buttons=0 before we add the event to
-		         * the queue.
-		         * In any other case we delete the timeout event.
+		         * The Qtronix keyboard has a built in PS/2
+		         * port for a mouse.  The firmware once in a
+		         * while posts a spurious button up
+		         * event. This event we ignore by doing a
+		         * timeout for 50 msecs.  If we receive
+		         * dx=dy=dz=buttons=0 before we add the event
+		         * to the queue.  In any other case we delete
+		         * the timeout event.
 		         */
 			if ((sc->sc_flags & UMS_FLAG_SBU) &&
 			    (dx == 0) && (dy == 0) && (dz == 0) && (dt == 0) &&
@@ -309,25 +297,21 @@ ums_intr_callback(struct usb2_xfer *xfer)
 		}
 	case USB_ST_SETUP:
 tr_setup:
-		if (sc->sc_flags & UMS_FLAG_INTR_STALL) {
-			usb2_transfer_start(sc->sc_xfer[UMS_INTR_CS]);
-		} else {
-			/* check if we can put more data into the FIFO */
-			if (usb2_fifo_put_bytes_max(
-			    sc->sc_fifo.fp[USB_FIFO_RX]) != 0) {
-				xfer->frlengths[0] = xfer->max_data_length;
-				usb2_start_hardware(xfer);
-			}
+		/* check if we can put more data into the FIFO */
+		if (usb2_fifo_put_bytes_max(
+		    sc->sc_fifo.fp[USB_FIFO_RX]) != 0) {
+			xfer->frlengths[0] = xfer->max_data_length;
+			usb2_start_hardware(xfer);
 		}
-		return;
+		break;
 
 	default:			/* Error */
 		if (xfer->error != USB_ERR_CANCELLED) {
-			/* start clear stall */
-			sc->sc_flags |= UMS_FLAG_INTR_STALL;
-			usb2_transfer_start(sc->sc_xfer[UMS_INTR_CS]);
+			/* try clear stall first */
+			xfer->flags.stall_pipe = 1;
+			goto tr_setup;
 		}
-		return;
+		break;
 	}
 }
 
@@ -341,16 +325,6 @@ static const struct usb2_config ums_config[UMS_N_TRANSFER] = {
 		.mh.bufsize = 0,	/* use wMaxPacketSize */
 		.mh.callback = &ums_intr_callback,
 	},
-
-	[UMS_INTR_CS] = {
-		.type = UE_CONTROL,
-		.endpoint = 0x00,	/* Control pipe */
-		.direction = UE_DIR_ANY,
-		.mh.bufsize = sizeof(struct usb2_device_request),
-		.mh.callback = &ums_clear_stall_callback,
-		.mh.timeout = 1000,	/* 1 second */
-		.mh.interval = 50,	/* 50ms */
-	},
 };
 
 static int
@@ -359,39 +333,34 @@ ums_probe(device_t dev)
 	struct usb2_attach_arg *uaa = device_get_ivars(dev);
 	struct usb2_interface_descriptor *id;
 	void *d_ptr;
-	int32_t error = 0;
+	int error;
 	uint16_t d_len;
 
 	DPRINTFN(11, "\n");
 
-	if (uaa->usb2_mode != USB_MODE_HOST) {
+	if (uaa->usb2_mode != USB_MODE_HOST)
 		return (ENXIO);
-	}
-	if (uaa->iface == NULL) {
-		return (ENXIO);
-	}
+
 	id = usb2_get_interface_descriptor(uaa->iface);
 
 	if ((id == NULL) ||
-	    (id->bInterfaceClass != UICLASS_HID)) {
+	    (id->bInterfaceClass != UICLASS_HID))
 		return (ENXIO);
-	}
-	error = usb2_req_get_hid_desc
-	    (uaa->device, &Giant,
+
+	error = usb2_req_get_hid_desc(uaa->device, &Giant,
 	    &d_ptr, &d_len, M_TEMP, uaa->info.bIfaceIndex);
 
-	if (error) {
+	if (error)
 		return (ENXIO);
-	}
+
 	if (hid_is_collection(d_ptr, d_len,
-	    HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_MOUSE))) {
+	    HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_MOUSE)))
 		error = 0;
-	} else if ((id->bInterfaceSubClass == UISUBCLASS_BOOT) &&
-	    (id->bInterfaceProtocol == UIPROTO_MOUSE)) {
+	else if ((id->bInterfaceSubClass == UISUBCLASS_BOOT) &&
+	    (id->bInterfaceProtocol == UIPROTO_MOUSE))
 		error = 0;
-	} else {
+	else
 		error = ENXIO;
-	}
 
 	free(d_ptr, M_TEMP);
 	return (error);
@@ -404,9 +373,10 @@ ums_attach(device_t dev)
 	struct ums_softc *sc = device_get_softc(dev);
 	void *d_ptr = NULL;
 	int unit = device_get_unit(dev);
-	int32_t isize;
+	int isize;
+	int isizebits;
+	int err;
 	uint32_t flags;
-	int32_t err;
 	uint16_t d_len;
 	uint8_t i;
 
@@ -443,14 +413,14 @@ ums_attach(device_t dev)
 		goto detach;
 	}
 	if (hid_locate(d_ptr, d_len, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X),
-	    hid_input, &sc->sc_loc_x, &flags)) {
+	    hid_input, &sc->sc_loc_x, &flags, &sc->sc_iid)) {
 
 		if ((flags & MOUSE_FLAGS_MASK) == MOUSE_FLAGS) {
 			sc->sc_flags |= UMS_FLAG_X_AXIS;
 		}
 	}
 	if (hid_locate(d_ptr, d_len, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y),
-	    hid_input, &sc->sc_loc_y, &flags)) {
+	    hid_input, &sc->sc_loc_y, &flags, &sc->sc_iid)) {
 
 		if ((flags & MOUSE_FLAGS_MASK) == MOUSE_FLAGS) {
 			sc->sc_flags |= UMS_FLAG_Y_AXIS;
@@ -458,9 +428,9 @@ ums_attach(device_t dev)
 	}
 	/* Try the wheel first as the Z activator since it's tradition. */
 	if (hid_locate(d_ptr, d_len, HID_USAGE2(HUP_GENERIC_DESKTOP,
-	    HUG_WHEEL), hid_input, &sc->sc_loc_z, &flags) ||
+	    HUG_WHEEL), hid_input, &sc->sc_loc_z, &flags, &sc->sc_iid) ||
 	    hid_locate(d_ptr, d_len, HID_USAGE2(HUP_GENERIC_DESKTOP,
-	    HUG_TWHEEL), hid_input, &sc->sc_loc_z, &flags)) {
+	    HUG_TWHEEL), hid_input, &sc->sc_loc_z, &flags, &sc->sc_iid)) {
 		if ((flags & MOUSE_FLAGS_MASK) == MOUSE_FLAGS) {
 			sc->sc_flags |= UMS_FLAG_Z_AXIS;
 		}
@@ -469,14 +439,14 @@ ums_attach(device_t dev)
 		 * put the Z on the W coordinate.
 		 */
 		if (hid_locate(d_ptr, d_len, HID_USAGE2(HUP_GENERIC_DESKTOP,
-		    HUG_Z), hid_input, &sc->sc_loc_w, &flags)) {
+		    HUG_Z), hid_input, &sc->sc_loc_w, &flags, &sc->sc_iid)) {
 
 			if ((flags & MOUSE_FLAGS_MASK) == MOUSE_FLAGS) {
 				sc->sc_flags |= UMS_FLAG_W_AXIS;
 			}
 		}
 	} else if (hid_locate(d_ptr, d_len, HID_USAGE2(HUP_GENERIC_DESKTOP,
-	    HUG_Z), hid_input, &sc->sc_loc_z, &flags)) {
+	    HUG_Z), hid_input, &sc->sc_loc_z, &flags, &sc->sc_iid)) {
 
 		if ((flags & MOUSE_FLAGS_MASK) == MOUSE_FLAGS) {
 			sc->sc_flags |= UMS_FLAG_Z_AXIS;
@@ -490,7 +460,7 @@ ums_attach(device_t dev)
 	 * TWHEEL
 	 */
 	if (hid_locate(d_ptr, d_len, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_TWHEEL),
-	    hid_input, &sc->sc_loc_t, &flags)) {
+	    hid_input, &sc->sc_loc_t, &flags, &sc->sc_iid)) {
 
 		sc->sc_loc_t.pos += 8;
 
@@ -502,14 +472,14 @@ ums_attach(device_t dev)
 
 	for (i = 0; i < UMS_BUTTON_MAX; i++) {
 		if (!hid_locate(d_ptr, d_len, HID_USAGE2(HUP_BUTTON, (i + 1)),
-		    hid_input, &sc->sc_loc_btn[i], NULL)) {
+			hid_input, &sc->sc_loc_btn[i], NULL, &sc->sc_iid)) {
 			break;
 		}
 	}
 
 	sc->sc_buttons = i;
 
-	isize = hid_report_size(d_ptr, d_len, hid_input, &sc->sc_iid);
+	isize = hid_report_size(d_ptr, d_len, hid_input, NULL);
 
 	/*
 	 * The Microsoft Wireless Notebook Optical Mouse seems to be in worse
@@ -533,27 +503,23 @@ ums_attach(device_t dev)
 		sc->sc_loc_btn[1].pos = 9;
 		sc->sc_loc_btn[2].pos = 10;
 	}
+
 	/*
-	 * The Microsoft Wireless Notebook Optical Mouse 3000 Model 1049 has
-	 * five Report IDs: 19 23 24 17 18 (in the order they appear in report
-	 * descriptor), it seems that report id 17 contains the necessary
-	 * mouse information(3-buttons,X,Y,wheel) so we specify it manually.
+	 * Some Microsoft devices have incorrectly high location
+	 * positions. Correct this:
 	 */
-	if ((uaa->info.idVendor == USB_VENDOR_MICROSOFT) &&
-	    (uaa->info.idProduct == USB_PRODUCT_MICROSOFT_WLNOTEBOOK3)) {
-		sc->sc_flags = (UMS_FLAG_X_AXIS |
-		    UMS_FLAG_Y_AXIS |
-		    UMS_FLAG_Z_AXIS);
-		sc->sc_buttons = 3;
-		isize = 5;
-		sc->sc_iid = 17;
-		sc->sc_loc_x.pos = 8;
-		sc->sc_loc_y.pos = 16;
-		sc->sc_loc_z.pos = 24;
-		sc->sc_loc_btn[0].pos = 0;
-		sc->sc_loc_btn[1].pos = 1;
-		sc->sc_loc_btn[2].pos = 2;
+	isizebits = isize * 8;
+	if ((sc->sc_iid != 0) && (isizebits > 8)) {
+		isizebits -= 8;	/* remove size of report ID */
+		sc->sc_loc_w.pos %= isizebits;
+		sc->sc_loc_x.pos %= isizebits;
+		sc->sc_loc_y.pos %= isizebits;
+		sc->sc_loc_z.pos %= isizebits;
+		sc->sc_loc_t.pos %= isizebits;
+		for (i = 0; i != UMS_BUTTON_MAX; i++)
+			sc->sc_loc_btn[i].pos %= isizebits;
 	}
+
 	if (usb2_test_quirk(uaa, UQ_MS_REVZ)) {
 		/* Some wheels need the Z axis reversed. */
 		sc->sc_flags |= UMS_FLAG_REVZ;
@@ -668,7 +634,6 @@ ums_stop_read(struct usb2_fifo *fifo)
 {
 	struct ums_softc *sc = fifo->priv_sc0;
 
-	usb2_transfer_stop(sc->sc_xfer[UMS_INTR_CS]);
 	usb2_transfer_stop(sc->sc_xfer[UMS_INTR_DT]);
 	usb2_callout_stop(&sc->sc_callout);
 }
diff --git a/sys/dev/usb/quirk/usb_quirk.c b/sys/dev/usb/quirk/usb_quirk.c
index fe49ec7..bc1b527 100644
--- a/sys/dev/usb/quirk/usb_quirk.c
+++ b/sys/dev/usb/quirk/usb_quirk.c
@@ -105,10 +105,7 @@ static struct usb2_quirk_entry usb2_quirks[USB_DEV_QUIRKS_MAX] = {
 	{USB_QUIRK_ENTRY(USB_VENDOR_METAGEEK, USB_PRODUCT_METAGEEK_WISPY1B, 0x0000, 0xFFFF, UQ_KBD_IGNORE, UQ_HID_IGNORE, UQ_NONE)},
 	{USB_QUIRK_ENTRY(USB_VENDOR_TENX, USB_PRODUCT_TENX_UAUDIO0, 0x0101, 0x0101, UQ_AUDIO_SWAP_LR, UQ_NONE)},
 	/* MS keyboards do weird things */
-	{USB_QUIRK_ENTRY(USB_VENDOR_MICROSOFT, USB_PRODUCT_MICROSOFT_WLNOTEBOOK, 0x0000, 0xFFFF, UQ_MS_BAD_CLASS, UQ_MS_LEADING_BYTE, UQ_NONE)},
-	{USB_QUIRK_ENTRY(USB_VENDOR_MICROSOFT, USB_PRODUCT_MICROSOFT_WLNOTEBOOK2, 0x0000, 0xFFFF, UQ_MS_BAD_CLASS, UQ_MS_LEADING_BYTE, UQ_NONE)},
 	{USB_QUIRK_ENTRY(USB_VENDOR_MICROSOFT, USB_PRODUCT_MICROSOFT_WLINTELLIMOUSE, 0x0000, 0xFFFF, UQ_MS_LEADING_BYTE, UQ_NONE)},
-	{USB_QUIRK_ENTRY(USB_VENDOR_MICROSOFT, USB_PRODUCT_MICROSOFT_COMFORT3000, 0x0000, 0xFFFF, UQ_MS_BAD_CLASS, UQ_MS_LEADING_BYTE, UQ_NONE)},
 	{USB_QUIRK_ENTRY(USB_VENDOR_METAGEEK, USB_PRODUCT_METAGEEK_WISPY24X, 0x0000, 0xFFFF, UQ_KBD_IGNORE, UQ_HID_IGNORE, UQ_NONE)},
 };
 
diff --git a/sys/dev/usb/usb_hid.c b/sys/dev/usb/usb_hid.c
index 2adf208..75da249 100644
--- a/sys/dev/usb/usb_hid.c
+++ b/sys/dev/usb/usb_hid.c
@@ -155,7 +155,7 @@ top:
 	}
 	for (;;) {
 		p = s->p;
-		if (p >= s->end)
+		if ((p >= s->end) || (p < s->start))
 			return (0);
 
 		bSize = *p++;
@@ -388,32 +388,53 @@ top:
  *	hid_report_size
  *------------------------------------------------------------------------*/
 int
-hid_report_size(const void *buf, int len, enum hid_kind k, uint8_t *idp)
+hid_report_size(const void *buf, int len, enum hid_kind k, uint8_t *id)
 {
 	struct hid_data *d;
 	struct hid_item h;
-	int hi, lo, size, id;
+	uint32_t temp;
+	uint32_t hpos;
+	uint32_t lpos;
+	uint8_t any_id;
 
-	id = 0;
-	hi = lo = -1;
-	for (d = hid_start_parse(buf, len, 1 << k); hid_get_item(d, &h);)
+	any_id = 0;
+	hpos = 0;
+	lpos = 0xFFFFFFFF;
+
+	for (d = hid_start_parse(buf, len, 1 << k); hid_get_item(d, &h);) {
 		if (h.kind == k) {
-			if (h.report_ID != 0 && !id)
-				id = h.report_ID;
-			if (h.report_ID == id) {
-				if (lo < 0)
-					lo = h.loc.pos;
-				hi = h.loc.pos + h.loc.size * h.loc.count;
+			/* check for ID-byte presense */
+			if ((h.report_ID != 0) && !any_id) {
+				if (id != NULL)
+					*id = h.report_ID;
+				any_id = 1;
 			}
+			/* compute minimum */
+			if (lpos > h.loc.pos)
+				lpos = h.loc.pos;
+			/* compute end position */
+			temp = h.loc.pos + (h.loc.size * h.loc.count);
+			/* compute maximum */
+			if (hpos < temp)
+				hpos = temp;
 		}
+	}
 	hid_end_parse(d);
-	size = hi - lo;
-	if (id != 0) {
-		size += 8;
-		*idp = id;		/* XXX wrong */
-	} else
-		*idp = 0;
-	return ((size + 7) / 8);
+
+	/* safety check - can happen in case of currupt descriptors */
+	if (lpos > hpos)
+		temp = 0;
+	else
+		temp = hpos - lpos;
+
+	/* check for ID byte */
+	if (any_id)
+		temp += 8;
+	else if (id != NULL)
+		*id = 0;
+
+	/* return length in bytes rounded up */
+	return ((temp + 7) / 8);
 }
 
 /*------------------------------------------------------------------------*
@@ -421,7 +442,7 @@ hid_report_size(const void *buf, int len, enum hid_kind k, uint8_t *idp)
  *------------------------------------------------------------------------*/
 int
 hid_locate(const void *desc, int size, uint32_t u, enum hid_kind k,
-    struct hid_location *loc, uint32_t *flags)
+    struct hid_location *loc, uint32_t *flags, uint8_t *id)
 {
 	struct hid_data *d;
 	struct hid_item h;
@@ -432,12 +453,19 @@ hid_locate(const void *desc, int size, uint32_t u, enum hid_kind k,
 				*loc = h.loc;
 			if (flags != NULL)
 				*flags = h.flags;
+			if (id != NULL)
+				*id = h.report_ID;
 			hid_end_parse(d);
 			return (1);
 		}
 	}
+	if (loc != NULL)
+		loc->size = 0;
+	if (flags != NULL)
+		*flags = 0;
+	if (id != NULL)
+		*id = 0;
 	hid_end_parse(d);
-	loc->size = 0;
 	return (0);
 }
 
@@ -450,26 +478,35 @@ hid_get_data(const uint8_t *buf, uint32_t len, struct hid_location *loc)
 	uint32_t hpos = loc->pos;
 	uint32_t hsize = loc->size;
 	uint32_t data;
-	int i, s, t;
+	uint32_t rpos;
+	uint8_t n;
 
 	DPRINTFN(11, "hid_get_data: loc %d/%d\n", hpos, hsize);
 
+	/* Range check and limit */
 	if (hsize == 0)
 		return (0);
+	if (hsize > 32)
+		hsize = 32;
 
+	/* Get data in a safe way */	
 	data = 0;
-	s = hpos / 8;
-	for (i = hpos; i < (hpos + hsize); i += 8) {
-		t = (i / 8);
-		if (t < len) {
-			data |= buf[t] << ((t - s) * 8);
-		}
+	rpos = (hpos / 8);
+	n = (hsize + 7) / 8;
+	rpos += n;
+	while (n--) {
+		rpos--;
+		if (rpos < len)
+			data |= buf[rpos] << (8 * n);
 	}
-	data >>= hpos % 8;
-	data &= (1 << hsize) - 1;
-	hsize = 32 - hsize;
-	/* Sign extend */
-	data = ((int32_t)data << hsize) >> hsize;
+
+	/* Correctly shift down data */
+	data = (data >> (hpos % 8));
+
+	/* Mask and sign extend in one */
+	n = 32 - hsize;
+	data = ((int32_t)data << n) >> n;
+
 	DPRINTFN(11, "hid_get_data: loc %d/%d = %lu\n",
 	    loc->pos, loc->size, (long)data);
 	return (data);
diff --git a/sys/dev/usb/usb_hid.h b/sys/dev/usb/usb_hid.h
index 3fb7368..7ba858d 100644
--- a/sys/dev/usb/usb_hid.h
+++ b/sys/dev/usb/usb_hid.h
@@ -79,9 +79,11 @@ struct hid_item {
 struct hid_data *hid_start_parse(const void *d, int len, int kindset);
 void	hid_end_parse(struct hid_data *s);
 int	hid_get_item(struct hid_data *s, struct hid_item *h);
-int	hid_report_size(const void *buf, int len, enum hid_kind k, uint8_t *id);
+int	hid_report_size(const void *buf, int len, enum hid_kind k,
+	    uint8_t *id);
 int	hid_locate(const void *desc, int size, uint32_t usage,
-	    enum hid_kind kind, struct hid_location *loc, uint32_t *flags);
+	    enum hid_kind kind, struct hid_location *loc,
+	    uint32_t *flags, uint8_t *id);
 uint32_t hid_get_data(const uint8_t *buf, uint32_t len,
 	    struct hid_location *loc);
 int	hid_is_collection(const void *desc, int size, uint32_t usage);