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 <anurag.kumar.vulisha@xxxxxxxxxx> writes:
> The present code in dwc3_gadget_ep_reclaim_completed_trb() will check
> for IOC/LST bit in the event->status and returns if IOC/LST bit is
> set. This logic doesn't work if multiple TRBs are queued per
> request and the IOC/LST bit is set on the last TRB of that request.
> Consider an example where a queued request has multiple queued TRBs
> and IOC/LST bit is set only for the last TRB. In this case, the Core
> generates XferComplete/XferInProgress events only for the last TRB
> (since IOC/LST are set only for the last TRB). As per the logic in
> dwc3_gadget_ep_reclaim_completed_trb() event->status is checked for
> IOC/LST bit and returns on the first TRB. This makes the remaining
> TRBs left unhandled.
> To aviod this, changed the code to check for IOC/LST bits in both
> event->status & TRB->ctrl. This patch does the same.
>
> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xxxxxxxxxx>
> Reviewed-by: Thinh Nguyen <thinhn@xxxxxxxxxxxx>
> Tested-by: Tejas Joglekar <tejas.joglekar@xxxxxxxxxxxx>
> ---
>  Changes in v7:
> 	1. None
>
>  Changes in v6:
> 	1. None
>
>  Changes in v5:
> 	1. None
>
>  Changes in v4:
> 	1. None
>
>  Changes in v3:
> 	1. None
>
>  Changes in v2:
> 	1. None
> ---
>  drivers/usb/dwc3/gadget.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 9ddc9fd..216179e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -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.

Can you further describe the situation here? Perhaps share tracepoints
exposing the problem?

-- 
balbi

Attachment: signature.asc
Description: PGP signature