RE: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields

[Felipe Balbi]

Hi,

Anurag Kumar Vulisha <anuragku@xxxxxxxxxx> writes:
>>> @@ -2286,7 +2286,12 @@ static int
>>dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>>>  	if (event->status & DEPEVT_STATUS_SHORT && !chain)
>>>  		return 1;
>>>
>>> -	if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST))
>>> +	if ((event->status & DEPEVT_STATUS_IOC) &&
>>> +	    (trb->ctrl & DWC3_TRB_CTRL_IOC))
>>> +		return 1;
>>
>>this shouldn't be necessary. According to databook, event->status
>>contains the bits from the completed TRB.  Which means that
>>event->status & IOC will always be equal to trb->ctrl & IOC.
>>
> 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:

modified   drivers/usb/dwc3/gadget.c
@@ -2372,7 +2372,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 	if (event->status & DEPEVT_STATUS_SHORT && !chain)
 		return 1;
 
-	if (event->status & DEPEVT_STATUS_IOC)
+	if (event->status & DEPEVT_STATUS_IOC && !chain)
 		return 1;
 
 	return 0;

-- 
balbi