Re: dwc3: unusual handling of setup requests with wLength == 0

[Thinh Nguyen]

On Wed, Aug 23, 2023, Alan Stern wrote:
> On Wed, Aug 23, 2023 at 02:14:32AM +0000, Thinh Nguyen wrote:
> > On Sun, Aug 20, 2023, Alan Stern wrote:
> > > On Sat, Aug 19, 2023 at 12:06:53AM +0000, Thinh Nguyen wrote:
> > > > On Fri, Aug 18, 2023, Alan Stern wrote:
> > > > > Actually I agree with you.  When a new SETUP packet arrives before the 
> > > > > old control transfer has finished, the UDC driver should cancel all 
> > > > > pending requests and then invoke the ->setup() callback.  (I don't think 
> > > > > there is a standard error code for the cancelled requests; net2280 seems 
> > > > > to use -EPROTO whereas dummy-hcd seems to use -EOVERFLOW.)
> > > > 
> > > > Those are very odd choice of error codes for cancelled request. Even
> > > > though the gadget side doesn't have very defined error codes, I try to
> > > > follow the equivalent doc from the host side
> > > > (driver-api/usb/error-codes.rst), which is -ECONNRESET.
> > > > 
> > > > Whenever I see -EPROTO, I associate that to a specific host error:
> > > > transaction error. For -EOVERFLOW, I think of babble errors.
> > > 
> > > Do you have a suggestion for an error code that all the UDCs should use 
> > > in this situation?  -ECONNRESET is currently being used for requests 
> > > that were cancelled by usb_ep_dequeue().  Would -EREMOTEIO be more 
> > > suitable for requests attached to an aborted control transfer?
> > > 
> > 
> > That works. It would be great if we can document the usb error codes for
> > gadget side and keep them consistent across UDC drivers. I think there
> > are only a few common errors:
> > 
> > * Request aborted
> > * Request dequeued
> > * STALL
> > * Data dropped (isoc only)
> > 
> > Any other?
> 
> * Overflow
> 
> STALL is not a valid status for usb_requests on the gadget side; it 
> applies only on the host side (the host doesn't halt its endpoints).

The host can send a CLEAR_FEATURE(halt_ep). This will reset the data
sequence of the endpoint. In xhci spec (section 4.6.8), it suggests to
send this when the endpoint is reset. The endpoint is reset typically
when there's a transaction error.

The problem here is that typical protocol spec like MSC/UVC don't
specify how to handle CLEAR_FEATURE(halt_ep).

For Windows MSC driver, when the host recovers from the transaction
error, it sends CLEAR_FEATURE(halt_ep) and expects the transfer to be
cancelled. To synchronize with the host, the gadget driver needs to
cancel the request. Dwc3 needs to notify the gadget driver of this.

For other class driver, it may expect the transfer to resume after data
sequence reset.

As a result, for an endpoint that's STALL (or not), and if the host
sends CLEAR_FEATURE(halt_ep), the dwc3 returns the request with some
status code and let the gadget driver handle it. If the gadget driver
wants to cancel the transfer, it can drop the transfer. If the gadget
driver wants to resume, it can requeue the same requests with the saved
status to resume where it left off.

> 
> Requests can be aborted for two different reasons:
> 
> 	The endpoint is being disabled, either by an config/altsetting
> 	change or because of a disconnect
> 
> 	The request was for an aborted control transfer
> 
> Putting this together, I get the following status codes:
> 
> -ESHUTDOWN	Request aborted because ep was disabled
> -EREMOTEIO	Request was for an aborted control transfer
> -ECONNRESET	Request was cancelled by usb_ep_dequeue()
> -EXDEV		Data dropped (isoc only)
> -EOVERFLOW	The host sent more data than the request wanted
> 		(will never happen if the request's length is a
> 		nonzero multiple of the maxpacket size)
> 
> This applies only to the .status field of struct usb_request.  Calls to 
> usb_ep_queue() may return different error codes.
> 
> How does that sound?
> 

That looks great!

Thanks,
Thinh