Hi, On Wed, Jun 06, 2012 at 09:42:44AM +0530, Pratyush Anand wrote: > On 6/5/2012 8:28 PM, Felipe Balbi wrote: > >>- if (!list_empty(&dep->req_queued)) > >>> + if (!list_empty(&dep->req_queued)&& !dep->end_xfer_issued) > >I would rather not wait for ENDXFER IRQ, I have a patch for that, can > >you test it ? (attached) > > I tested it. But it does not seem to resolve issue. > > 1. I see warning WARN_ON(!dep->res_trans_idx) from > dwc3_stop_active_transfer. I see it because, > > in dwc3_gadget_disconnect_interrupt we do, > dwc3_stop_active_transfers: > dwc3_disconnect_gadget: > dwc3_disconnect_gadget will call in turn disable function which will > further call dwc3_stop_active_transfer. > > I think to resolve this we need to traverse all the nodes of > req_queued and giveback for all of them and then giveback for all > nodes of request_list. > > > 2. I see NULL pointer dereference from dwc3_endpoint_interrupt. > > I see it because, after end transfer command issue we receive > xferinprogress. But dep->desc has been assigned to NULL during > disable. So it crashes. Hmm, that needs to be fixed up, indeed. We cannot have multiple ways of reaching the same function. That's a bug in the code, for sure. I'll give it a shot at fixing that part too. > May be Paul can confirm, if it is expected to receive atleast one > xferinprogress after END_TRANSFER issue? > > > Other than this, I have one comment for patch 0001: > Yes this code was redundant which I had also noticed and raised a > query to IP Vendor. > 100us seems to be guaranteed time for END_TRANSFER completion. > Similarly, what is the guaranteed time for completion of > START_TRANSFER? If we read res_trans_idx just after issueing of > START_TRANSFER , is it reliable? I will come back, when I receive the > reply. > > Regards > Pratyush > > PS: I know, my patches are not elegant and using so many flags to > synchronize these issues, which I too do not like much. But I could > not see other way to do it. I have tested it and seems to be working. I understand that, but keeping START_/END_TRANSFER IRQ is just causing too much trouble and maintaining that huge amount of flags over the years will be a pain. That's just a recipe for disaster. -- balbi
Attachment:
signature.asc
Description: Digital signature
