Re: [PATCH v2] usb: dwc3: Stop active transfers before halting the controller

2020-09-01 Thread Thinh Nguyen
Wesley Cheng wrote:
>
> On 9/1/2020 3:14 PM, Wesley Cheng wrote:
>>
>> On 8/29/2020 2:35 PM, Thinh Nguyen wrote:
>>> Wesley Cheng wrote:
 In the DWC3 databook, for a device initiated disconnect or bus reset, the
 driver is required to send dependxfer commands for any pending transfers.
 In addition, before the controller can move to the halted state, the SW
 needs to acknowledge any pending events.  If the controller is not halted
 properly, there is a chance the controller will continue accessing stale or
 freed TRBs and buffers.

 Signed-off-by: Wesley Cheng 

 ---
 Changes in v2:
  - Moved cleanup code to the pullup() API to differentiate between device
disconnect and hibernation.
  - Added cleanup code to the bus reset case as well.
  - Verified the move to pullup() did not reproduce the problen using the
same test sequence.

 Verified fix by adding a check for ETIMEDOUT during the run stop call.
 Shell script writing to the configfs UDC file to trigger disconnect and
 connect.  Batch script to have PC execute data transfers over adb (ie adb
 push)  After a few iterations, we'd run into a scenario where the
 controller wasn't halted.  With the following change, no failed halts after
 many iterations.
 ---
  drivers/usb/dwc3/ep0.c|  2 +-
  drivers/usb/dwc3/gadget.c | 52 ++-
  2 files changed, 52 insertions(+), 2 deletions(-)

 diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
 index 59f2e8c31bd1..456aa87e8778 100644
 --- a/drivers/usb/dwc3/ep0.c
 +++ b/drivers/usb/dwc3/ep0.c
 @@ -197,7 +197,7 @@ int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct 
 usb_request *request,
int ret;
  
spin_lock_irqsave(>lock, flags);
 -  if (!dep->endpoint.desc) {
 +  if (!dep->endpoint.desc || !dwc->pullups_connected) {
dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
dep->name);
ret = -ESHUTDOWN;
 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 3ab6f118c508..df8d89d6bdc9 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -1516,7 +1516,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep 
 *dep, struct dwc3_request *req)
  {
struct dwc3 *dwc = dep->dwc;
  
 -  if (!dep->endpoint.desc) {
 +  if (!dep->endpoint.desc || !dwc->pullups_connected) {
dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
dep->name);
return -ESHUTDOWN;
 @@ -1926,6 +1926,24 @@ static int dwc3_gadget_set_selfpowered(struct 
 usb_gadget *g,
return 0;
  }
  
 +static void dwc3_stop_active_transfers(struct dwc3 *dwc)
 +{
 +  u32 epnum;
 +
 +  for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
 +  struct dwc3_ep *dep;
 +
 +  dep = dwc->eps[epnum];
 +  if (!dep)
 +  continue;
 +
 +  if (!(dep->flags & DWC3_EP_ENABLED))
 +  continue;
>>> Don't do the enabled check here. Let the dwc3_stop_active_transfer() do
>>> that checking.
>>>
>> Hi Thinh,
>>
>> Thanks for the detailed review, as always.  Got it, we can allow that to
>> catch it based off the DWC3_EP_TRANSFER_STARTED.
>>
 +
 +  dwc3_remove_requests(dwc, dep);
 +  }
 +}
 +
  static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
  {
u32 reg;
 @@ -1994,9 +2012,39 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, 
 int is_on)
}
}
  
 +  /*
 +   * Synchronize and disable any further event handling while controller
 +   * is being enabled/disabled.
 +   */
 +  disable_irq(dwc->irq_gadget);
>>> I think it's better to do dwc3_gadget_disable_irq(). This only stops
>>> handling events. Although it's unlikely, the controller may still
>>> generate events before it's halted.
>>>
>> I think its better if we can do both.  At least with the disable_irq()
>> call present, we can ensure the irq handlers are complete, or we can do
>> as Felipe suggested, and first disable the controller events (using
>> dwc3_gadget_disable_irq()) and then calling synchronize_irq().
>>
>> The concern I had is the pullup() API updating the lpos, and the hardirq
>> handler referencing it to update the evt buf cache and waking up the
>> threaded irq handler. (since we don't clear the evt->count explicitly,
>> it may reference empty/stale events)

I see, then we should do both.

