Hi, Anurag Kumar Vulisha <anuragku@xxxxxxxxxx> writes: >>> Thanks for spending your time in reviewing this patch. The reason for adding the >>> timer is when streams are enabled there could be chances for the host and gadget >>> controller to become out of sync, the gadget may wait for the host to issue prime >>> transaction and the host may wait for the gadget to issue ERDY. To avoid such a >>> potential deadlock conditions, timeout needs to be implemented in dwc3 driver. >> >>"in dwc3 driver" is an implementation choice. The situation you describe >>could happen with any UDC, right? >> > > Yes this could happen to other UDC drivers also, unless controller is capable of handling > >>> After timeout occurs, gadget will first stop transfer and restart the transfer again. >>> This issue is mentioned in databook 2.90A section 9.5.2. I am not aware of how >>> other controllers are handling the streams, but since this issue looks more like a >> >>We should get in touch with other UDC authors. We have at least Renesas, >>net2280, bcd_udc and mtu3 supporting superspeed. >> > > Thanks for pointing other drivers. Will refer these drivers to see how they are handling streams > >>> dwc3 specific issue, I think it would be more convincing to add the timer in dwc3 >>> gadget driver rather than adding in udc framework. Also we are stopping the timer >> >>why? When the situation you describe is something that can happen with >>any udc, why should we reimplement the solution on all UDCs supporting >>streams when we can give generic support for handling certain >>situations? >> > > I agree with you. As you suggested will work on implementing changes in UDC > >>> when a valid StreamEvnt is found, which would be difficult to handle if the timer is >> >>Why difficult? udc-core would call: >> >>mod_timer(gadget->stream_timeout_timer, msecs_to_jiffies(50)); >> >>Once you receive stream event, dwc3 would call: >> >>if (timer_pending(dwc->gadget.stream_timeout_timer)) >> del_timer(dwc->gadget.stream_timeout_timer); >> >>Why is that difficult? You could even avoid anything to be written in >>dwc3 and put the del_timer() inside usb_gadget_giveback_request() >>itself. That why, dwc3 doesn't even have to know that there's a timer >>running. Also, you're timer function, instead of calling dwc3's private >>functions, should be relying on the gadget API. >> >>Your timer, apparently, should be fired per-request, then your timer >>function would call: >> >>usb_ep_dequeue(request); >>usb_ep_queue(request); >> >>If the timer expires. This would work for any UDC, not only dwc3. Then, >>this is something we document for all UDCs and they'd have to adhere to >>the API. >> >>In summary, not that many changes needed to dwc3. Nothing related to >>timers inside dwc3. Almost everythin can, and should, be done >>generically. > > Thanks a lot for giving a detailed explanation. Will implement the timeout > changes into UDC core. no problem. I just wanna make sure that this work happens in one place and one place only :) cheers -- balbi
Attachment:
signature.asc
Description: PGP signature
