| Message ID | 20181221232005.5301-1-jackp@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup | expand |
Hi, Jack Pham <jackp@codeaurora.org> writes: > OUT endpoint requests may somtimes have this flag set when > preparing to be submitted to HW indicating that there is an > additional TRB chained to the request for alignment purposes. > If that request is removed before the controller can execute the > transfer (e.g. ep_dequeue/ep_disable), the request will not go > through the dwc3_gadget_ep_cleanup_completed_request() handler > and will not have its needs_extra_trb flag cleared when > dwc3_gadget_giveback() is called. This same request could be > later requeued for a new transfer that does not require an > extra TRB and if it is successfully completed, the cleanup > and TRB reclamation will incorrectly process the additional TRB > which belongs to the next request, and incorrectly advances the > TRB dequeue pointer, thereby messing up calculation of the next > requeust's actual/remaining count when it completes. > > The right thing to do here is to ensure that the flag is cleared > before it is given back to the function driver. A good place > to do that is in dwc3_gadget_del_and_unmap_request(). > > Signed-off-by: Jack Pham <jackp@codeaurora.org> > --- > Hi Felipe, > > There's probably zero chance this is making it to 4.20, so if you take > this after 4.21-rc1 so be it. But should this be Cc: stable? If so it > needs to be sent separately for <= 4.19 as needs_extra_trb was previously > req->unaligned and req->zero. we need a Cc stable, indeed. And a Fixes tag.
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 2ecde30ad0b7..e97b14f444c8 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -177,6 +177,7 @@ static void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep, req->started = false; list_del(&req->list); req->remaining = 0; + req->needs_extra_trb = false; if (req->request.status == -EINPROGRESS) req->request.status = status;
OUT endpoint requests may somtimes have this flag set when preparing to be submitted to HW indicating that there is an additional TRB chained to the request for alignment purposes. If that request is removed before the controller can execute the transfer (e.g. ep_dequeue/ep_disable), the request will not go through the dwc3_gadget_ep_cleanup_completed_request() handler and will not have its needs_extra_trb flag cleared when dwc3_gadget_giveback() is called. This same request could be later requeued for a new transfer that does not require an extra TRB and if it is successfully completed, the cleanup and TRB reclamation will incorrectly process the additional TRB which belongs to the next request, and incorrectly advances the TRB dequeue pointer, thereby messing up calculation of the next requeust's actual/remaining count when it completes. The right thing to do here is to ensure that the flag is cleared before it is given back to the function driver. A good place to do that is in dwc3_gadget_del_and_unmap_request(). Signed-off-by: Jack Pham <jackp@codeaurora.org> --- Hi Felipe, There's probably zero chance this is making it to 4.20, so if you take this after 4.21-rc1 so be it. But should this be Cc: stable? If so it needs to be sent separately for <= 4.19 as needs_extra_trb was previously req->unaligned and req->zero. Thanks, Jack drivers/usb/dwc3/gadget.c | 1 + 1 file changed, 1 insertion(+)