hathach / tinyusb

An open source cross-platform USB stack for embedded system
https://www.tinyusb.org
MIT License
4.66k stars 997 forks source link

USB Host: No set address recovery interval #2673

Open Ryzee119 opened 2 weeks ago

Ryzee119 commented 2 weeks ago

Operating System

Windows 11

Board

N/A

Firmware

N/A

What happened ?

Ref USB Spec 9.2.6.3

In the case of the SetAddress() request, the Status stage successfully completes when the device sends the zero-length Status packet or when the device sees the ACK in response to the Status stage data packet. After successful completion of the Status stage, the device is allowed a SetAddress() recovery interval of 2 ms

I have a device (HID game controller) that will STALL on the first request after the set address command is issued without this delay.

The enumeration is a bit of a complex state machine so didnt know the best way to include it. I just did this

     case ENUM_GET_DEVICE_DESC: {
+
+      // Wait 2ms for address recovery time, Ref USB Spec 9.2.6.3
+      osal_task_delay(2);

How to reproduce ?

Very dependant on your USB device. But I just plugged it in and got STALL error on first command after the set address.

Debug Log as txt file (LOG/CFG_TUSB_DEBUG=2)

N/A

Screenshots

No response

I have checked existing issues, dicussion and documentation

hathach commented 2 weeks ago

this is interesting, I overlook the specs on this. Blindly wait for 2 seconds isn't good though, would your device still response after a few seconds after stalled response ? I am thinking we can allow the control transfer to retry up to 3 seconds right after SET_ADDRESS e.g. increase the delay to 1000 ms if the first descritptor isn't retrieved after addressed https://github.com/hathach/tinyusb/blob/master/src/host/usbh.c#L1314 ?

Ryzee119 commented 1 week ago

It is 2 milliseconds, not seconds so not too bad to wait on I don't think.

Your retry mechanism is good and it does recover from it 😊

hathach commented 1 week ago

ah sorry, indeed, I overlook it again, indeed 2 ms is OK :) . I think We can safely add that delay since it is part of the specs.