From: Yasushi Asano <yasano@xxxxxxxxxxxxxx> Dear Alan Dear Greg I would like to have a consultation with you. The customer's product board needs to get a USB certification and compliance, but it can not pass the test using the current USB hub driver. According to the USB compliance plan[1], the“6.7.22 Device No Response” test requires the detection of device enumeration failure within 30 seconds. The kernel(USB hub driver) must notify userspace of the enumeration failure within 30 seconds. In the current hub driver, the place to detect device enumeration failure is [2]. I have modified the hub driver to issue a uevent here, but the procedure of device enumeration (new_scheme+old_scheme) takes too long to execute, it couldn't reach [2] within 30 seconds after starting the test. According to the result of PETtool [3], the state of "Device No response" is that usb_control_msg(USB_REQ_GET_DESCRIPTOR) [4] in hub_port_initn times out. That means r == -ETIMEDOUT. because of the default setting of initial_descriptor_timeout is 5000 msec[5], usb_control_msg() took 5000msec until -ETIMEDOUT. I will try to describe the flow of device enumeration processing below[6]. If my understanding is correct, the enumeration failure [2] will be reached after performing all the loops of [7][8][9]+[7][10]. In the new scheme, 12 times check will be performed ([7]/2*[8]*[9] => 2*2*3 => 12). In the old scheme , 4 times check will be performed ([7]/2*[10] => 2*2 => 4). In total, it checks 16 times, and it took 5000msec to ETIMEDOUT every time. Therefore, It takes about 80 seconds(16*5000msec) to reach [2]. This does not pass the "Device No response" test. If I set the module parameter old_schene_first=Y and use_both_schemes=N, it can be reached [2] within 30 seconds. However, the new_scheme will no longer be performed. I think we can't choose this, as previously-detected devices may become undetectable. new_scheme is taking too long and I think 12 times checks are redundant. So, I confirmed the USB specification. This is the only description of the read of the device descriptor[12]. I couldn't find the description related retry count or timing here and anywhere in this specification. And I couldn't find the description related timing in the “No silent failures" requirements in other documents[13] also. Because I'm not sure where this number of loop count is decided, I'm not sure how should it be modified the driver to pass the USB compliance test. Is it possible for me to receive a proposal for a solution? As my thought, the number of loops may be redundant, so I think if the user can change it arbitrarily, the problem will be solved. Currently, the timeout value can be adjusted with a module parameter, but the loop count cannot. Is there any problem if it is changed like this patch? The original handling of the driver is unchanged by this modification. I think the user will be able to adjust the time to enumeration failure freely. Is this patch acceptable? I would appreciate it much if I could receive the feedback from you. ---------------------------------------------------------------------------------------------------------------------------------------- [1] https://www.usb.org/sites/default/files/otgeh_compliance_plan_1_2.pdf 6.7.22 A-UUT “Device No Response” for connection timeout 6.7.22.2 Test Procedure for A-UUT which does not support sessions - 5. If operator clicks OK before 30s elapses since VBUS went on, then UUT passes test. - 6. If 30s elapses first, then UUT fails test. ---------------------------------------------------------------------------------------------------------------------------------------- [2] hub_port_connect() if (hub->hdev->parent || !hcd->driver->port_handed_over || !(hcd->driver->port_handed_over)(hcd, port1)) { if (status != -ENOTCONN && status != -ENODEV) dev_err(&port_dev->dev, "unable to enumerate USB device\n"); } ---------------------------------------------------------------------------------------------------------------------------------------- [3] http://www.mqp.com/usbcomp.htm ---------------------------------------------------------------------------------------------------------------------------------------- [4] hub_port_init() r = usb_control_msg(udev, usb_rcvaddr0pipe(), USB_REQ_GET_DESCRIPTOR, USB_DIR_IN, USB_DT_DEVICE << 8, 0, buf, GET_DESCRIPTOR_BUFSIZE, initial_descriptor_timeout); ---------------------------------------------------------------------------------------------------------------------------------------- [5] static int initial_descriptor_timeout = USB_CTRL_GET_TIMEOUT; include/linux/usb.h:#define USB_CTRL_GET_TIMEOUT 5000 ---------------------------------------------------------------------------------------------------------------------------------------- [6] The flow of device enumeration processing drivers/usb/core/hub.c hub_port_connect(){ for (x = 0; x < SET_CONFIG_TRIES; x++) { ...[7] SET_CONFIG_TRIES is 4 as default setting hub_port_init(){ if( x < 2 ) { ------ new scheme ------ for ( y= 0; y < 2; ++y ) { ...[8] 2==GET_DESCRIPTOR_TRIES for ( z = 0; z < 3; ++z ) { ...[9] usb_control_msg() ...[3] ETIMEDOUT(-110) is detected. Timieout value=5000msec. } } } else { ------ old scheme ------ for ( y= 0; y < 2; ++y ) { ...[10] 2==SET_ADDRESS_TRIES hub_set_address() ...[11] ETIMEDOUT(-110) is detected. Timieout value=5000msec. } } } } unable to enumerate USB device. ...[2] } ---------------------------------------------------------------------------------------------------------------------------------------- [12] https://www.usb.org/document-library/usb-20-specification Universal Serial Bus Specification (usb_20.pdf) 9.1.2 Bus Enumeration 6. Before the USB device receives a unique address, its Default Control Pipe is still accessible via the default address. The host reads the device descriptor to determine what actual maximum data payload size this USB device's default pipe can use. ---------------------------------------------------------------------------------------------------------------------------------------- [13] https://www.usb.org/document-library/usb-20-specification On-The-Go and Embedded Host Supplement to the USB Revision 2.0 Specification (USB_OTG_and_EH_2-0-version 1_1a.pdf) 3.5 No Silent Failures ---------------------------------------------------------------------------------------------------------------------------------------- Signed-off-by: Yasushi Asano <yasano@xxxxxxxxxxxxxx> --- drivers/usb/core/hub.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 052d5ac..01c2b2d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -99,6 +99,18 @@ MODULE_PARM_DESC(use_both_schemes, "try the other device initialization scheme if the " "first one fails"); +static int get_descriptor_tries = 2; +module_param(get_descriptor_tries, int, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(get_descriptor_tries, + "The number of a 64-byte GET_DESCRIPTOR request tries " + "(default 2)"); + +static int get_descriptor_operations = 3; +module_param(get_descriptor_operations, int, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(get_descriptor_operations, + "The number of a 64-byte GET_DESCRIPTOR operation " + "(default 3)"); + /* Mutual exclusion for EHCI CF initialization. This interferes with * port reset on some companion controllers. */ @@ -2707,7 +2719,8 @@ static unsigned hub_is_wusb(struct usb_hub *hub) #define PORT_RESET_TRIES 5 #define SET_ADDRESS_TRIES 2 -#define GET_DESCRIPTOR_TRIES 2 +#define GET_DESCRIPTOR_TRIES get_descriptor_tries +#define GET_DESCRIPTOR_OPERATIONS get_descriptor_operations #define SET_CONFIG_TRIES (2 * (use_both_schemes + 1)) #define USE_NEW_SCHEME(i, scheme) ((i) / 2 == (int)(scheme)) @@ -4684,7 +4697,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, * 255 is for WUSB devices, we actually need to use * 512 (WUSB1.0[4.8.1]). */ - for (operations = 0; operations < 3; ++operations) { + for (operations = 0; operations < GET_DESCRIPTOR_OPERATIONS; ++operations) { buf->bMaxPacketSize0 = 0; r = usb_control_msg(udev, usb_rcvaddr0pipe(), USB_REQ_GET_DESCRIPTOR, USB_DIR_IN, -- 2.7.4
