Hi, John Youn <John.Youn@xxxxxxxxxxxx> writes: >>>>>>>> Just from the code it seems there will be some fundamental issues with >>>>>>>> it, such as the value of maxpacket size and some alt-interface >>>>>>>> stuff. At least when used with DWC3. >>>>>>> >>>>>>> such as? >>>>>> >>>>>> I'll see if I can write up the exact issues later. I have to go back >>>>>> to my notes to remind myself. >>> >>> Ok, coming back to this uas gadget stuff. >>> >>> I've finally had a chance to go back to my notes. Here are some of the >>> concrete issues that I found, that I was able to workaround in some >>> way. >>> >>> * EP's for UAS alt-interface cannot be configured correctly because >>> the config_ep_for_speed() in composite.c does not take into account >>> alt-interfaces. This results in incorrectly configured EP in the >>> controller (no streaming enabled, wrong direction, etc). >> >> cool, that's a bug in composite.c which needs to be fixed. >> >>> * START_XFER command needs to enable streams >> >> bug in dwc3 which needs to be fixed. >> >>> * XFER_NOT_READY event needs to be enabled for streams >> >> bug in dwc3 which needs to be fixed :-) In any case, can you point me to >> section in documention which states Streams *requires* XFER_NOT_READY? > > It should be the same as used for BULK. Maybe it's not needed with > your recent enhancements? Not sure. I have to rebase and test. okay >>> * For OUT EPs, the TCM/target framework sets receive buffers size as >>> 512 bytes. This cannot work in SUPERSPEED as you must always provide >>> at least MPS-sized buffers. This causes all writes to fail. I'm not >> >> that's a good point. TCM gadget should be checking for ep out aligned >> quirk flag. > > Is this an existing quirk? It's not just about alignment, but the size > of the rx buffer, which it should know based on the MPS for the speed. Yeah, it's part of struct usb_gadget: * @quirk_ep_out_aligned_size: epout requires buffer size to be aligned to * MaxPacketSize. >>> sure how to properly fix this as it should be fixed at the function >>> driver level, and this size comes from the target framework. I put a >>> workaround at the controller-level. >>> >>> After addressing these issues, UAS read/write works to some extent. >>> But it is still unstable in ways that point to issues in the target >>> framework, things to do with locking/race conditions there. >> >> Alright, those are all bugs which need to be fixed. Certainly we don't >> need an entire new gadget just because you've found some bugs, right? > > Sure. I was just curious if there was any interest in it since we have > an existing driver we could port. I see. >> We'd be much better off fixing these problems because then more folks >> would benefit from them. > > Agree if it was working and being used in the first place. Well, if you found bugs with it, you used it :-) We have so many gadgets you can't expect us to always constantly test them all, there's just not enough time for _me_ to test all gadgets. I have to rely on the fact that hundreds are involved with the gadget framework and assume some of these gadgets will get tested eventually. > But anyways I'll focus on TCM. thanks -- balbi
Attachment:
signature.asc
Description: PGP signature
