| Message ID | 20220307214639.4164547-1-m.grzeschik@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: dwc3: gadget: set status of request on every completed trb | expand |
Michael Grzeschik wrote: > Currently the status of the request being completed comes from the > dwc3_event_depevt status. The resulting status will then be applied to > the request on dwc3_gadget_del_and_unmap_request. > > This assigned status is not right in every case. Since it is possible > that more requests can be ready on the interrupt handler we have to set > the actual status for every request from the trbstatus instead. > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > --- > drivers/usb/dwc3/gadget.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index a0c883f19a417c..760af09d6d8ef7 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -3171,6 +3171,11 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, > count = trb->size & DWC3_TRB_SIZE_MASK; > req->remaining += count; > > + if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC) > + req->request.status = -EXDEV; > + else if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_OK) > + req->request.status = 0; > + > if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN) > return 1; > req->request.status should only be set at one place, and currently it's in dwc3_gadget_del_and_unmap_request() when we give back the request. You need to fix dwc3_gadget_ep_reclaim_completed_trb() to not use the "status" from the event. Check and save the first TRB error and report to the request's status. Don't overwrite it with something new if there are chained TRBs in the request. I also notice that DEPEVT_STATUS_BUSERR is not applicable to Transfer in Progress and Transfer Complete events (that bit is reserved for those events). While you're working on this, maybe create a separate patch to remove that check also. Thanks, Thinh
Hello! On 3/8/22 12:46 AM, Michael Grzeschik wrote: > Currently the status of the request being completed comes from the > dwc3_event_depevt status. The resulting status will then be applied to > the request on dwc3_gadget_del_and_unmap_request. > > This assigned status is not right in every case. Since it is possible > that more requests can be ready on the interrupt handler we have to set > the actual status for every request from the trbstatus instead. > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > --- > drivers/usb/dwc3/gadget.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index a0c883f19a417c..760af09d6d8ef7 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -3171,6 +3171,11 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, > count = trb->size & DWC3_TRB_SIZE_MASK; > req->remaining += count; > > + if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC) > + req->request.status = -EXDEV; > + else if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_OK) > + req->request.status = 0; > + Shouldn't that be a *switch* statement instead? [...] MBR, Sergey
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index a0c883f19a417c..760af09d6d8ef7 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -3171,6 +3171,11 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, count = trb->size & DWC3_TRB_SIZE_MASK; req->remaining += count; + if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC) + req->request.status = -EXDEV; + else if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_OK) + req->request.status = 0; + if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN) return 1;
Currently the status of the request being completed comes from the dwc3_event_depevt status. The resulting status will then be applied to the request on dwc3_gadget_del_and_unmap_request. This assigned status is not right in every case. Since it is possible that more requests can be ready on the interrupt handler we have to set the actual status for every request from the trbstatus instead. Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> --- drivers/usb/dwc3/gadget.c | 5 +++++ 1 file changed, 5 insertions(+)