Hi, Anurag Kumar Vulisha <anuragku@xxxxxxxxxx> writes: >>>>> Thanks for reviewing this patch. Lets consider an example where a >>>>> request has num_sgs > 0 and each sg is mapped to a TRB and the last >>>>> TRB has the IOC bit set. Once the controller is done with the >>>>> transfer, it generates XferInProgress for the last TRB (since IOC bit >>>>> is set). As a part of trb reclaim process >>>>> dwc3_gadget_ep_reclaim_trb_sg() calls >>>>> dwc3_gadget_ep_reclaim_completed_trb() for req->num_sgs times. Since >>>>> the event already has the IOC bit set, the loop is exited from the >>>>> loop at the very first TRB and the remaining TRBs (mapped to the sglist) are left >>>>unhandled. >>>>> To avoid this we modified the code to exit only if both TRB & event >>>>> has the IOC bit set. >>>> >>>>Seems like IOC case should just test for chain flag as well: >>>> >>> >>> Okay. Along with this logic the code for updating chain bit should also be modified I >>guess. >> >>not really >> >>> Since the IOC bit is also set when there are not enough TRBs available, the code >>should be >>> modified to not set DWC3_TRB_CTRL_CHN bit when the IOC bit is set. I will update >>below >>> changes along with your suggestions and resend the patches. >> >>no. Actually I don't think we're allowed to split a scatter/gather like >>that. I did that quite a while ago, but I don't think we're allowed to >>do so. What we should do, in that case, is not even queue that request >>until we have enough for all members of the scatter/gather. But that's a >>separate patch, anyway. >> > > Okay. I have a doubt here, not pushing the request until all sgs are mapped to enough TRBs > might remove the driver complexity but reduce the performance (since we are waiting > until enough TRBs are available). Are we okay with that? The only other way would be to copy the buffer over to a contiguous buffer. That will also reduce performance. I think we need to consider how frequently this may actually happen. I dare to say we don't have any usb function in kernel as of today that can, easily and frequently, fall into such a situation. Besides, the performance loss can be amortized by a deeper request queue. IMO, this is a minor problem. But, certainly, if you have the setup, _do_ run some benchmarking and report your findings :-) -- balbi
Attachment:
signature.asc
Description: PGP signature
