xhci spec (mis?)interpretation on hubs

[Sprow <webpages%sprow.co.uk@localhost>]

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).

Looking in the XHCI spec I think I see the problem. The text in table 59 is
badly worded, it says
  "If this device is LS/FS and connected through a HS hub, then this field
   contains...of the parent HS hub"
   
The 'parent' isn't the immediately adjacent (as in parent/child relationship)
hub, I think the operative word is *through*, and it really means the first
ancestor which is HS, but not necessarily adjacent. The footnote (a) sort of
says that, by defining the 'parent' as the point where there's a transition
from HS to FS/LS signalling.

For example, with
  HS(1)->HS(2)->FS(3)->FS(4)->FS(5)->LS
it would be hub 2, not 5.

Looking at the driver we have:

	if (myhsport && myhub && myhub->ud_depth &&
	    myhub->ud_speed == USB_SPEED_HIGH &&
	    (speed == USB_SPEED_LOW || speed == USB_SPEED_FULL)) {
		ttportnum = myhsport->up_portno;
		tthubslot = myhsport->up_parent->ud_addr;
	} else {
		ttportnum = 0;
		tthubslot = 0;
	}

The check of myhub->ud_speed looks wrong, because that's checking the
adjacent hub (5 in my example). As a result the expression evaluates to false
and the ttportnum & tthubslot are 0, which blocks the controller from routing
the GetDescriptor properly.

I believe it should be checking the one that myhsport is on, but since
myhsport is implicitly high speed, that'd be redundant. The same construct
appears around line 3222 too.

The FreeBSD folks seem to have interpreted the spec that way:

https://svnweb.freebsd.org/base/head/sys/dev/usb/usb_device.c?revision=326255&view=markup#l1644

they walk up the tree of devices until the first HS hub is encountered and
set parent_hs_hub if so

https://svnweb.freebsd.org/base/head/sys/dev/usb/controller/xhci.c?revision=326255&view=markup#l2676

for FS/LS devices they fill in the TT fields from parent_hs_hub regardless of
what type the adjacent hub is.

Rather than opening a PR straight away, does anyone else happen to have kit
to do a HS->FS->LS cascade to check that the LS device is not enumerated?
Does removing the test on myhub->ud_speed then make it work? It does for me,
Sprow.