On Fri, Aug 7, 2015 at 7:37 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, Aug 07, 2015 at 12:49:39PM +0300, Tal Shorer wrote: >> In include/linux/usb/hcd.h, we have >> >> /* class requests from USB 3.0 hub spec, table 10-5 */ >> #define SetHubDepth (0x3000 | HUB_SET_DEPTH) >> #define GetPortErrorCount (0x8000 | HUB_GET_PORT_ERR_COUNT) >> >> However, from the usb 3.1 spec I downloaded at >> http://www.usb.org/developers/docs/, >> table 10-5 appears to be unrelated (Table 10-5. Enhanced SuperSpeed >> Hub Descriptor). > > Um, look at the comment you copied "3.0 hub spec", vs. the spec you > downloaded and referred to "3.1 spec". Things might have changed :) > Yeah, my bad. >> Table 10-7 (Table 10-7. Hub Class Requests) lists the two values as: >> Request bmRequestType bRequest wValue wIndex wLength Data >> SetHubDepth 00100000B SET_HUB_DEPTH Hub Depth Zero Zero None >> GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of >> Link Errors on this port >> >> It appears the correct bmRequestType values should be: >> #define SetHubDepth (0x2000 | HUB_SET_DEPTH) >> #define GetPortErrorCount (0xa300 | HUB_GET_PORT_ERR_COUNT) >> >> Which are the bmRequestType values of SetHubFeature and GetPortStatus >> (and friends). >> >> These lines were introduced in >> commit 0eadcc09203349b11ca477ec367079b23d32ab91 >> Author: Tatyana Brokhman <tlinder@xxxxxxxxxxxxxx> >> Date: Mon Nov 1 18:18:24 2010 +0200 >> >> usb: USB3.0 ch11 definitions >> >> Adding hub SuperSpeed usb definitions as defined by ch10 of the USB3.0 >> spec. >> >> Signed-off-by: Tatyana Brokhman <tlinder@xxxxxxxxxxxxxx> >> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx> >> >> The values are only used by dummy_hcd which does nothing with them and >> by max3421-hcd which returns an error. >> Can anyone clarify what I'm missing or if the values are actually wrong? > > Can you compare the 3.1 to the 3.0 spec and see if they changed things > here? > > thanks, > > greg k-h USB 3.0 spec download from http://www.usb.org/developers/docs/documents_archive/ Table 10-6. Hub Class Requests GetPortErrorCount 10000000B GET_PORT_ERR_COUNT Zero Port Two Number of Link Errors on this port SetHubDepth 00100000B SET_HUB_DEPTH Hub Depth Zero Zero None It appears it wasn't changed from 3.0 to 3.1. Looking at the code in hub.c, it appears root hubs are never called with SetHubDepth. 1049 if (hdev->parent && hub_is_superspeed(hdev)) { 1050 ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0), 1051 HUB_SET_DEPTH, USB_RT_HUB, 1052 hdev->level - 1, 0, NULL, 0, 1053 USB_CTRL_SET_TIMEOUT); There are no more uses of HUB_SET_DEPTH except for this one and the one in hcd.h tal@tal-H87-D3H:~/Dev/linux|0 $ git grep HUB_SET_DEPTH drivers/usb/core/hub.c: HUB_SET_DEPTH, USB_RT_HUB, include/linux/usb/hcd.h:#define SetHubDepth (0x3000 | HUB_SET_DEPTH) include/uapi/linux/usb/ch11.h:#define HUB_SET_DEPTH 12 And HUB_GET_PORT_ERR_COUNT is never used by the hub driver: tal@tal-H87-D3H:~/Dev/linux|1 $ git grep HUB_GET_PORT_ERR_COUNT include/linux/usb/hcd.h:#define GetPortErrorCount (0x8000 | HUB_GET_PORT_ERR_COUNT) include/uapi/linux/usb/ch11.h:#define HUB_GET_PORT_ERR_COUNT 13 So even if the constant was right, root hubs using it would never reach the code handling it. Would you prefer to eliminate these completely or make them right in case anyone uses them in the future? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html