Wesley Cheng wrote: > > > On 3/19/2021 5:40 PM, Thinh Nguyen wrote: >> Hi, >> >> Wesley Cheng wrote: >>> The current dwc3_gadget_reset_interrupt() will stop any active >>> transfers, but only addresses blocking of EP queuing for while we are >>> coming from a disconnected scenario, i.e. after receiving the disconnect >>> event. If the host decides to issue a bus reset on the device, the >>> connected parameter will still be set to true, allowing for EP queuing >>> to continue while we are disabling the functions. To avoid this, set the >>> connected flag to false until the stop active transfers is complete. >>> >>> Signed-off-by: Wesley Cheng <wcheng@xxxxxxxxxxxxxx> >>> --- >>> drivers/usb/dwc3/gadget.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index 6e14fdc..d5ed0f69 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -3327,6 +3327,15 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) >>> u32 reg; >>> >>> /* >>> + * Ideally, dwc3_reset_gadget() would trigger the function >>> + * drivers to stop any active transfers through ep disable. >>> + * However, for functions which defer ep disable, such as mass >>> + * storage, we will need to rely on the call to stop active >>> + * transfers here, and avoid allowing of request queuing. >>> + */ >>> + dwc->connected = false; >>> + >>> + /* >>> * WORKAROUND: DWC3 revisions <1.88a have an issue which >>> * would cause a missing Disconnect Event if there's a >>> * pending Setup Packet in the FIFO. >>> >> >> This doesn't look right. Did you have rebase issue with your local >> change again? >> >> BR, >> Thinh >> > Hi Thinh, > > This was rebased on Greg's usb-linus branch, which has commit > f09ddcfcb8c5 ("usb: dwc3: gadget: Prevent EP queuing while stopping > transfers") merged. Ah I see. > > commit f09ddcfcb8c5 moved the dwc->connected = true to after we have > finished stop active transfers. However, this change will also ensure > that the connected flag is set to false to ensure that when we call stop > active transfers, nothing can prepare TRBs. (previous commit only > addresses the case where we get the reset interrupt when coming from a > disconnected state) > That still doesn't address this issue. Because: 1) We're still protected by the spin_lock_irq*(), so this change doesn't make any difference while handling an event. 2) We don't enable the interrupt for END_TRANSFER command completion when doing dwc3_stop_active_transfers(), the DWC3_EP_END_TRANSFER_PENDING flag will not be set to prevent preparing new requests. We should do dwc->connected = true when we handle connection_done interrupt instead. The END_TRANSFER command should complete before this. Thanks, Thinh