On Fri, Aug 7, 2015 at 8:31 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > 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. Yes, but with the current implementation of hub.c, these uses will never run. I don't really think it's a good idea to delete these uses, since this feature might be implemented in the future. > >> 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? Sure, I'll send it shortly. > > 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
