usb: dwc3: gadget: Improve END_TRANSFER handling
Subsystems (2)
Thinh Nguyen
maintainercover usb: dwc3: gadget: Improve END_TRANSFER handling
This patch series improves the handling of the END_TRANSFER command:
1) Don't issue multiple END_TRANSFER commands while the previous hasn't completed.
2) Don't issue START_TRANSFER command while END_TRANSFER hasn't completed
3) Remove arbirary 100us delay
Thinh Nguyen (3):
usb: dwc3: gadget: Check END_TRANSFER completion
usb: dwc3: gadget: Delay starting transfer
usb: dwc3: gadget: Remove END_TRANSFER delay
drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/ep0.c | 4 +++- drivers/usb/dwc3/gadget.c | 34 +++++++++++++++++++++------------- 3 files changed, 26 insertions(+), 14 deletions(-)
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 1/3 usb: dwc3: gadget: Check END_TRANSFER completion
While the END_TRANSFER command is sent but not completed, any request dequeue during this time will cause the driver to issue the END_TRANSFER command. The driver needs to submit the command only once to stop the controller from processing further. The controller may take more time to process the same command multiple times unnecessarily. Let's add a flag DWC3_EP_END_TRANSFER_PENDING to check for this condition.
--- drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/ep0.c | 4 +++- drivers/usb/dwc3/gadget.c | 6 +++++- 3 files changed, 9 insertions(+), 2 deletions(-)
drivers/usb/dwc3/core.h +1 -0
@@ -688,6 +688,7 @@ struct dwc3_ep {#define DWC3_EP_STALL BIT(1)#define DWC3_EP_WEDGE BIT(2)#define DWC3_EP_TRANSFER_STARTED BIT(3) +#define DWC3_EP_END_TRANSFER_PENDING BIT(4)#define DWC3_EP_PENDING_REQUEST BIT(5)/* This last one is specific to EP0 */
drivers/usb/dwc3/ep0.c +3 -1
@@ -1136,8 +1136,10 @@ void dwc3_ep0_interrupt(struct dwc3 *dwc,case DWC3_DEPEVT_EPCMDCMPLT : cmd =DEPEVT_PARAMETER_CMD (event->parameters); -if (cmd ==DWC3_DEPCMD_ENDTRANSFER ) +if (cmd ==DWC3_DEPCMD_ENDTRANSFER ) { + dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING ; dep->flags &= ~DWC3_EP_TRANSFER_STARTED ; + }break ; } }
drivers/usb/dwc3/gadget.c +5 -1
@@ -2621,6 +2621,7 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, cmd =DEPEVT_PARAMETER_CMD (event->parameters);if (cmd ==DWC3_DEPCMD_ENDTRANSFER ) { + dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING ; dep->flags &= ~DWC3_EP_TRANSFER_STARTED ;dwc3_gadget_ep_cleanup_cancelled_requests (dep); }
@@ -2679,7 +2680,8 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,u32 cmd;int ret; -if (!(dep->flags &DWC3_EP_TRANSFER_STARTED )) +if (!(dep->flags &DWC3_EP_TRANSFER_STARTED ) || + (dep->flags &DWC3_EP_END_TRANSFER_PENDING ))return ;/*
@@ -2724,6 +2726,8 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,if (!interrupt) dep->flags &= ~DWC3_EP_TRANSFER_STARTED ; +else + dep->flags |=DWC3_EP_END_TRANSFER_PENDING ;if (dwc3_is_usb31 (dwc) || dwc->revision <DWC3_REVISION_310A )udelay (100 );
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 2/3 usb: dwc3: gadget: Delay starting transfer
In reply to Thinh NguyenIf the END_TRANSFER command hasn't completed yet, then don't send the START_TRANSFER command. The controller may not be able to start if that's the case. Some controller revisions depend on this. See commit 76a638f8ac0d ("usb: dwc3: gadget: wait for End Transfer to complete"). Let's only send START_TRANSFER command after the END_TRANSFER command had completed.
--- drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/gadget.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
drivers/usb/dwc3/core.h +1 -0
@@ -690,6 +690,7 @@ struct dwc3_ep {#define DWC3_EP_TRANSFER_STARTED BIT(3)#define DWC3_EP_END_TRANSFER_PENDING BIT(4)#define DWC3_EP_PENDING_REQUEST BIT(5) +#define DWC3_EP_DELAY_START BIT(6)/* This last one is specific to EP0 */ #define DWC3_EP0_DIR_IN BIT(31)
drivers/usb/dwc3/gadget.c +11 -0
@@ -1450,6 +1450,12 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)list_add_tail (&req->list, &dep->pending_list); req->status =DWC3_REQUEST_STATUS_QUEUED ; +/* Start the transfer only after the END_TRANSFER is completed */ +if (dep->flags &DWC3_EP_END_TRANSFER_PENDING ) { + dep->flags |=DWC3_EP_DELAY_START ; +return 0 ; + } +/* * NOTICE: Isochronous endpoints should NEVER be prestarted. We must * wait for a XferNotReady event so we will know what's the current
@@ -2624,6 +2630,11 @@ 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_DELAY_START ) && + !usb_endpoint_xfer_isoc (dep->endpoint.desc)) +__dwc3_gadget_kick_transfer (dep); + + dep->flags &= ~DWC3_EP_DELAY_START ; }break ;case DWC3_DEPEVT_STREAMEVT :
Signature · 1 line
2.11.0
Thinh Nguyen
maintainerPATCH 3/3 usb: dwc3: gadget: Remove END_TRANSFER delay
In reply to Thinh NguyenWe had a 100us delay to synchronize the END_TRANSFER command completion before giving back requests to the function drivers. Now, the controller driver can handle cancelled TRBs with the requests' cancelled_list and it can also wait until the END_TRANSFER completion before starting new transfers. Synchronization can simply base on the controller's command completion interrupt. The 100us delay is no longer needed. Remove this arbitrary delay.
--- drivers/usb/dwc3/gadget.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)
drivers/usb/dwc3/gadget.c +5 -12
@@ -2702,16 +2702,13 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,* much trouble synchronizing between us and gadget driver. * * We have discussed this with the IP Provider and it was -* suggested to giveback all requests here, but give HW some -* extra time to synchronize with the interconnect. We're using -* an arbitrary 100us delay for that. +* suggested to giveback all requests here. * * Note also that a similar handling was tested by Synopsys * (thanks a lot Paul) and nothing bad has come out of it. -* In short, what we're doing is: -* -* - Issue EndTransfer WITH CMDIOC bit set -* - Wait 100us +* In short, what we're doing is issuing EndTransfer with +* CMDIOC bit set and delay kicking transfer until the +* EndTransfer command had completed. * * As of IP version 3.10a of the DWC_usb3 IP, the controller * supports a mode to work around the above limitation. The
@@ -2720,8 +2717,7 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,* by writing GUCTL2[14]. This polling is already done in the * dwc3_send_gadget_ep_cmd() function so if the mode is * enabled, the EndTransfer command will have completed upon -* returning from this function and we don't need to delay for -* 100us. +* returning from this function. * * This mode is NOT available on the DWC_usb31 IP. */
@@ -2739,9 +2735,6 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, dep->flags &= ~DWC3_EP_TRANSFER_STARTED ; else dep->flags |= DWC3_EP_END_TRANSFER_PENDING; - -if (dwc3_is_usb31 (dwc) || dwc->revision <DWC3_REVISION_310A ) -udelay (100 ); }static void dwc3_clear_stall_all_ep (struct dwc3 *dwc)
Signature · 1 line
2.11.0