Hi Marcus, Marcus Glocker <mar...@nazgul.ch> writes:
> There are a few threads going on related to problems with ugen(4) and > uhidev(4) devices on xhci(4). This is related to the issue patrick@ > already explained; while ehci(4) can save the last data toggle state, > xhci(4) resets it on every open/close cycle, getting out of sync with > the device. Is this related to the terrible contortions we go through in src/lib/libfido2/src/hid_openbsd.c:terrible_ping_kludge? Same code is also included into firefox and our chromium to get fido(4) to work. Security keys work no better or worse with your patch. If you believe there's a chance to remove terrible_ping_kludge, I'll try. > This diff addresses a possible solution for ugen(4) and uhidev(4). > For ugen(4) we already did some positive testing related to scanner > devices, see e.g.: > > https://marc.info/?l=openbsd-bugs&m=161056827312265&w=2 > > I would appreciate some regression testing and feedback. On xhci(4) > it might resolve issues related to that, and on ehci(4) it shouldn't > break your existing setup. I tested with xhci. Other than this message I see no changes in behavior: uhidev_clear_iface_eps: clear endpoints failed 3! uhidev_clear_iface_eps: clear endpoints failed 3! uhidev_clear_iface_eps: clear endpoints failed 3! To make this a little more useful, I applied a bit more diagnostics on top of your patch: diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c index 4ddf9425170..6296684fa8d 100644 --- a/sys/dev/usb/uhidev.c +++ b/sys/dev/usb/uhidev.c @@ -966,6 +966,7 @@ uhidev_clear_iface_eps(struct uhidev_softc *sc, struct usbd_interface *iface) usb_endpoint_descriptor_t *ed; uint8_t xfertype; int i; + int reason = 0; /* Only clear interface endpoints when none are in use. */ if (sc->sc_ipipe || sc->sc_opipe) @@ -973,22 +974,28 @@ uhidev_clear_iface_eps(struct uhidev_softc *sc, struct usbd_interface *iface) DPRINTFN(1,("%s: clear interface eps\n", __func__)); id = usbd_get_interface_descriptor(iface); - if (id == NULL) + if (id == NULL) { + reason = 1; goto bad; + } for (i = 0; i < id->bNumEndpoints; i++) { ed = usbd_interface2endpoint_descriptor(iface, i); - if (ed == NULL) + if (ed == NULL) { + reason = 2; goto bad; + } xfertype = UE_GET_XFERTYPE(ed->bmAttributes); if (xfertype == UE_BULK || xfertype == UE_INTERRUPT) { if (usbd_clear_endpoint_feature(sc->sc_udev, - ed->bEndpointAddress, UF_ENDPOINT_HALT)) + ed->bEndpointAddress, UF_ENDPOINT_HALT)) { + reason = 3; goto bad; + } } } return; bad: - printf("%s: clear endpoints failed!\n", __func__); + printf("%s: clear endpoints failed %d!\n", __func__, reason); } dmesg with your original patch: xhci0 at pci0 dev 20 function 0 "Intel 8 Series xHCI" rev 0x04: msi, xHCI 1.0 usb0 at xhci0: USB revision 3.0 uhub0 at usb0 configuration 1 interface 0 "Intel xHCI root hub" rev 3.00/1.00 addr 1 usb1 at ehci0: USB revision 2.0 uhub1 at usb1 configuration 1 interface 0 "Intel EHCI root hub" rev 2.00/1.00 addr 1 uhidev0 at uhub3 port 3 configuration 1 interface 0 "Microsoft Microsoft 3-Button Mouse with IntelliEye(TM)" rev 1.10/3.00 addr 5 uhidev0: iclass 3/1 ums0 at uhidev0: 3 buttons, Z dir uhidev1 at uhub3 port 1 configuration 1 interface 0 "Google Inc. hg_0004" rev 2.00/1.00 addr 9 uhidev1: iclass 3/0 fido0 at uhidev1: input=64, output=64, feature=0 uhidev2 at uhub3 port 4 configuration 1 interface 0 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 10 uhidev2: iclass 3/1 ums1 at uhidev2: 3 buttons, Z dir uhidev3 at uhub3 port 4 configuration 1 interface 1 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 10 uhidev3: iclass 3/1 ukbd0 at uhidev3: 8 variable keys, 6 key codes uhidev4 at uhub3 port 4 configuration 1 interface 2 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 10 uhidev4: iclass 3/0, 2 report ids uhid0 at uhidev4 reportid 1: input=1, output=0, feature=0 uhid1 at uhidev4 reportid 2: input=1, output=0, feature=0 uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! uhidev5 at uhub0 port 2 configuration 1 interface 0 "Yubico Security Key by Yubico" rev 2.00/3.33 addr 11 uhidev5: iclass 3/0 fido1 at uhidev5: input=64, output=64, feature=0 fido1 detached uhidev5 detached uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! uhidev5 at uhub0 port 2 configuration 1 interface 0 "Yubico Security Key by Yubico" rev 2.00/3.33 addr 11 uhidev5: iclass 3/0 fido1 at uhidev5: input=64, output=64, feature=0 fido1 detached uhidev5 detached uhidev5 at uhub0 port 2 configuration 1 interface 0 "Yubico Security Key by Yubico" rev 2.00/3.33 addr 11 uhidev5: iclass 3/0 fido1 at uhidev5: input=64, output=64, feature=0 uhidev_clear_iface_eps: clear endpoints failed! fido1 detached uhidev5 detached uhidev5 at uhub0 port 2 configuration 1 interface 0 "Yubico Security Key by Yubico" rev 2.00/3.33 addr 11 uhidev5: iclass 3/0 fido1 at uhidev5: input=64, output=64, feature=0 fido1 detached uhidev5 detached uhidev5 at uhub0 port 2 configuration 1 interface 0 "Yubico Security Key by Yubico" rev 2.00/3.33 addr 11 uhidev5: iclass 3/0 fido1 at uhidev5: input=64, output=64, feature=0 uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! fido1 detached uhidev5 detached uhidev5 at uhub0 port 2 configuration 1 interface 0 "Yubico Security Key by Yubico" rev 2.00/3.33 addr 11 uhidev5: iclass 3/0 fido1 at uhidev5: input=64, output=64, feature=0 uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! uhidev_clear_iface_eps: clear endpoints failed! fido1 detached uhidev5 detached uhidev_clear_iface_eps: clear endpoints failed! uhidev5 at uhub0 port 2 configuration 1 interface 0 "Yubico Security Key by Yubico" rev 2.00/3.33 addr 11 uhidev5: iclass 3/0 fido1 at uhidev5: input=64, output=64, feature=0
