On Sun, Jun 16, 2024 at 4:33 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > This series below is the one you're referring to, right? > > > > https://lore.kernel.org/linux-kernel/20190124030228.19840-1-paul.elder@xxxxxxxxxxxxxxxx/ > > Yes, that's it. I'm impressed that you were able to find it; I had lost > track of it. Luckily, it was one of the first google results on the topic :) > > Do you know why it wasn't merged? (CC Paul). There are no comments on > > the latest version I managed to find. > > I guess Felipe Balbi (the USB Gadget maintainer at the time) just wasn't > very interested in fixing the problem. Ok, I see. > > Also, just to check my understanding: with that series in place and > > assuming the UDC drivers are updated, a gadget driver would need to > > first do usb_ep_queue with the proper length and explicit_status == > > true to get the data for the control OUT request, and then either do > > usb_ep_queue again with length 0 to ack or do usb_ep_set_halt to > > stall? > > Yes, that's how it worked. Alternatively, if the gadget driver didn't > set explicit_status in the control-OUT request then the UDC core would > automatically call usb_ep_queue again with a 0-length transfer to send > the status. That way existing gadget drivers would continue to work > after the UDC drivers were updated, and updated UDC drivers wouldn't > have to worry about doing an automatic acknowledge only some of the > time. > > Note that in order to avoid breaking things during the transition > period, it would also be necessary to add a flag to the usb_gadget > structure, indicating that the UDC driver has been updated to support > explicit_status. Ack, thank you for explaining! I've been collecting different kinds of non-critical issues and inconsistencies within the Gadget subsystem I hit while testing Raw Gadget. It's unlikely I'll get to working on them in the foreseeable future, but it's good to know what needs fixing should the need arise. So far, I have: - Allow stalling non-0-length control OUT transfers (https://github.com/xairy/raw-gadget/issues/71); - Contain USB_GADGET_DELAYED_STATUS within composite framework (https://github.com/xairy/raw-gadget/issues/70); - Support isochronous transfers in Dummy HCD/UDC (https://github.com/xairy/raw-gadget/issues/72); - Fix dwc2 issuing disconnect instead of reset (https://github.com/xairy/raw-gadget/issues/48); - dwc3 doesn't support low speed, even through a comment says every UDC must (https://github.com/xairy/raw-gadget/issues/41#issuecomment-1783022764). > PS: There's another weakness in the Gadget API which you might possibly > run across in your project. It's less likely to arise because it > involves lengthy delays. > > Say there's a control transfer with delayed status, and the gadget > driver delays for so long that the host times out the transfer. Then > the host starts a new control transfer before the gadget driver queues > its status reply. Since the Gadget API doesn't have any way to indicate > which control transfer a usb_request was meant for, the reply that was > meant for the old transfer would get sent to the host, and the host > would think it was a reply to the new transfer. > > This problem could be solved by adding a unique ID tag to each > usb_request, and passing the transfer ID as an extra argument to the > gadget driver's setup() callback. That would explicitly indicate which > transfer a request was meant for. But doing this would also require > updating every function driver and every UDC driver. Probably not worth > the effort, considering how unlikely it is that the situation will ever > arise. Ah, good to know! My experience confirms that this is unlikely: I haven't hit this issue in practice yet. P.S. By the way, I've been lately using the dwc3-based UDC built into one of my ThinkPad laptops for testing gadget drivers. I had to switch a hidden BIOS option to enable it (which was not straightforward), but it's nice to avoid dealing with external boards. I've written an article about this, in case you're interested: https://xairy.io/articles/thinkpad-xdci. Thank you!
