On Fri, Aug 07, 2015 at 07:56:37PM +0300, Tal Shorer wrote: > 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 People use SetHubDepth today. > 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. But again, GetPortErrorCount is being used. > Would you prefer to eliminate these completely or make them right in > case anyone uses them in the future? We should fix these if they were incorrect. Care to make up a patch? thanks, greg k-h -- 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