Re: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields
From: Felipe Balbi
Date: Wed Dec 05 2018 - 04:07:57 EST
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