| Message ID | 20210327181742.1810969-1-raychi@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 04dd6e76b228891d29e49759e2351eb4a4303fc9 |
| Headers | show |
| Series | [v4] usb: dwc3: add cancelled reasons for dwc3 requests | expand |
Ray Chi wrote: > Currently, when dwc3 handles request cancelled, dwc3 just returns > -ECONNRESET for all requests. It will cause USB function drivers > can't know if the requests are cancelled by other reasons. > > This patch will replace DWC3_REQUEST_STATUS_CANCELLED with the > reasons below. > - DWC3_REQUEST_STATUS_DISCONNECTED > - DWC3_REQUEST_STATUS_DEQUEUED > - DWC3_REQUEST_STATUS_STALLED > > Signed-off-by: Ray Chi <raychi@google.com> > --- > Changelog since v3: > - add error log and pass -ECONNRESET in default case > > Changelog since v2: > - modify the changelog > - remove theree definitions > - replace DWC3_REQUEST_STATUS_CANCELLED with new cancelled reason > - In dwc3_gadget_ep_cleanup_cancelled_requests(), > add a switch case to give different error code base on the reason > > Changelog since v1: > - Added new parameter to dwc3_gadget_move_cancelled_request() > - Added three definitions for cancelled reasons > - Passed the reason to req->request.status > > drivers/usb/dwc3/core.h | 12 +++++++----- > drivers/usb/dwc3/gadget.c | 24 ++++++++++++++++++++---- > drivers/usb/dwc3/gadget.h | 6 ++++-- > 3 files changed, 31 insertions(+), 11 deletions(-) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 4ca4b4be85e4..51a3eec2428a 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -908,11 +908,13 @@ struct dwc3_request { > unsigned int remaining; > > unsigned int status; > -#define DWC3_REQUEST_STATUS_QUEUED 0 > -#define DWC3_REQUEST_STATUS_STARTED 1 > -#define DWC3_REQUEST_STATUS_CANCELLED 2 > -#define DWC3_REQUEST_STATUS_COMPLETED 3 > -#define DWC3_REQUEST_STATUS_UNKNOWN -1 > +#define DWC3_REQUEST_STATUS_QUEUED 0 > +#define DWC3_REQUEST_STATUS_STARTED 1 > +#define DWC3_REQUEST_STATUS_DISCONNECTED 2 > +#define DWC3_REQUEST_STATUS_DEQUEUED 3 > +#define DWC3_REQUEST_STATUS_STALLED 4 > +#define DWC3_REQUEST_STATUS_COMPLETED 5 > +#define DWC3_REQUEST_STATUS_UNKNOWN -1 The -1 here is an weird... as the dwc3_request status is an unsigned int type. We can make this fix in a separate patch, and what's done here looks good. > > u8 epnum; > struct dwc3_trb *trb; > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index e1442fc763e1..492086519539 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1402,7 +1402,7 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) > dwc3_stop_active_transfer(dep, true, true); > > list_for_each_entry_safe(req, tmp, &dep->started_list, list) > - dwc3_gadget_move_cancelled_request(req); > + dwc3_gadget_move_cancelled_request(req, DWC3_REQUEST_STATUS_DEQUEUED); > > /* If ep isn't started, then there's no end transfer pending */ > if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) > @@ -1729,10 +1729,25 @@ static void dwc3_gadget_ep_cleanup_cancelled_requests(struct dwc3_ep *dep) > { > struct dwc3_request *req; > struct dwc3_request *tmp; > + struct dwc3 *dwc = dep->dwc; > > list_for_each_entry_safe(req, tmp, &dep->cancelled_list, list) { > dwc3_gadget_ep_skip_trbs(dep, req); > - dwc3_gadget_giveback(dep, req, -ECONNRESET); > + switch (req->status) { > + case DWC3_REQUEST_STATUS_DISCONNECTED: > + dwc3_gadget_giveback(dep, req, -ESHUTDOWN); > + break; > + case DWC3_REQUEST_STATUS_DEQUEUED: > + dwc3_gadget_giveback(dep, req, -ECONNRESET); > + break; > + case DWC3_REQUEST_STATUS_STALLED: > + dwc3_gadget_giveback(dep, req, -EPIPE); > + break; > + default: > + dev_err(dwc->dev, "request cancelled with wrong reason:%d\n", req->status); > + dwc3_gadget_giveback(dep, req, -ECONNRESET); > + break; > + } > } > } > > @@ -1776,7 +1791,8 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > * cancelled. > */ > list_for_each_entry_safe(r, t, &dep->started_list, list) > - dwc3_gadget_move_cancelled_request(r); > + dwc3_gadget_move_cancelled_request(r, > + DWC3_REQUEST_STATUS_DEQUEUED); > > dep->flags &= ~DWC3_EP_WAIT_TRANSFER_COMPLETE; > > @@ -1848,7 +1864,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol) > dwc3_stop_active_transfer(dep, true, true); > > list_for_each_entry_safe(req, tmp, &dep->started_list, list) > - dwc3_gadget_move_cancelled_request(req); > + dwc3_gadget_move_cancelled_request(req, DWC3_REQUEST_STATUS_STALLED); > > if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) { > dep->flags |= DWC3_EP_PENDING_CLEAR_STALL; > diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h > index 0cd281949970..77df4b6d6c13 100644 > --- a/drivers/usb/dwc3/gadget.h > +++ b/drivers/usb/dwc3/gadget.h > @@ -90,15 +90,17 @@ static inline void dwc3_gadget_move_started_request(struct dwc3_request *req) > /** > * dwc3_gadget_move_cancelled_request - move @req to the cancelled_list > * @req: the request to be moved > + * @reason: cancelled reason for the dwc3 request > * > * Caller should take care of locking. This function will move @req from its > * current list to the endpoint's cancelled_list. > */ > -static inline void dwc3_gadget_move_cancelled_request(struct dwc3_request *req) > +static inline void dwc3_gadget_move_cancelled_request(struct dwc3_request *req, > + unsigned int reason) > { > struct dwc3_ep *dep = req->dep; > > - req->status = DWC3_REQUEST_STATUS_CANCELLED; > + req->status = reason; > list_move_tail(&req->list, &dep->cancelled_list); > } > > Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> Thanks for the patch, Thinh
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 4ca4b4be85e4..51a3eec2428a 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -908,11 +908,13 @@ struct dwc3_request { unsigned int remaining; unsigned int status; -#define DWC3_REQUEST_STATUS_QUEUED 0 -#define DWC3_REQUEST_STATUS_STARTED 1 -#define DWC3_REQUEST_STATUS_CANCELLED 2 -#define DWC3_REQUEST_STATUS_COMPLETED 3 -#define DWC3_REQUEST_STATUS_UNKNOWN -1 +#define DWC3_REQUEST_STATUS_QUEUED 0 +#define DWC3_REQUEST_STATUS_STARTED 1 +#define DWC3_REQUEST_STATUS_DISCONNECTED 2 +#define DWC3_REQUEST_STATUS_DEQUEUED 3 +#define DWC3_REQUEST_STATUS_STALLED 4 +#define DWC3_REQUEST_STATUS_COMPLETED 5 +#define DWC3_REQUEST_STATUS_UNKNOWN -1 u8 epnum; struct dwc3_trb *trb; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index e1442fc763e1..492086519539 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1402,7 +1402,7 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) dwc3_stop_active_transfer(dep, true, true); list_for_each_entry_safe(req, tmp, &dep->started_list, list) - dwc3_gadget_move_cancelled_request(req); + dwc3_gadget_move_cancelled_request(req, DWC3_REQUEST_STATUS_DEQUEUED); /* If ep isn't started, then there's no end transfer pending */ if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) @@ -1729,10 +1729,25 @@ static void dwc3_gadget_ep_cleanup_cancelled_requests(struct dwc3_ep *dep) { struct dwc3_request *req; struct dwc3_request *tmp; + struct dwc3 *dwc = dep->dwc; list_for_each_entry_safe(req, tmp, &dep->cancelled_list, list) { dwc3_gadget_ep_skip_trbs(dep, req); - dwc3_gadget_giveback(dep, req, -ECONNRESET); + switch (req->status) { + case DWC3_REQUEST_STATUS_DISCONNECTED: + dwc3_gadget_giveback(dep, req, -ESHUTDOWN); + break; + case DWC3_REQUEST_STATUS_DEQUEUED: + dwc3_gadget_giveback(dep, req, -ECONNRESET); + break; + case DWC3_REQUEST_STATUS_STALLED: + dwc3_gadget_giveback(dep, req, -EPIPE); + break; + default: + dev_err(dwc->dev, "request cancelled with wrong reason:%d\n", req->status); + dwc3_gadget_giveback(dep, req, -ECONNRESET); + break; + } } } @@ -1776,7 +1791,8 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, * cancelled. */ list_for_each_entry_safe(r, t, &dep->started_list, list) - dwc3_gadget_move_cancelled_request(r); + dwc3_gadget_move_cancelled_request(r, + DWC3_REQUEST_STATUS_DEQUEUED); dep->flags &= ~DWC3_EP_WAIT_TRANSFER_COMPLETE; @@ -1848,7 +1864,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol) dwc3_stop_active_transfer(dep, true, true); list_for_each_entry_safe(req, tmp, &dep->started_list, list) - dwc3_gadget_move_cancelled_request(req); + dwc3_gadget_move_cancelled_request(req, DWC3_REQUEST_STATUS_STALLED); if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) { dep->flags |= DWC3_EP_PENDING_CLEAR_STALL; diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h index 0cd281949970..77df4b6d6c13 100644 --- a/drivers/usb/dwc3/gadget.h +++ b/drivers/usb/dwc3/gadget.h @@ -90,15 +90,17 @@ static inline void dwc3_gadget_move_started_request(struct dwc3_request *req) /** * dwc3_gadget_move_cancelled_request - move @req to the cancelled_list * @req: the request to be moved + * @reason: cancelled reason for the dwc3 request * * Caller should take care of locking. This function will move @req from its * current list to the endpoint's cancelled_list. */ -static inline void dwc3_gadget_move_cancelled_request(struct dwc3_request *req) +static inline void dwc3_gadget_move_cancelled_request(struct dwc3_request *req, + unsigned int reason) { struct dwc3_ep *dep = req->dep; - req->status = DWC3_REQUEST_STATUS_CANCELLED; + req->status = reason; list_move_tail(&req->list, &dep->cancelled_list); }
Currently, when dwc3 handles request cancelled, dwc3 just returns -ECONNRESET for all requests. It will cause USB function drivers can't know if the requests are cancelled by other reasons. This patch will replace DWC3_REQUEST_STATUS_CANCELLED with the reasons below. - DWC3_REQUEST_STATUS_DISCONNECTED - DWC3_REQUEST_STATUS_DEQUEUED - DWC3_REQUEST_STATUS_STALLED Signed-off-by: Ray Chi <raychi@google.com> --- Changelog since v3: - add error log and pass -ECONNRESET in default case Changelog since v2: - modify the changelog - remove theree definitions - replace DWC3_REQUEST_STATUS_CANCELLED with new cancelled reason - In dwc3_gadget_ep_cleanup_cancelled_requests(), add a switch case to give different error code base on the reason Changelog since v1: - Added new parameter to dwc3_gadget_move_cancelled_request() - Added three definitions for cancelled reasons - Passed the reason to req->request.status drivers/usb/dwc3/core.h | 12 +++++++----- drivers/usb/dwc3/gadget.c | 24 ++++++++++++++++++++---- drivers/usb/dwc3/gadget.h | 6 ++++-- 3 files changed, 31 insertions(+), 11 deletions(-)