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
