usb: dwc3: gadget: Avoid canceling current request for queuing error
Subsystems (2)
PATCH 1/1 usb: dwc3: gadget: Avoid canceling current request for queuing error
If an error is received when issuing a start or update transfer command, the error handler will stop all active requests (including the current USB request), and call dwc3_gadget_giveback() to notify function drivers of the requests which have been stopped. Avoid having to cancel the current request which is trying to be queued, as the function driver will handle the EP queue error accordingly. Simply unmap the request as it was done before, and allow previously started transfers to be cleaned up.
--- drivers/usb/dwc3/gadget.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/usb/dwc3/gadget.c +5 -0
@@ -1399,6 +1399,11 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)if (ret == -EAGAIN )return ret; +/* Avoid canceling current request, as it has not been started */ +if (req->trb) +memset (req->trb,0 ,sizeof (struct dwc3_trb )); +dwc3_gadget_del_and_unmap_request (dep, req, ret); +dwc3_stop_active_transfer (dep, true, true);list_for_each_entry_safe (req, tmp, &dep->started_list, list)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Quoted 19 lines · Wesley Cheng
If an error is received when issuing a start or update transfer command, the error handler will stop all active requests (including the current USB request), and call
dwc3_gadget_giveback()to notify function drivers of the requests which have been stopped. Avoid having to cancel the current request which is trying to be queued, as the function driver will handle the EP queue error accordingly. Simply unmap the request as it was done before, and allow previously started transfers to be cleaned up.Signed-off-by Wesley Cheng <[email protected]>--- drivers/usb/dwc3/gadget.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/usb/dwc3/gadget.c +0
-0
drivers/usb/dwc3/gadget.c static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) · line 1404
Quoted 6 lines
drivers/usb/dwc3/gadget.c +3
-0@@ -1399,6 +1399,11 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)if (ret == -EAGAIN )return ret; +/* Avoid canceling current request, as it has not been started */ +if (req->trb) +memset (req->trb,0 ,sizeof (struct dwc3_trb ));
we don't need a full memset. I think ensuring HWO bit is zero is enough.
Signature · 1 line
balbi
Quoted 24 lines · Felipe Balbi
Quoted 22 lines · Wesley Cheng
If an error is received when issuing a start or update transfer command, the error handler will stop all active requests (including the current USB request), and call
dwc3_gadget_giveback()to notify function drivers of the requests which have been stopped. Avoid having to cancel the current request which is trying to be queued, as the function driver will handle the EP queue error accordingly. Simply unmap the request as it was done before, and allow previously started transfers to be cleaned up.Signed-off-by Wesley Cheng <[email protected]>--- drivers/usb/dwc3/gadget.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/usb/dwc3/gadget.c +0
-0@@ -1399,6 +1399,11 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)if (ret == -EAGAIN )return ret;
Quoted 6 lines
drivers/usb/dwc3/gadget.c static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) · line 1404
Quoted 3 lines
drivers/usb/dwc3/gadget.c +3
-0+/* Avoid canceling current request, as it has not been started */ +if (req->trb) +memset (req->trb,0 ,sizeof (struct dwc3_trb ));we don't need a full memset. I think ensuring HWO bit is zero is enough.
Hi Felipe,
Thanks for the input/review, will make this change to just clear the HWO.
Thanks
Wesley Cheng
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Thinh Nguyen
maintainerQuoted 33 lines · Wesley Cheng
Quoted 24 lines · Felipe Balbi
Quoted 22 lines · Wesley Cheng
If an error is received when issuing a start or update transfer command, the error handler will stop all active requests (including the current USB request), and call
dwc3_gadget_giveback()to notify function drivers of the requests which have been stopped. Avoid having to cancel the current request which is trying to be queued, as the function driver will handle the EP queue error accordingly. Simply unmap the request as it was done before, and allow previously started transfers to be cleaned up.Signed-off-by Wesley Cheng <[email protected]>--- drivers/usb/dwc3/gadget.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/usb/dwc3/gadget.c +0
-0@@ -1399,6 +1399,11 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)if (ret == -EAGAIN )return ret;Quoted 6 lines
drivers/usb/dwc3/gadget.c static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) · line 1404
Quoted 3 lines
drivers/usb/dwc3/gadget.c +3
-0+/* Avoid canceling current request, as it has not been started */ +if (req->trb) +memset (req->trb,0 ,sizeof (struct dwc3_trb ));we don't need a full memset. I think ensuring HWO bit is zero is enough.
Quoted 4 lines
Hi Felipe,
Thanks for the input/review, will make this change to just clear the HWO.
Make sure to increment the dequeue pointer also. I think you can use dwc3_gadget_ep_skip_trbs().
BR,
Thinh
Thinh Nguyen
maintainerQuoted 34 lines · Thinh Nguyen
Quoted 33 lines · Wesley Cheng
Quoted 30 lines · Felipe Balbi
Quoted 19 lines · Wesley Cheng
If an error is received when issuing a start or update transfer command, the error handler will stop all active requests (including the current USB request), and call
dwc3_gadget_giveback()to notify function drivers of the requests which have been stopped. Avoid having to cancel the current request which is trying to be queued, as the function driver will handle the EP queue error accordingly. Simply unmap the request as it was done before, and allow previously started transfers to be cleaned up.Signed-off-by Wesley Cheng <[email protected]>--- drivers/usb/dwc3/gadget.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/usb/dwc3/gadget.c +0
-0drivers/usb/dwc3/gadget.c static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) · line 1404
Quoted 6 lines
drivers/usb/dwc3/gadget.c +3
-0@@ -1399,6 +1399,11 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)if (ret == -EAGAIN )return ret; +/* Avoid canceling current request, as it has not been started */ +if (req->trb) +memset (req->trb,0 ,sizeof (struct dwc3_trb ));we don't need a full memset. I think ensuring HWO bit is zero is enough.
Quoted 8 lines
Quoted 4 lines
Hi Felipe,
Thanks for the input/review, will make this change to just clear the HWO.
Make sure to increment the dequeue pointer also. I think you can use
dwc3_gadget_ep_skip_trbs().
Nevermind. There maybe a problem with using dwc3_gadget_ep_skip_trbs().
Thinh
Quoted 40 lines · Thinh Nguyen
Quoted 39 lines · Thinh Nguyen
Quoted 37 lines · Wesley Cheng
Quoted 24 lines · Felipe Balbi
Quoted 22 lines · Wesley Cheng
If an error is received when issuing a start or update transfer command, the error handler will stop all active requests (including the current USB request), and call
dwc3_gadget_giveback()to notify function drivers of the requests which have been stopped. Avoid having to cancel the current request which is trying to be queued, as the function driver will handle the EP queue error accordingly. Simply unmap the request as it was done before, and allow previously started transfers to be cleaned up.Signed-off-by Wesley Cheng <[email protected]>--- drivers/usb/dwc3/gadget.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/usb/dwc3/gadget.c +0
-0@@ -1399,6 +1399,11 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)if (ret == -EAGAIN )return ret;Quoted 6 lines
drivers/usb/dwc3/gadget.c static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) · line 1404
Quoted 3 lines
drivers/usb/dwc3/gadget.c +3
-0+/* Avoid canceling current request, as it has not been started */ +if (req->trb) +memset (req->trb,0 ,sizeof (struct dwc3_trb ));we don't need a full memset. I think ensuring HWO bit is zero is enough.
Hi Felipe,
Thanks for the input/review, will make this change to just clear the HWO.
Quoted 8 lines
Quoted 3 lines
Make sure to increment the dequeue pointer also. I think you can use
dwc3_gadget_ep_skip_trbs().Nevermind. There maybe a problem with using
dwc3_gadget_ep_skip_trbs().Thinh
Hi Thinh,
Thank you for your input. In this case (if kick transfer fails w/ an error), would we still need to mess with the enqueue/dequeue pointers? Not sure if my assumption is correct, but the TRB wouldn't have been started, so we can use the same (failed) TRB for future requests, right?
I think one thing I will need to update is to loop through num_trbs and clear all HWO bits if the above is not needed.
Thanks
Wesley Cheng
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Thinh Nguyen
maintainerQuoted 53 lines · Wesley Cheng
Quoted 40 lines · Thinh Nguyen
Quoted 39 lines · Thinh Nguyen
Quoted 37 lines · Wesley Cheng
Quoted 24 lines · Felipe Balbi
Quoted 22 lines · Wesley Cheng
If an error is received when issuing a start or update transfer command, the error handler will stop all active requests (including the current USB request), and call
dwc3_gadget_giveback()to notify function drivers of the requests which have been stopped. Avoid having to cancel the current request which is trying to be queued, as the function driver will handle the EP queue error accordingly. Simply unmap the request as it was done before, and allow previously started transfers to be cleaned up.Signed-off-by Wesley Cheng <[email protected]>--- drivers/usb/dwc3/gadget.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/usb/dwc3/gadget.c +0
-0@@ -1399,6 +1399,11 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)if (ret == -EAGAIN )return ret;Quoted 6 lines
drivers/usb/dwc3/gadget.c static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) · line 1404
Quoted 3 lines
drivers/usb/dwc3/gadget.c +3
-0+/* Avoid canceling current request, as it has not been started */ +if (req->trb) +memset (req->trb,0 ,sizeof (struct dwc3_trb ));we don't need a full memset. I think ensuring HWO bit is zero is enough.
Hi Felipe,
Thanks for the input/review, will make this change to just clear the HWO.
Quoted 8 lines
Quoted 3 lines
Make sure to increment the dequeue pointer also. I think you can use
dwc3_gadget_ep_skip_trbs().Nevermind. There maybe a problem with using
dwc3_gadget_ep_skip_trbs().Thinh
Hi Thinh,
Quoted 8 lines
Thank you for your input. In this case (if kick transfer fails w/ an error), would we still need to mess with the enqueue/dequeue pointers? Not sure if my assumption is correct, but the TRB wouldn't have been started, so we can use the same (failed) TRB for future requests, right?
I think one thing I will need to update is to loop through
num_trbsand clear all HWO bits if the above is not needed.
No. We track and increment the TRB enqueue counter whenever we prepared one. What you need to do is the opposite as dwc3_gadget_ep_skip_trbs(). You'd need to decrement the enqueue counter of all the TRB prepared by the request.
Not every request is the same, and we shouldn't rely on the function driver behavior to reuse/resubmit the same request.
Also, we use __dwc3_gadget_kick_transfer() everywhere and not just when queuing new request. Make sure that your change only applies when queuing new request failed and not because of other scenarios.
Thanks,
Thinh