Date: Tue, 24 Feb 2009 03:34:05 +0000 (UTC) From: Andrew Thompson <thompsa@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r188981 - in head/sys/dev/usb: . input quirk Message-ID: <200902240334.n1O3Y50B049405@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: thompsa Date: Tue Feb 24 03:34:05 2009 New Revision: 188981 URL: http://svn.freebsd.org/changeset/base/188981 Log: 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 Modified: head/sys/dev/usb/input/ums.c head/sys/dev/usb/quirk/usb_quirk.c head/sys/dev/usb/usb_hid.c head/sys/dev/usb/usb_hid.h Modified: head/sys/dev/usb/input/ums.c ============================================================================== --- head/sys/dev/usb/input/ums.c Tue Feb 24 01:16:40 2009 (r188980) +++ head/sys/dev/usb/input/ums.c Tue Feb 24 03:34:05 2009 (r188981) @@ -87,8 +87,7 @@ SYSCTL_INT(_hw_usb2_ums, OID_AUTO, debug 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_conf .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); } Modified: head/sys/dev/usb/quirk/usb_quirk.c ============================================================================== --- head/sys/dev/usb/quirk/usb_quirk.c Tue Feb 24 01:16:40 2009 (r188980) +++ head/sys/dev/usb/quirk/usb_quirk.c Tue Feb 24 03:34:05 2009 (r188981) @@ -105,10 +105,7 @@ static struct usb2_quirk_entry usb2_quir {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)}, }; Modified: head/sys/dev/usb/usb_hid.c ============================================================================== --- head/sys/dev/usb/usb_hid.c Tue Feb 24 01:16:40 2009 (r188980) +++ head/sys/dev/usb/usb_hid.c Tue Feb 24 03:34:05 2009 (r188981) @@ -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; + + any_id = 0; + hpos = 0; + lpos = 0xFFFFFFFF; - id = 0; - hi = lo = -1; - for (d = hid_start_parse(buf, len, 1 << k); hid_get_item(d, &h);) + 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 *------------------------------------------------------------------------*/ 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, u *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_ 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); Modified: head/sys/dev/usb/usb_hid.h ============================================================================== --- head/sys/dev/usb/usb_hid.h Tue Feb 24 01:16:40 2009 (r188980) +++ head/sys/dev/usb/usb_hid.h Tue Feb 24 03:34:05 2009 (r188981) @@ -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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200902240334.n1O3Y50B049405>