linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch v4] usb: dwc3: add cancelled reasons for dwc3 requests
@ 2021-03-27 18:17 Ray Chi
  2021-03-31  0:41 ` Thinh Nguyen
  0 siblings, 1 reply; 2+ messages in thread
From: Ray Chi @ 2021-03-27 18:17 UTC (permalink / raw)
  To: balbi, gregkh, Thinh.Nguyen
  Cc: linux-usb, linux-kernel, albertccwang, Ray Chi

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
 
 	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);
 }
 
-- 
2.31.0.291.g576ba9dcdaf-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [Patch v4] usb: dwc3: add cancelled reasons for dwc3 requests
  2021-03-27 18:17 [Patch v4] usb: dwc3: add cancelled reasons for dwc3 requests Ray Chi
@ 2021-03-31  0:41 ` Thinh Nguyen
  0 siblings, 0 replies; 2+ messages in thread
From: Thinh Nguyen @ 2021-03-31  0:41 UTC (permalink / raw)
  To: Ray Chi, balbi, gregkh, Thinh Nguyen
  Cc: linux-usb, linux-kernel, albertccwang

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

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-03-31  0:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-27 18:17 [Patch v4] usb: dwc3: add cancelled reasons for dwc3 requests Ray Chi
2021-03-31  0:41 ` Thinh Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).