tinyusb
tinyusb copied to clipboard
When clearing an endpoint stall, reset DTOG
According to the USB spec, the DTOG of an endpoint should always be reset to DATA0 when receiving CLEAR_FEATURE(ENDPOINT_HALT), regardless of if the endpoint had been stalled (See citations in #1529).
I believe the attached changes should be made to USBD to facilitate this requirement.
However, some issues to discuss are:
- When stalling endpoints (even when not in response to a control request), do we need to purge the event queue from transfers on that endpoint?
- When SET_CONFIGURATION() is received with the active configuration, does DTOG need to be reset? Does the class driver need to be reset? (Currently the code treats SET_CONFIG(active config) as a NOOP.)
- Is there a better way to tell if an EP is a control endpoint, besides checking its address? (USB devices may have multiple control endpoints) Or perhaps we can remove the address check and unconditionally call usbd_edpt_stall?
- Will these changes cause deadlocks or extra bugs in existing class drivers? (e.g., class initiates OUT transfer, host clear(halt) on the EP, class doesn't react to the transaction being cancelled)? Though perhaps the class drivers would already not have properly handled these situations, so it wouldn't be any more broken than it is now?
- Or do we need to push this logic into class drivers?
- Are there error cases when a class driver should be able to force the endpoint to stay stalled, even when the host requests that it be cleared? (I don't need this at the moment, but this was mentioned in the USBTMC spec).
- Perhaps a simpler solution would be to make the
usbd_edpt_clear_stall
unconditionally clear the stall... but does that run into race conditions?
(If this is merged, then the workaround in #1531 can be removed from the USBTMC driver)
(I've force-pushed this branch with a revision to clean up handling of default control pipe.)
I've convinced myself that this patch removes some bugs, but we still need to examine how endpoint halts interact with class libraries and in-progress transfers.
In particular, I'm worried about race conditions like:
- Host sends BULK OUT packets (packet 1 through 4 of 8 packet transfer), and device class handles these packets.
- Host sends BULK OUT packet (perhaps the 5th packet of an 8 packet transfer).
- Host sends CLEAR_FEATURE(endpoint_halt)
- DCD queues transfer events in order of EP within interrupt: first the clear feature and then the BULK OUT message
- USBD handles the CLEAR_FEATURE (class responds by resetting its state machine).
- USBD dispatches the bulk-out transfer to the class driver
- Class driver becomes confused because it received a packet without the proper header. (only the 1st packet of the transfer has the correct message header.)
A possible deadlock may be:
- Class starts transfer on BULK OUT (EP will ACK)
- Host sends CLEAR_FEATURE on EP
- Device sets endpoint to NAK, based on clear halt
- Class never receives the transfer, because EP is in NAK state, and waits forever. (Does class need to re-start the transfer? Should DCD have set EP to ACK when clearing halt since there was a pending transfer?)
Another issue is the case that the class driver wants to reject the endpoint halt from being cleared.