>>
spin_lock_irqsave(>lock, flags);
 +
 +  /* Controller is not halted until pending events are acknowledged */
 +  if (!is_on) {
 +  u32 reg;
 +

Re: [PATCH v2] usb: dwc3: Stop active transfers before halting the controller

2020-09-01 Thread Wesley Cheng



On 9/1/2020 3:14 PM, Wesley Cheng wrote:
> 
> 
> On 8/29/2020 2:35 PM, Thinh Nguyen wrote:
>> Wesley Cheng wrote:
>>> In the DWC3 databook, for a device initiated disconnect or bus reset, the
>>> driver is required to send dependxfer commands for any pending transfers.
>>> In addition, before the controller can move to the halted state, the SW
>>> needs to acknowledge any pending events.  If the controller is not halted
>>> properly, there is a chance the controller will continue accessing stale or
>>> freed TRBs and buffers.
>>>
>>> Signed-off-by: Wesley Cheng 
>>>
>>> ---
>>> Changes in v2:
>>>  - Moved cleanup code to the pullup() API to differentiate between device
>>>disconnect and hibernation.
>>>  - Added cleanup code to the bus reset case as well.
>>>  - Verified the move to pullup() did not reproduce the problen using the
>>>same test sequence.
>>>
>>> Verified fix by adding a check for ETIMEDOUT during the run stop call.
>>> Shell script writing to the configfs UDC file to trigger disconnect and
>>> connect.  Batch script to have PC execute data transfers over adb (ie adb
>>> push)  After a few iterations, we'd run into a scenario where the
>>> controller wasn't halted.  With the following change, no failed halts after
>>> many iterations.
>>> ---
>>>  drivers/usb/dwc3/ep0.c|  2 +-
>>>  drivers/usb/dwc3/gadget.c | 52 ++-
>>>  2 files changed, 52 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>>> index 59f2e8c31bd1..456aa87e8778 100644
>>> --- a/drivers/usb/dwc3/ep0.c
>>> +++ b/drivers/usb/dwc3/ep0.c
>>> @@ -197,7 +197,7 @@ int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct 
>>> usb_request *request,
>>> int ret;
>>>  
>>> spin_lock_irqsave(>lock, flags);
>>> -   if (!dep->endpoint.desc) {
>>> +   if (!dep->endpoint.desc || !dwc->pullups_connected) {
>>> dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
>>> dep->name);
>>> ret = -ESHUTDOWN;
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 3ab6f118c508..df8d89d6bdc9 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -1516,7 +1516,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep 
>>> *dep, struct dwc3_request *req)
>>>  {
>>> struct dwc3 *dwc = dep->dwc;
>>>  
>>> -   if (!dep->endpoint.desc) {
>>> +   if (!dep->endpoint.desc || !dwc->pullups_connected) {
>>> dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
>>> dep->name);
>>> return -ESHUTDOWN;
>>> @@ -1926,6 +1926,24 @@ static int dwc3_gadget_set_selfpowered(struct 
>>> usb_gadget *g,
>>> return 0;
>>>  }
>>>  
>>> +static void dwc3_stop_active_transfers(struct dwc3 *dwc)
>>> +{
>>> +   u32 epnum;
>>> +
>>> +   for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
>>> +   struct dwc3_ep *dep;
>>> +
>>> +   dep = dwc->eps[epnum];
>>> +   if (!dep)
>>> +   continue;
>>> +
>>> +   if (!(dep->flags & DWC3_EP_ENABLED))
>>> +   continue;
>>
>> Don't do the enabled check here. Let the dwc3_stop_active_transfer() do
>> that checking.
>>
> 
> Hi Thinh,
> 
> Thanks for the detailed review, as always.  Got it, we can allow that to
> catch it based off the DWC3_EP_TRANSFER_STARTED.
> 
>>> +
>>> +   dwc3_remove_requests(dwc, dep);
>>> +   }
>>> +}
>>> +
>>>  static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>>>  {
>>> u32 reg;
>>> @@ -1994,9 +2012,39 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, 
>>> int is_on)
>>> }
>>> }
>>>  
>>> +   /*
>>> +* Synchronize and disable any further event handling while controller
>>> +* is being enabled/disabled.
>>> +*/
>>> +   disable_irq(dwc->irq_gadget);
>>
>> I think it's better to do dwc3_gadget_disable_irq(). This only stops
>> handling events. Although it's unlikely, the controller may still
>> generate events before it's halted.
>>
> 
> I think its better if we can do both.  At least with the disable_irq()
> call present, we can ensure the irq handlers are complete, or we can do
> as Felipe suggested, and first disable the controller events (using
> dwc3_gadget_disable_irq()) and then calling synchronize_irq().
> 
> The concern I had is the pullup() API updating the lpos, and the hardirq
> handler referencing it to update the evt buf cache and waking up the
> threaded irq handler. (since we don't clear the evt->count explicitly,
> it may reference empty/stale events)
> 
>>> spin_lock_irqsave(>lock, flags);
>>> +
>>> +   /* Controller is not halted until pending events are acknowledged */
>>> +   if (!is_on) {
>>> +   u32 reg;
>>> +
>>> +   __dwc3_gadget_ep_disable(dwc->eps[0]);
>>> +   __dwc3_gadget_ep_disable(dwc->eps[1]);
>>
>> You 

Re: [PATCH v2] usb: dwc3: Stop active transfers before halting the controller

2020-09-01 Thread Wesley Cheng



On 8/29/2020 2:35 PM, Thinh Nguyen wrote:
> Wesley Cheng wrote:
>> In the DWC3 databook, for a device initiated disconnect or bus reset, the
>> driver is required to send dependxfer commands for any pending transfers.
>> In addition, before the controller can move to the halted state, the SW
>> needs to acknowledge any pending events.  If the controller is not halted
>> properly, there is a chance the controller will continue accessing stale or
>> freed TRBs and buffers.
>>
>> Signed-off-by: Wesley Cheng 
>>
>> ---
>> Changes in v2:
>>  - Moved cleanup code to the pullup() API to differentiate between device
>>disconnect and hibernation.
>>  - Added cleanup code to the bus reset case as well.
>>  - Verified the move to pullup() did not reproduce the problen using the
>>same test sequence.
>>
>> Verified fix by adding a check for ETIMEDOUT during the run stop call.
>> Shell script writing to the configfs UDC file to trigger disconnect and
>> connect.  Batch script to have PC execute data transfers over adb (ie adb
>> push)  After a few iterations, we'd run into a scenario where the
>> controller wasn't halted.  With the following change, no failed halts after
>> many iterations.
>> ---
>>  drivers/usb/dwc3/ep0.c|  2 +-
>>  drivers/usb/dwc3/gadget.c | 52 ++-
>>  2 files changed, 52 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>> index 59f2e8c31bd1..456aa87e8778 100644
>> --- a/drivers/usb/dwc3/ep0.c
>> +++ b/drivers/usb/dwc3/ep0.c
>> @@ -197,7 +197,7 @@ int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct 
>> usb_request *request,
>>  int ret;
>>  
>>  spin_lock_irqsave(>lock, flags);
>> -if (!dep->endpoint.desc) {
>> +if (!dep->endpoint.desc || !dwc->pullups_connected) {
>>  dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
>>  dep->name);
>>  ret = -ESHUTDOWN;
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 3ab6f118c508..df8d89d6bdc9 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1516,7 +1516,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
>> struct dwc3_request *req)
>>  {
>>  struct dwc3 *dwc = dep->dwc;
>>  
>> -if (!dep->endpoint.desc) {
>> +if (!dep->endpoint.desc || !dwc->pullups_connected) {
>>  dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
>>  dep->name);
>>  return -ESHUTDOWN;
>> @@ -1926,6 +1926,24 @@ static int dwc3_gadget_set_selfpowered(struct 
>> usb_gadget *g,
>>  return 0;
>>  }
>>  
>> +static void dwc3_stop_active_transfers(struct dwc3 *dwc)
>> +{
>> +u32 epnum;
>> +
>> +for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
>> +struct dwc3_ep *dep;
>> +
>> +dep = dwc->eps[epnum];
>> +if (!dep)
>> +continue;
>> +
>> +if (!(dep->flags & DWC3_EP_ENABLED))
>> +continue;
> 
> Don't do the enabled check here. Let the dwc3_stop_active_transfer() do
> that checking.
> 

Hi Thinh,

Thanks for the detailed review, as always.  Got it, we can allow that to
catch it based off the DWC3_EP_TRANSFER_STARTED.

>> +
>> +dwc3_remove_requests(dwc, dep);
>> +}
>> +}
>> +
>>  static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>>  {
>>  u32 reg;
>> @@ -1994,9 +2012,39 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, 
>> int is_on)
>>  }
>>  }
>>  
>> +/*
>> + * Synchronize and disable any further event handling while controller
>> + * is being enabled/disabled.
>> + */
>> +disable_irq(dwc->irq_gadget);
> 
> I think it's better to do dwc3_gadget_disable_irq(). This only stops
> handling events. Although it's unlikely, the controller may still
> generate events before it's halted.
> 

