The function driver may queue new requests right after halting the endpoint (i.e. queue new requests while the endpoint is stalled). There's no restriction preventing it from doing so. However, dwc3 currently drops those requests after CLEAR_STALL. The driver should only drop started requests. Keep the pending requests in the pending list to resume and process them after the host issues ClearFeature(Halt) to the endpoint.

Cc: stable@vger.kernel.org Fixes: cb11ea56f37a ("usb: dwc3: gadget: Properly handle ClearFeature(halt)") Signed-off-by: Thinh Nguyen thinhn@synopsys.com --- Changes in v2: - Account for wedged endpoint - Account for CLEAR_FEATURE on stopped endpoints with pending requests (END_TRANSFER command won't be issued for stopped endpoints, so just kick pending request right after CLEAR_STALL)

drivers/usb/dwc3/gadget.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index c2a0f64f8d1e..c04f7b29535e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1628,8 +1628,13 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req) if (dep->flags & DWC3_EP_WAIT_TRANSFER_COMPLETE) return 0;

- /* Start the transfer only after the END_TRANSFER is completed */ - if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) { + /* + * Start the transfer only after the END_TRANSFER is completed + * and endpoint STALL is cleared. + */ + if ((dep->flags & DWC3_EP_END_TRANSFER_PENDING) || + (dep->flags & DWC3_EP_WEDGE) || + (dep->flags & DWC3_EP_STALL)) { dep->flags |= DWC3_EP_DELAY_START; return 0; } @@ -1836,13 +1841,14 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol) list_for_each_entry_safe(req, tmp, &dep->started_list, list) dwc3_gadget_move_cancelled_request(req);

- list_for_each_entry_safe(req, tmp, &dep->pending_list, list) - dwc3_gadget_move_cancelled_request(req); - - if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) { - dep->flags &= ~DWC3_EP_DELAY_START; + if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) dwc3_gadget_ep_cleanup_cancelled_requests(dep); - } + + if ((dep->flags & DWC3_EP_DELAY_START) && + !usb_endpoint_xfer_isoc(dep->endpoint.desc)) + __dwc3_gadget_kick_transfer(dep); + + dep->flags &= ~DWC3_EP_DELAY_START; }

return ret;

According the programming guide (for all DWC3 IPs), when the driver handles ClearFeature(halt) request, it should issue CLEAR_STALL command _after_ the END_TRANSFER command completes. The END_TRANSFER command may take some time to complete. So, delay the ClearFeature(halt) request control status stage and wait for END_TRANSFER command completion interrupt. Only after END_TRANSFER command completes that the driver may issue CLEAR_STALL command.

Cc: stable@vger.kernel.org Fixes: cb11ea56f37a ("usb: dwc3: gadget: Properly handle ClearFeature(halt)") Signed-off-by: Thinh Nguyen thinhn@synopsys.com --- Changes in v2: - None

drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/ep0.c | 16 ++++++++++++++++ drivers/usb/dwc3/gadget.c | 40 +++++++++++++++++++++++++++++++-------- drivers/usb/dwc3/gadget.h | 1 + 4 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 2f04b3e42bf1..eb026c9cca28 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -706,6 +706,7 @@ struct dwc3_ep { #define DWC3_EP_IGNORE_NEXT_NOSTREAM BIT(8) #define DWC3_EP_FORCE_RESTART_STREAM BIT(9) #define DWC3_EP_FIRST_STREAM_PRIMED BIT(10) +#define DWC3_EP_PENDING_CLEAR_STALL BIT(11)

/* This last one is specific to EP0 */ #define DWC3_EP0_DIR_IN BIT(31) diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index 59f2e8c31bd1..92bc1044e7ab 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -524,6 +524,11 @@ static int dwc3_ep0_handle_endpoint(struct dwc3 *dwc, ret = __dwc3_gadget_ep_set_halt(dep, set, true); if (ret) return -EINVAL; + + /* ClearFeature(Halt) may need delayed status */ + if (!set && (dep->flags & DWC3_EP_END_TRANSFER_PENDING)) + return USB_GADGET_DELAYED_STATUS; + break; default: return -EINVAL; @@ -1042,6 +1047,17 @@ static void dwc3_ep0_do_control_status(struct dwc3 *dwc, __dwc3_ep0_do_control_status(dwc, dep); }

+void dwc3_ep0_send_delayed_status(struct dwc3 *dwc) +{ + unsigned int direction = !dwc->ep0_expect_in; + + if (dwc->ep0state != EP0_STATUS_PHASE) + return; + + dwc->delayed_status = false; + __dwc3_ep0_do_control_status(dwc, dwc->eps[direction]); +} + static void dwc3_ep0_end_control_data(struct dwc3 *dwc, struct dwc3_ep *dep) { struct dwc3_gadget_ep_cmd_params params; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index c04f7b29535e..f9843ad93577 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1827,6 +1827,18 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol) return 0; }

+ dwc3_stop_active_transfer(dep, true, true); + + list_for_each_entry_safe(req, tmp, &dep->started_list, list) + dwc3_gadget_move_cancelled_request(req); + + if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) { + dep->flags |= DWC3_EP_PENDING_CLEAR_STALL; + return 0; + } + + dwc3_gadget_ep_cleanup_cancelled_requests(dep); + ret = dwc3_send_clear_stall_ep_cmd(dep); if (ret) { dev_err(dwc->dev, "failed to clear STALL on %s\n", @@ -1836,14 +1848,6 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol)

dep->flags &= ~(DWC3_EP_STALL | DWC3_EP_WEDGE);

- dwc3_stop_active_transfer(dep, true, true); - - list_for_each_entry_safe(req, tmp, &dep->started_list, list) - dwc3_gadget_move_cancelled_request(req); - - if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) - dwc3_gadget_ep_cleanup_cancelled_requests(dep); - if ((dep->flags & DWC3_EP_DELAY_START) && !usb_endpoint_xfer_isoc(dep->endpoint.desc)) __dwc3_gadget_kick_transfer(dep); @@ -3003,6 +3007,26 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING; dep->flags &= ~DWC3_EP_TRANSFER_STARTED; dwc3_gadget_ep_cleanup_cancelled_requests(dep); + + if (dep->flags & DWC3_EP_PENDING_CLEAR_STALL) { + struct dwc3 *dwc = dep->dwc; + + dep->flags &= ~DWC3_EP_PENDING_CLEAR_STALL; + if (dwc3_send_clear_stall_ep_cmd(dep)) { + struct usb_ep *ep0 = &dwc->eps[0]->endpoint; + + dev_err(dwc->dev, "failed to clear STALL on %s\n", + dep->name); + if (dwc->delayed_status) + __dwc3_gadget_ep0_set_halt(ep0, 1); + return; + } + + dep->flags &= ~(DWC3_EP_STALL | DWC3_EP_WEDGE); + if (dwc->delayed_status) + dwc3_ep0_send_delayed_status(dwc); + } + if ((dep->flags & DWC3_EP_DELAY_START) && !usb_endpoint_xfer_isoc(dep->endpoint.desc)) __dwc3_gadget_kick_transfer(dep); diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h index bd85eb7fa9ef..a7791cb827c4 100644 --- a/drivers/usb/dwc3/gadget.h +++ b/drivers/usb/dwc3/gadget.h @@ -113,6 +113,7 @@ int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value); int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct usb_request *request, gfp_t gfp_flags); int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol); +void dwc3_ep0_send_delayed_status(struct dwc3 *dwc);

/** * dwc3_gadget_ep_get_transfer_index - Gets transfer index from HW