John Youn <john.y...@synopsys.com> 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.


>>> * 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.



Attachment: signature.asc
Description: PGP signature

Reply via email to