Hi Greg, On 3/9/2022 2:21 AM, Greg KH wrote: > On Tue, Mar 08, 2022 at 04:41:48PM -0800, Wesley Cheng wrote: >> From: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> >> >> If the request being dequeued is currently active, then the current >> logic is to issue a stop transfer command, and allow the command >> completion to cleanup the cancelled list. The DWC3 controller will >> run into an end transfer command timeout if there is an ongoing EP0 >> transaction. If this is the case, wait for the EP0 completion event >> before proceeding to retry the endxfer command again. >> >> Co-developed-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx> >> Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx> >> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> > > > You sent this twice? What is the differences between the patches? > Sorry for sending it twice. It was a mistake on my end, where I thought the patch wasn't getting sent out by our email server, so I sent it out several times to check. Ended up just showing up after 30 mins or so :/. > And as you sent it, your signed-off-by needs to be at the end, as per > the kernel documentation. > I'll fix this. >> --- >> Patch discussion below: >> https://lore.kernel.org/linux-usb/1644836933-141376-1-git-send-email-dh10.jung@xxxxxxxxxxx/T/#t > > So this is a v2? > > This is an entirely different change, but the change was discussed in the above thread. >> >> drivers/usb/dwc3/core.h | 2 +- >> drivers/usb/dwc3/ep0.c | 14 ++++++++++++++ >> drivers/usb/dwc3/gadget.c | 13 ++++++++----- >> drivers/usb/dwc3/gadget.h | 1 + >> 4 files changed, 24 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index eb9c1efced05..f557f5f36a7f 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -736,7 +736,7 @@ struct dwc3_ep { >> #define DWC3_EP_FIRST_STREAM_PRIMED BIT(10) >> #define DWC3_EP_PENDING_CLEAR_STALL BIT(11) >> #define DWC3_EP_TXFIFO_RESIZED BIT(12) >> - >> +#define DWC3_EP_DELAY_STOP BIT(13) > > Why did you loose the blank line? > I can add that back in. Thanks Wesley Cheng >> /* 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 658739410992..1064be5518f6 100644 >> --- a/drivers/usb/dwc3/ep0.c >> +++ b/drivers/usb/dwc3/ep0.c >> @@ -271,6 +271,7 @@ void dwc3_ep0_out_start(struct dwc3 *dwc) >> { >> struct dwc3_ep *dep; >> int ret; >> + int i; >> >> complete(&dwc->ep0_in_setup); >> >> @@ -279,6 +280,19 @@ void dwc3_ep0_out_start(struct dwc3 *dwc) >> DWC3_TRBCTL_CONTROL_SETUP, false); >> ret = dwc3_ep0_start_trans(dep); >> WARN_ON(ret < 0); >> + for (i = 2; i < DWC3_ENDPOINTS_NUM; i++) { >> + struct dwc3_ep *dwc3_ep; >> + >> + dwc3_ep = dwc->eps[i]; >> + if (!dwc3_ep) >> + continue; >> + >> + if (!(dwc3_ep->flags & DWC3_EP_DELAY_STOP)) >> + continue; >> + >> + dwc3_ep->flags &= ~DWC3_EP_DELAY_STOP; >> + dwc3_stop_active_transfer(dwc3_ep, true, true); >> + } >> } >> >> static struct dwc3_ep *dwc3_wIndex_to_dep(struct dwc3 *dwc, __le16 wIndex_le) >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index a0c883f19a41..ccef508b1296 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -654,9 +654,6 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep, unsigned int action) >> return dwc3_send_gadget_ep_cmd(dep, DWC3_DEPCMD_SETEPCONFIG, ¶ms); >> } >> >> -static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, >> - bool interrupt); >> - >> /** >> * dwc3_gadget_calc_tx_fifo_size - calculates the txfifo size value >> * @dwc: pointer to the DWC3 context >> @@ -1899,6 +1896,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req) >> */ >> if ((dep->flags & DWC3_EP_END_TRANSFER_PENDING) || >> (dep->flags & DWC3_EP_WEDGE) || >> + (dep->flags & DWC3_EP_DELAY_STOP) || >> (dep->flags & DWC3_EP_STALL)) { >> dep->flags |= DWC3_EP_DELAY_START; >> return 0; >> @@ -2033,6 +2031,9 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, >> if (r == req) { >> struct dwc3_request *t; >> >> + if (dwc->ep0state != EP0_SETUP_PHASE && !dwc->delayed_status) >> + dep->flags |= DWC3_EP_DELAY_STOP; >> + >> /* wait until it is processed */ >> dwc3_stop_active_transfer(dep, true, true); >> >> @@ -2116,7 +2117,8 @@ 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, DWC3_REQUEST_STATUS_STALLED); >> >> - if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) { >> + if (dep->flags & DWC3_EP_END_TRANSFER_PENDING || >> + (dep->flags & DWC3_EP_DELAY_STOP)) { >> dep->flags |= DWC3_EP_PENDING_CLEAR_STALL; >> return 0; >> } >> @@ -3596,7 +3598,7 @@ static void dwc3_reset_gadget(struct dwc3 *dwc) >> } >> } >> >> -static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, >> +void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, >> bool interrupt) > > This is a horrid api (2 booleans?) But you aren't adding it so I guess > we can live with it :( > > thanks, > > greg k-h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project