| Message ID | 20220815213134.23783-1-quic_wcheng@quicinc.com |
|---|---|
| Headers | show |
| Series | Fix controller halt and endxfer timeout issues | expand |
Hi Wesley, On 8/15/2022, Wesley Cheng wrote: > Changes in v3: > - Modified the msleep() duration to ~2s versus ~10s due to the minimum > mdelay() value. > - Removed patch to modify DEP flags during dwc3_stop_active_transfer(). > This was not required after fixing the logic to allow EP xfercomplete > events to be handled on EP0. > - Added some changes to account for a cable disconnect scenario, where > dwc3_gadget_pullup() would not be executed to stop active transfers. > Needed to add some logic to the disconnect interrupt to ensure that we > cleanup/restart any pending SETUP transaction, so that we can clear the > EP0 delayed stop status. (if pending) > - Added patch to ensure that we don't proceed with umapping buffers > until the endxfer was actually sent. > > Changes in v2: > - Moved msleep() to before reading status register for halted state > - Fixed kernel bot errors > - Clearing DEP flags in __dwc3_stop_active_transfers() > - Added Suggested-by tags and link references to previous discussions > > This patch series addresses some issues seen while testing with the latest > soft disconnect implementation where EP events are allowed to process while > the controller halt is occurring. > > #1 > Since routines can now interweave, we can see that the soft disconnect can > occur while conndone is being serviced. This leads to a controller halt > timeout, as the soft disconnect clears the DEP flags, for which conndone > interrupt handler will issue a __dwc3_ep_enable(ep0), that leads to > re-issuing the set ep config command for every endpoint. > > #2 > Function drivers can ask for a delayed_status phase, while it processes the > received SETUP packet. This can lead to large delays when handling the > soft disconnect routine. To improve the timing, forcefully send the status > phase, as we are going to disconnect from the host. > > #3 > Ensure that local interrupts are left enabled, so that EP0 events can be > processed while the soft disconnect/dequeue is happening. > > #4 > Since EP0 events can occur during controller halt, it may increase the time > needed for the controller to fully stop. > > #5 > Account for cable disconnect scenarios where nothing may cause the endxfer > retry if DWC3_EP_DELAY_STOP is set. > > #6 > Avoid unmapping pending USB requests that were never stopped. This would > lead to a potential SMMU fault. > > Wesley Cheng (8): > usb: dwc3: Do not service EP0 and conndone events if soft disconnected > usb: dwc3: gadget: Force sending delayed status during soft disconnect > usb: dwc3: gadget: Synchronize IRQ between soft connect/disconnect > usb: dwc3: gadget: Continue handling EP0 xfercomplete events > usb: dwc3: Avoid unmapping USB requests if endxfer is not complete > usb: dwc3: Increase DWC3 controller halt timeout > usb: dwc3: gadget: Skip waiting for CMDACT cleared during endxfer > usb: dwc3: gadget: Submit endxfer command if delayed during disconnect > > drivers/usb/dwc3/core.c | 4 ---- > drivers/usb/dwc3/core.h | 3 +++ > drivers/usb/dwc3/ep0.c | 11 ++++++--- > drivers/usb/dwc3/gadget.c | 48 +++++++++++++++++++++++++++++++++------ > 4 files changed, 52 insertions(+), 14 deletions(-) > Beside the comment on [patch 6/8] increasing halt timeout, the rest looks fine to me. Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> Thanks for the patches! Thinh