I think its better if we can do both.  At least with the disable_irq()
call present, we can ensure the irq handlers are complete, or we can do
as Felipe suggested, and first disable the controller events (using
dwc3_gadget_disable_irq()) and then calling synchronize_irq().

The concern I had is the pullup() API updating the lpos, and the hardirq
handler referencing it to update the evt buf cache and waking up the
threaded irq handler. (since we don't clear the evt->count explicitly,
it may reference empty/stale events)

>>  spin_lock_irqsave(>lock, flags);
>> +
>> +/* Controller is not halted until pending events are acknowledged */
>> +if (!is_on) {
>> +u32 reg;
>> +
>> +__dwc3_gadget_ep_disable(dwc->eps[0]);
>> +__dwc3_gadget_ep_disable(dwc->eps[1]);
> 
> You can just do __dwc3_gadget_stop(), and do that after
> dwc3_stop_active_transfers().
> 

Got it.

>> +
>> +/*
>> + * The databook 

Re: [PATCH v2] usb: dwc3: Stop active transfers before halting the controller

2020-08-29 Thread Thinh Nguyen
Wesley Cheng wrote:
> In the DWC3 databook, for a device initiated disconnect or bus reset, the
> driver is required to send dependxfer commands for any pending transfers.
> In addition, before the controller can move to the halted state, the SW
> needs to acknowledge any pending events.  If the controller is not halted
> properly, there is a chance the controller will continue accessing stale or
> freed TRBs and buffers.
>
> Signed-off-by: Wesley Cheng 
>
> ---
> Changes in v2:
>  - Moved cleanup code to the pullup() API to differentiate between device
>disconnect and hibernation.
>  - Added cleanup code to the bus reset case as well.
>  - Verified the move to pullup() did not reproduce the problen using the
>same test sequence.
>
> Verified fix by adding a check for ETIMEDOUT during the run stop call.
> Shell script writing to the configfs UDC file to trigger disconnect and
> connect.  Batch script to have PC execute data transfers over adb (ie adb
> push)  After a few iterations, we'd run into a scenario where the
> controller wasn't halted.  With the following change, no failed halts after
> many iterations.
> ---
>  drivers/usb/dwc3/ep0.c|  2 +-
>  drivers/usb/dwc3/gadget.c | 52 ++-
>  2 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 59f2e8c31bd1..456aa87e8778 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -197,7 +197,7 @@ int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct 
> usb_request *request,
>   int ret;
>  
>   spin_lock_irqsave(>lock, flags);
> - if (!dep->endpoint.desc) {
> + if (!dep->endpoint.desc || !dwc->pullups_connected) {
>   dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
>   dep->name);
>   ret = -ESHUTDOWN;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 3ab6f118c508..df8d89d6bdc9 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1516,7 +1516,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
> struct dwc3_request *req)
>  {
>   struct dwc3 *dwc = dep->dwc;
>  
> - if (!dep->endpoint.desc) {
> + if (!dep->endpoint.desc || !dwc->pullups_connected) {
>   dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
>   dep->name);
>   return -ESHUTDOWN;
> @@ -1926,6 +1926,24 @@ static int dwc3_gadget_set_selfpowered(struct 
> usb_gadget *g,
>   return 0;
>  }
>  
> +static void dwc3_stop_active_transfers(struct dwc3 *dwc)
> +{
> + u32 epnum;
> +
> + for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
> + struct dwc3_ep *dep;
> +
> + dep = dwc->eps[epnum];
> + if (!dep)
> + continue;
> +
> + if (!(dep->flags & DWC3_EP_ENABLED))
> + continue;

Don't do the enabled check here. Let the dwc3_stop_active_transfer() do
that checking.

> +
> + dwc3_remove_requests(dwc, dep);
> + }
> +}
> +
>  static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>  {
>   u32 reg;
> @@ -1994,9 +2012,39 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, 
> int is_on)
>   }
>   }
>  
> + /*
> +  * Synchronize and disable any further event handling while controller
> +  * is being enabled/disabled.
> +  */
> + disable_irq(dwc->irq_gadget);

I think it's better to do dwc3_gadget_disable_irq(). This only stops
handling events. Although it's unlikely, the controller may still
generate events before it's halted.

>   spin_lock_irqsave(>lock, flags);
> +
> + /* Controller is not halted until pending events are acknowledged */
> + if (!is_on) {
> + u32 reg;
> +
> + __dwc3_gadget_ep_disable(dwc->eps[0]);
> + __dwc3_gadget_ep_disable(dwc->eps[1]);

You can just do __dwc3_gadget_stop(), and do that after
dwc3_stop_active_transfers().

> +
> + /*
> +  * The databook explicitly mentions for a device-initiated
> +  * disconnect sequence, the SW needs to ensure that it ends any
> +  * active transfers.
> +  */
> + dwc3_stop_active_transfers(dwc);
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
> + reg &= DWC3_GEVNTCOUNT_MASK;

Can we use another variable "count" instead of reusing reg to make it a
little clearer?

> + if (reg > 0) {
> + dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), reg);
> + dwc->ev_buf->lpos = (dwc->ev_buf->lpos + reg) %
> + dwc->ev_buf->length;
> + }
> + }
> +
>   ret = dwc3_gadget_run_stop(dwc, is_on, false);
>   spin_unlock_irqrestore(>lock, 

[PATCH v2] usb: dwc3: Stop active transfers before halting the controller

2020-08-28 Thread Wesley Cheng
In the DWC3 databook, for a device initiated disconnect or bus reset, the
driver is required to send dependxfer commands for any pending transfers.
In addition, before the controller can move to the halted state, the SW
needs to acknowledge any pending events.  If the controller is not halted
properly, there is a chance the controller will continue accessing stale or
freed TRBs and buffers.

Signed-off-by: Wesley Cheng 

---
Changes in v2:
 - Moved cleanup code to the pullup() API to differentiate between device
   disconnect and hibernation.
 - Added cleanup code to the bus reset case as well.
 - Verified the move to pullup() did not reproduce the problen using the
   same test sequence.

Verified fix by adding a check for ETIMEDOUT during the run stop call.
Shell script writing to the configfs UDC file to trigger disconnect and
connect.  Batch script to have PC execute data transfers over adb (ie adb
push)  After a few iterations, we'd run into a scenario where the
controller wasn't halted.  With the following change, no failed halts after
many iterations.
---
 drivers/usb/dwc3/ep0.c|  2 +-
 drivers/usb/dwc3/gadget.c | 52 ++-
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 59f2e8c31bd1..456aa87e8778 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -197,7 +197,7 @@ int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct 
usb_request *request,
int ret;
 
spin_lock_irqsave(>lock, flags);
-   if (!dep->endpoint.desc) {
+   if (!dep->endpoint.desc || !dwc->pullups_connected) {
dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
dep->name);
ret = -ESHUTDOWN;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3ab6f118c508..df8d89d6bdc9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1516,7 +1516,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
struct dwc3_request *req)
 {
struct dwc3 *dwc = dep->dwc;
 
-   if (!dep->endpoint.desc) {
+   if (!dep->endpoint.desc || !dwc->pullups_connected) {
dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
dep->name);
return -ESHUTDOWN;
@@ -1926,6 +1926,24 @@ static int dwc3_gadget_set_selfpowered(struct usb_gadget 
*g,
return 0;
 }
 
+static void dwc3_stop_active_transfers(struct dwc3 *dwc)
+{
+   u32 epnum;
+
+   for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
+   struct dwc3_ep *dep;
+
+   dep = dwc->eps[epnum];
+   if (!dep)
+   continue;
+
+   if (!(dep->flags & DWC3_EP_ENABLED))
+   continue;
+
+   dwc3_remove_requests(dwc, dep);
+   }
+}
+
 static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
 {
u32 reg;
@@ -1994,9 +2012,39 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int 
is_on)
}
}
 
