Re: xhci spec (mis?)interpretation on hubs

[Nick Hudson <skrll%netbsd.org@localhost>]

On 12/17/17 10:40, Nick Hudson wrote:
> On 12/08/17 18:25, Sprow wrote:
> > Hi,
> > I've been having a look on a USB analyser at a class of hubs that 
> > don't seem
> > to work on a driver based on
> >    src/sys/dev/usb/xhci.c
> >
> > Comparing the dialogue for a non XHCI controller I see the issue is 
> > that the
> > cascade of hubs is HS->FS->LS. The GetDescriptor of the LS device 
> > never makes
> > it through. It happens to be a KVM which presents a fake 
> > keyboard/mouse HID
> > plugged into port 3 of a 4 port (FS) hub, which I've then plugged 
> > into a HS
> > hub, which is itself attached to the root hub (ie. the XHCI root hub 
> > is in
> > USB2 mode).
Can you try this patch?

Thanks,
Nick

Index: sys/dev/usb/xhci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/xhci.c,v
retrieving revision 1.82
diff -u -p -r1.82 xhci.c
--- sys/dev/usb/xhci.c	19 Dec 2017 16:04:27 -0000	1.82
+++ sys/dev/usb/xhci.c	19 Dec 2017 16:08:27 -0000
@@ -3138,6 +3138,7 @@ xhci_setup_tthub(struct usbd_pipe *pipe,
 	struct usbd_port *myhsport = dev->ud_myhsport;
 	usb_device_descriptor_t * const dd = &dev->ud_ddesc;
 	uint32_t speed = dev->ud_speed;
+	uint8_t rhaddr = dev->ud_bus->ub_rhaddr;
 	uint8_t tthubslot, ttportnum;
 	bool ishub;
 	bool usemtt;
@@ -3159,8 +3160,8 @@ xhci_setup_tthub(struct usbd_pipe *pipe,
 	 *   parent hub is not HS hub ||
 	 *   attached to root hub.
 	 */
-	if (myhsport && myhub && myhub->ud_depth &&
-	    myhub->ud_speed == USB_SPEED_HIGH &&
+	if (myhsport &&
+	    myhsport->up_parent->ud_addr != rhaddr &&
 	    (speed == USB_SPEED_LOW || speed == USB_SPEED_FULL)) {
 		ttportnum = myhsport->up_portno;
 		tthubslot = myhsport->up_parent->ud_addr;
@@ -3185,29 +3186,30 @@ xhci_setup_tthub(struct usbd_pipe *pipe,
 		DPRINTFN(4, "nports=%jd ttt=%jd", hd->bNbrPorts, ttt, 0, 0);
 	}
 
-#define IS_TTHUB(dd) \
-    ((dd)->bDeviceProtocol == UDPROTO_HSHUBSTT || \
+#define IS_MTTHUB(dd) \
      (dd)->bDeviceProtocol == UDPROTO_HSHUBMTT)
 
 	/*
 	 * MTT flag is set if
-	 * 1. this is HS hub && MTT is enabled
-	 *  or
-	 * 2. this is not hub && this is LS or FS device &&
-	 *    MTT of parent HS hub (and its parent, too) is enabled
+	 * 1. this is HS hub && MTTs are supported and enabled;  or
+	 * 2. this is LS or FS device && there is a parent HS hub where MTTs
+	 *    are supported and enabled.
+	 *
+	 * XXX enabled is not tested yet
 	 */
-	if (ishub && speed == USB_SPEED_HIGH && IS_TTHUB(dd))
+	if (ishub && speed == USB_SPEED_HIGH && IS_MTTHUB(dd))
 		usemtt = true;
-	else if (!ishub && (speed == USB_SPEED_LOW || speed == USB_SPEED_FULL) &&
-	     myhub && myhub->ud_depth && myhub->ud_speed == USB_SPEED_HIGH &&
-	     myhsport && IS_TTHUB(&myhsport->up_parent->ud_ddesc))
+	else if ((speed == USB_SPEED_LOW || speed == USB_SPEED_FULL) &&
+	    myhsport &&
+	    myhsport->up_parent->ud_addr != rhaddr &&
+	    IS_MTTHUB(&myhsport->up_parent->ud_ddesc))
 		usemtt = true;
 	else
 		usemtt = false;
 	DPRINTFN(4, "class %ju proto %ju ishub %jd usemtt %jd",
 	    dd->bDeviceClass, dd->bDeviceProtocol, ishub, usemtt);
 
-#undef IS_TTHUB
+#undef IS_MTTHUB
 
 	cp[0] |=
 	    XHCI_SCTX_0_HUB_SET(ishub ? 1 : 0) |