* [PATCH] usb: dwc3: add cancelled reason for dwc3 requests
@ 2021-03-25 11:54 Ray Chi
2021-03-25 12:25 ` Felipe Balbi
2021-03-25 20:52 ` Thinh Nguyen
0 siblings, 2 replies; 4+ messages in thread
From: Ray Chi @ 2021-03-25 11:54 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 class drivers can't
know if the requests are cancelled by other reasons.
This patch will add the reason when the requests are cancelled.
Signed-off-by: Ray Chi <raychi@google.com>
---
drivers/usb/dwc3/gadget.c | 6 +++---
drivers/usb/dwc3/gadget.h | 10 +++++++++-
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index e1442fc763e1..cc65fc064078 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_DEQUEUED);
/* If ep isn't started, then there's no end transfer pending */
if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING))
@@ -1776,7 +1776,7 @@ 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_DEQUEUED);
dep->flags &= ~DWC3_EP_WAIT_TRANSFER_COMPLETE;
@@ -1848,7 +1848,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_STALL);
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..a23e85bd3933 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -56,6 +56,12 @@ struct dwc3;
/* Frame/Microframe Number Mask */
#define DWC3_FRNUMBER_MASK 0x3fff
+
+/* Cancel reason for dwc3 request */
+#define DWC3_REQUEST_DEQUEUED -ECONNRESET /* Request get dequeued */
+#define DWC3_REQUEST_DISCONNECTED -ESHUTDOWN /* Device is disconnected/disabled */
+#define DWC3_REQUEST_STALL -EPIPE /* Bus or protocol error */
+
/* -------------------------------------------------------------------------- */
#define to_dwc3_request(r) (container_of(r, struct dwc3_request, request))
@@ -90,15 +96,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, int reason)
{
struct dwc3_ep *dep = req->dep;
req->status = DWC3_REQUEST_STATUS_CANCELLED;
+ req->request.status = reason;
list_move_tail(&req->list, &dep->cancelled_list);
}
--
2.31.0.291.g576ba9dcdaf-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] usb: dwc3: add cancelled reason for dwc3 requests
2021-03-25 11:54 [PATCH] usb: dwc3: add cancelled reason for dwc3 requests Ray Chi
@ 2021-03-25 12:25 ` Felipe Balbi
2021-03-25 21:00 ` Thinh Nguyen
2021-03-25 20:52 ` Thinh Nguyen
1 sibling, 1 reply; 4+ messages in thread
From: Felipe Balbi @ 2021-03-25 12:25 UTC (permalink / raw)
To: Ray Chi, gregkh, Thinh.Nguyen
Cc: linux-usb, linux-kernel, albertccwang, Ray Chi
[-- Attachment #1: Type: text/plain, Size: 747 bytes --]
Hi,
Ray Chi <raychi@google.com> writes:
> diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
> index 0cd281949970..a23e85bd3933 100644
> --- a/drivers/usb/dwc3/gadget.h
> +++ b/drivers/usb/dwc3/gadget.h
> @@ -56,6 +56,12 @@ struct dwc3;
>
> /* Frame/Microframe Number Mask */
> #define DWC3_FRNUMBER_MASK 0x3fff
> +
> +/* Cancel reason for dwc3 request */
> +#define DWC3_REQUEST_DEQUEUED -ECONNRESET /* Request get dequeued */
> +#define DWC3_REQUEST_DISCONNECTED -ESHUTDOWN /* Device is disconnected/disabled */
> +#define DWC3_REQUEST_STALL -EPIPE /* Bus or protocol error */
this is just obfuscation, pass the errors directly. Also, make sure
these are documented in the API.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: dwc3: add cancelled reason for dwc3 requests
2021-03-25 12:25 ` Felipe Balbi
@ 2021-03-25 21:00 ` Thinh Nguyen
0 siblings, 0 replies; 4+ messages in thread
From: Thinh Nguyen @ 2021-03-25 21:00 UTC (permalink / raw)
To: Felipe Balbi, Ray Chi, gregkh, Thinh Nguyen
Cc: linux-usb, linux-kernel, albertccwang
Felipe Balbi wrote:
> Hi,
>
> Ray Chi <raychi@google.com> writes:
>> diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
>> index 0cd281949970..a23e85bd3933 100644
>> --- a/drivers/usb/dwc3/gadget.h
>> +++ b/drivers/usb/dwc3/gadget.h
>> @@ -56,6 +56,12 @@ struct dwc3;
>>
>> /* Frame/Microframe Number Mask */
>> #define DWC3_FRNUMBER_MASK 0x3fff
>> +
>> +/* Cancel reason for dwc3 request */
>> +#define DWC3_REQUEST_DEQUEUED -ECONNRESET /* Request get dequeued */
>> +#define DWC3_REQUEST_DISCONNECTED -ESHUTDOWN /* Device is disconnected/disabled */
>> +#define DWC3_REQUEST_STALL -EPIPE /* Bus or protocol error */
>
> this is just obfuscation, pass the errors directly. Also, make sure
> these are documented in the API.
>
Note: we don't have documentation for error codes for gadget side, only
host. These are just the equivalent error codes from the host side.
What we currently have for host:
https://www.kernel.org/doc/html/latest/driver-api/usb/error-codes.html
Maybe someone can work on that in a separate patch.
Thanks,
Thinh
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: dwc3: add cancelled reason for dwc3 requests
2021-03-25 11:54 [PATCH] usb: dwc3: add cancelled reason for dwc3 requests Ray Chi
2021-03-25 12:25 ` Felipe Balbi
@ 2021-03-25 20:52 ` Thinh Nguyen
1 sibling, 0 replies; 4+ messages in thread
From: Thinh Nguyen @ 2021-03-25 20:52 UTC (permalink / raw)
To: Ray Chi, balbi, gregkh, Thinh Nguyen
Cc: linux-usb, linux-kernel, albertccwang
Hi Ray,
Ray Chi wrote:
> Currently, when dwc3 handles request cancelled, dwc3 just returns
> -ECONNRESET for all requests. It will cause USB class drivers can't
class drivers -> gadget driver or function driver.
> know if the requests are cancelled by other reasons.
>
> This patch will add the reason when the requests are cancelled.
>
> Signed-off-by: Ray Chi <raychi@google.com>
> ---
> drivers/usb/dwc3/gadget.c | 6 +++---
> drivers/usb/dwc3/gadget.h | 10 +++++++++-
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index e1442fc763e1..cc65fc064078 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_DEQUEUED);
>
> /* If ep isn't started, then there's no end transfer pending */
> if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING))
> @@ -1776,7 +1776,7 @@ 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_DEQUEUED);
>
> dep->flags &= ~DWC3_EP_WAIT_TRANSFER_COMPLETE;
>
> @@ -1848,7 +1848,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_STALL);
>
> 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..a23e85bd3933 100644
> --- a/drivers/usb/dwc3/gadget.h
> +++ b/drivers/usb/dwc3/gadget.h
> @@ -56,6 +56,12 @@ struct dwc3;
>
> /* Frame/Microframe Number Mask */
> #define DWC3_FRNUMBER_MASK 0x3fff
> +
> +/* Cancel reason for dwc3 request */
> +#define DWC3_REQUEST_DEQUEUED -ECONNRESET /* Request get dequeued */
> +#define DWC3_REQUEST_DISCONNECTED -ESHUTDOWN /* Device is disconnected/disabled */
> +#define DWC3_REQUEST_STALL -EPIPE /* Bus or protocol error */
> +
> /* -------------------------------------------------------------------------- */
>
> #define to_dwc3_request(r) (container_of(r, struct dwc3_request, request))
> @@ -90,15 +96,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, int reason)
> {
> struct dwc3_ep *dep = req->dep;
>
> req->status = DWC3_REQUEST_STATUS_CANCELLED;
My suggestion previously was to replace DWC3_REQUEST_STATUS_CANCELLED
with more specific status:
DWC3_REQUEST_STATUS_DISCONNECTED
DWC3_REQUEST_STATUS_DEQUEUED
DWC3_REQUEST_STATUS_STALLED
and set req->status = reason;
> + req->request.status = reason;
Not to update this here as this doesn't do anything. Update
dwc3_gadget_ep_cleanup_cancelled_requests() to have a switch case and
provide the appropriate giveback error code base on the status above.
> list_move_tail(&req->list, &dep->cancelled_list);
> }
>
>
BR,
Thinh
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-25 21:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 11:54 [PATCH] usb: dwc3: add cancelled reason for dwc3 requests Ray Chi
2021-03-25 12:25 ` Felipe Balbi
2021-03-25 21:00 ` Thinh Nguyen
2021-03-25 20:52 ` 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).