+   /*
+* Synchronize and disable any further event handling while controller
+* is being enabled/disabled.
+*/
+   disable_irq(dwc->irq_gadget);
spin_lock_irqsave(>lock, flags);
+
+   /* Controller is not halted until pending events are acknowledged */
+   if (!is_on) {
+   u32 reg;
+
+   __dwc3_gadget_ep_disable(dwc->eps[0]);
+   __dwc3_gadget_ep_disable(dwc->eps[1]);
+
+   /*
+* The databook explicitly mentions for a device-initiated
+* disconnect sequence, the SW needs to ensure that it ends any
+* active transfers.
+*/
+   dwc3_stop_active_transfers(dwc);
+
+   reg = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
+   reg &= DWC3_GEVNTCOUNT_MASK;
+   if (reg > 0) {
+   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), reg);
+   dwc->ev_buf->lpos = (dwc->ev_buf->lpos + reg) %
+   dwc->ev_buf->length;
+   }
+   }
+
ret = dwc3_gadget_run_stop(dwc, is_on, false);
spin_unlock_irqrestore(>lock, flags);
+   enable_irq(dwc->irq_gadget);
 
return ret;
 }
@@ -3100,6 +3148,8 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
}
 
dwc3_reset_gadget(dwc);
+   /* Stop any active/pending transfers when receiving bus reset */
+   dwc3_stop_active_transfers(dwc);
 
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
reg &= ~DWC3_DCTL_TSTCTRL_MASK;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project