On Wed, Aug 23, 2023 at 7:18 PM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote: > > > > First, I noticed that some of the UDC drivers only expect to handle a > > > delayed Status stage for SET_CONFIGURATION requests. (Which is > > Just want to clarify that dwc3 would expect a delayed status for any > request returned with USB_GADGET_DELAYED_STATUS. The issue is that dwc3 > assumes the _first_ delayed request would be SET_CONFIGURATION. Any > subsequence control request with delayed request, it assumes the device > is already in configured state. > > > That expectation is wrong; gadget drivers can also want to delay the > > Status stage for a SET_INTERFACE request. And in theory they might want > > to delay any control-OUT transfer. > > Agree. Thanks Andrey and Alan for looking into dwc3. > > Regarding SET_INTERFACE, it should be fine because it should be done > while it's already in configured state, which is after > SET_CONFIGURATION. But it's true that dwc3 needs to fix this assumption > here. Ack. Thank you Thinh and Alan for taking the time to look into this! So to summarize the issue wrt the 0-length requests: 1. UDC drivers must always delay the status stage for 0-length requests until the gadget driver queues an empty request. 2. Many UDC drivers do not do this, and only delay the status stage when USB_GADGET_DELAYED_STATUS is returned from ->setup(). Some drivers also assume that only a SET_CONFIGURATION request can be delayed. 3. All such UDC drivers should be fixed and USB_GADGET_DELAYED_STATUS should be contained within the composite framework. 4. Fixing all such UDC drivers is a non-trivial amount of work, but this is the goal to strive towards. Alan, would it be acceptable if I add custom handling of USB_GADGET_DELAYED_STATUS to Raw Gadget in the meantime? It would be great to keep it at least somewhat working with dwc3. I can also do it for GadgetFS, if you think it's a good idea. I can also add some clarifying comments for USB_GADGET_DELAYED_STATUS and ->setup() to hopefully avoid having new UDC drivers being added with the same issue (e.g. cdns2 and renesas_usbf with the same issue were added just recently). Maybe it's also a good idea to add a checkpatch.pl check for using USB_GADGET_DELAYED_STATUS in UDC drivers. Or maybe ask Greg to keep an eye out for this?
