From owner-p4-projects@FreeBSD.ORG Tue Feb 17 21:14:07 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 9BEEA1065679; Tue, 17 Feb 2009 21:14:07 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 575CA106568D for ; Tue, 17 Feb 2009 21:14:07 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 4436F8FC22 for ; Tue, 17 Feb 2009 21:14:07 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n1HLE7O0017451 for ; Tue, 17 Feb 2009 21:14:07 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n1HLE7bq017449 for perforce@freebsd.org; Tue, 17 Feb 2009 21:14:07 GMT (envelope-from hselasky@FreeBSD.org) Date: Tue, 17 Feb 2009 21:14:07 GMT Message-Id: <200902172114.n1HLE7bq017449@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky To: Perforce Change Reviews Cc: Subject: PERFORCE change 157863 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Feb 2009 21:14:09 -0000 http://perforce.freebsd.org/chv.cgi?CH=157863 Change 157863 by hselasky@hselasky_laptop001 on 2009/02/17 21:13:56 USB HID support: - 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. Affected files ... .. //depot/projects/usb/src/sys/dev/usb2/core/usb2_hid.c#10 edit .. //depot/projects/usb/src/sys/dev/usb2/core/usb2_hid.h#10 edit .. //depot/projects/usb/src/sys/dev/usb2/input/ums2.c#15 edit .. //depot/projects/usb/src/sys/dev/usb2/quirk/usb2_quirk.c#15 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_hid.c#10 (text+ko) ==== @@ -155,7 +155,7 @@ } for (;;) { p = s->p; - if (p >= s->end) + if ((p >= s->end) || (p < s->start)) return (0); bSize = *p++; @@ -388,48 +388,53 @@ * 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; - uint32_t size; - uint32_t hi; - uint32_t lo; - uint8_t id; + uint32_t temp; + uint32_t hpos; + uint32_t lpos; + uint8_t any_id; + + any_id = 0; + hpos = 0; + lpos = 0xFFFFFFFF; - id = 0; - hi = 0; - lo = (uint32_t)(0-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) { - /* check if "lo" is greater than "pos" */ - if (lo > h.loc.pos) - lo = h.loc.pos; - /* compute end position */ - size = h.loc.pos + (h.loc.size * h.loc.count); - /* check if "size" wrapped */ - /* check if "hi" is less than "size" */ - if (size < h.loc.pos) - hi = (uint32_t)(0-1); - else if (hi < size) - hi = size; + /* 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); - if (lo > hi) - size = 0; + + /* safety check - can happen in case of currupt descriptors */ + if (lpos > hpos) + temp = 0; else - size = hi - lo; + temp = hpos - lpos; + + /* check for ID byte */ + if (any_id) + temp += 8; + else if (id != NULL) + *id = 0; - if (id != 0) { - size += 8; - *idp = id; /* XXX wrong */ - } else - *idp = 0; - return ((size + 7) / 8); + /* return length in bytes rounded up */ + return ((temp + 7) / 8); } /*------------------------------------------------------------------------* @@ -437,7 +442,7 @@ *------------------------------------------------------------------------*/ 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; @@ -448,12 +453,19 @@ *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); } @@ -466,26 +478,35 @@ 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); ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_hid.h#10 (text+ko) ==== @@ -79,9 +79,11 @@ 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); ==== //depot/projects/usb/src/sys/dev/usb2/input/ums2.c#15 (text+ko) ==== @@ -89,8 +89,7 @@ enum { UMS_INTR_DT, - UMS_INTR_CS, - UMS_N_TRANSFER = 2, + UMS_N_TRANSFER, }; struct ums_softc { @@ -115,9 +114,8 @@ #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; @@ -126,7 +124,6 @@ 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; @@ -161,19 +158,6 @@ } 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; @@ -196,9 +180,9 @@ 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 " @@ -209,21 +193,23 @@ (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. */ @@ -289,12 +275,14 @@ */ /* - * 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) && @@ -311,25 +299,21 @@ } 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; } } @@ -343,16 +327,6 @@ .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 @@ -361,39 +335,34 @@ 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); @@ -406,9 +375,10 @@ 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; @@ -445,14 +415,14 @@ 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; @@ -460,9 +430,9 @@ } /* 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; } @@ -471,14 +441,14 @@ * 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; @@ -492,7 +462,7 @@ * 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; @@ -504,14 +474,14 @@ 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 @@ -535,27 +505,22 @@ 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 (isizebits <= 0) { + 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; @@ -670,7 +635,6 @@ { 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); } ==== //depot/projects/usb/src/sys/dev/usb2/quirk/usb2_quirk.c#15 (text+ko) ==== @@ -105,10 +105,7 @@ {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)}, };