Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-11-08 Thread John Stultz 
On Mon, Oct 31, 2016 at 4:18 AM, Felipe Balbi
 wrote:
> John Stultz  writes:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>
>> Ends up If we don't disconnect the gadget state on reset, the
>> fifo-map doesn't get cleared properly, which causes WARN_ON
>> messages and also results in the device not properly being
>> setup as a gadget every other time the OTG port is connected.
>>
>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>> reset path so the state is properly cleared.
>>
>> With it, the gadget interface initializes properly on every
>> plug in.
>>
>> I don't know if this is actually the right fix, but it seems
>> to work well. Feedback would be greatly appreciated!
>>
[snip]
>> ---
>>  drivers/usb/dwc2/gadget.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 24fbebc..5505001 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
>> dwc2_hsotg *hsotg,
>>
>>   /* Kill any ep0 requests as controller will be reinitialized */
>>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>> + /* Make sure everything is disconnected */
>> + dwc2_hsotg_disconnect(hsotg);
>
> Dunno, seems like you're actually working around a different
> bug. Looking at USB Reset handler we have:
>
> if (gintsts & (GINTSTS_USBRST | GINTSTS_RESETDET)) {
>
> u32 usb_status = dwc2_readl(hsotg->regs + GOTGCTL);
> u32 connected = hsotg->connected;
>
> dev_dbg(hsotg->dev, "%s: USBRst\n", __func__);
> dev_dbg(hsotg->dev, "GNPTXSTS=%08x\n",
> dwc2_readl(hsotg->regs + GNPTXSTS));
>
> dwc2_writel(GINTSTS_USBRST, hsotg->regs + GINTSTS);
>
> /* Report disconnection if it is not already done. */
> dwc2_hsotg_disconnect(hsotg);
>
> if (usb_status & GOTGCTL_BSESVLD && connected)
> dwc2_hsotg_core_init_disconnected(hsotg, true);
> }
>
> so, dwc2_hsotg_disconnect() is already called from Reset path. What
> you're doing here is that you could, potentially, call
> driver->disconnect() twice.
>
> The real problem could be your implementation for ->pullup() which
> duplicates part of what ->udc_start() does:
>
> static int dwc2_hsotg_pullup(struct usb_gadget *gadget, int is_on)
> {
> struct dwc2_hsotg *hsotg = to_hsotg(gadget);
> unsigned long flags = 0;
>
> dev_dbg(hsotg->dev, "%s: is_on: %d op_state: %d\n", __func__, is_on,
> hsotg->op_state);
>
> /* Don't modify pullup state while in host mode */
> if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
> hsotg->enabled = is_on;
> return 0;
> }
>
> spin_lock_irqsave(>lock, flags);
> if (is_on) {
> hsotg->enabled = 1;
> dwc2_hsotg_core_init_disconnected(hsotg, false);
> dwc2_hsotg_core_connect(hsotg);
> } else {
> dwc2_hsotg_core_disconnect(hsotg);
> dwc2_hsotg_disconnect(hsotg);
> hsotg->enabled = 0;
> }
>
> hsotg->gadget.speed = USB_SPEED_UNKNOWN;
> spin_unlock_irqrestore(>lock, flags);
>
> return 0;
> }
>
> Here's what I think dwc2_hsotg_pullup() and dwc2_hsotg_udc_start()
> should contain:
>
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 9dc6c482b89e..dbe28947d3a9 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3388,6 +3388,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
> dwc2_writel(0, hsotg->regs + DAINTMSK);
>
> /* Be in disconnected state until gadget is registered */
> +   /* REVISIT This should be done in probe() */
> __orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
>
> /* setup fifos */
> @@ -3428,26 +3429,6 @@ static int dwc2_hsotg_udc_start(struct usb_gadget 
> *gadget,
> unsigned long flags;
> int ret;
>
> -   if (!hsotg) {
> -   pr_err("%s: called with no device\n", __func__);
> -   return -ENODEV;
> -   }
> -
> -   if (!driver) {
> -   dev_err(hsotg->dev, "%s: no driver\n", __func__);
> -   return -EINVAL;
> -   }
> -
> -   if (driver->max_speed < USB_SPEED_FULL)
> -   dev_err(hsotg->dev, "%s: bad speed\n", __func__);
> -
> -   if (!driver->setup) {
> -   dev_err(hsotg->dev, "%s: missing entry

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-11-08 Thread John Stultz 
On Mon, Oct 31, 2016 at 4:18 AM, Felipe Balbi
 wrote:
> John Stultz  writes:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>
>> Ends up If we don't disconnect the gadget state on reset, the
>> fifo-map doesn't get cleared properly, which causes WARN_ON
>> messages and also results in the device not properly being
>> setup as a gadget every other time the OTG port is connected.
>>
>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>> reset path so the state is properly cleared.
>>
>> With it, the gadget interface initializes properly on every
>> plug in.
>>
>> I don't know if this is actually the right fix, but it seems
>> to work well. Feedback would be greatly appreciated!
>>
[snip]
>> ---
>>  drivers/usb/dwc2/gadget.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 24fbebc..5505001 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
>> dwc2_hsotg *hsotg,
>>
>>   /* Kill any ep0 requests as controller will be reinitialized */
>>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>> + /* Make sure everything is disconnected */
>> + dwc2_hsotg_disconnect(hsotg);
>
> Dunno, seems like you're actually working around a different
> bug. Looking at USB Reset handler we have:
>
> if (gintsts & (GINTSTS_USBRST | GINTSTS_RESETDET)) {
>
> u32 usb_status = dwc2_readl(hsotg->regs + GOTGCTL);
> u32 connected = hsotg->connected;
>
> dev_dbg(hsotg->dev, "%s: USBRst\n", __func__);
> dev_dbg(hsotg->dev, "GNPTXSTS=%08x\n",
> dwc2_readl(hsotg->regs + GNPTXSTS));
>
> dwc2_writel(GINTSTS_USBRST, hsotg->regs + GINTSTS);
>
> /* Report disconnection if it is not already done. */
> dwc2_hsotg_disconnect(hsotg);
>
> if (usb_status & GOTGCTL_BSESVLD && connected)
> dwc2_hsotg_core_init_disconnected(hsotg, true);
> }
>
> so, dwc2_hsotg_disconnect() is already called from Reset path. What
> you're doing here is that you could, potentially, call
> driver->disconnect() twice.
>
> The real problem could be your implementation for ->pullup() which
> duplicates part of what ->udc_start() does:
>
> static int dwc2_hsotg_pullup(struct usb_gadget *gadget, int is_on)
> {
> struct dwc2_hsotg *hsotg = to_hsotg(gadget);
> unsigned long flags = 0;
>
> dev_dbg(hsotg->dev, "%s: is_on: %d op_state: %d\n", __func__, is_on,
> hsotg->op_state);
>
> /* Don't modify pullup state while in host mode */
> if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
> hsotg->enabled = is_on;
> return 0;
> }
>
> spin_lock_irqsave(>lock, flags);
> if (is_on) {
> hsotg->enabled = 1;
> dwc2_hsotg_core_init_disconnected(hsotg, false);
> dwc2_hsotg_core_connect(hsotg);
> } else {
> dwc2_hsotg_core_disconnect(hsotg);
> dwc2_hsotg_disconnect(hsotg);
> hsotg->enabled = 0;
> }
>
> hsotg->gadget.speed = USB_SPEED_UNKNOWN;
> spin_unlock_irqrestore(>lock, flags);
>
> return 0;
> }
>
> Here's what I think dwc2_hsotg_pullup() and dwc2_hsotg_udc_start()
> should contain:
>
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 9dc6c482b89e..dbe28947d3a9 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3388,6 +3388,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
> dwc2_writel(0, hsotg->regs + DAINTMSK);
>
> /* Be in disconnected state until gadget is registered */
> +   /* REVISIT This should be done in probe() */
> __orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
>
> /* setup fifos */
> @@ -3428,26 +3429,6 @@ static int dwc2_hsotg_udc_start(struct usb_gadget 
> *gadget,
> unsigned long flags;
> int ret;
>
> -   if (!hsotg) {
> -   pr_err("%s: called with no device\n", __func__);
> -   return -ENODEV;
> -   }
> -
> -   if (!driver) {
> -   dev_err(hsotg->dev, "%s: no driver\n", __func__);
> -   return -EINVAL;
> -   }
> -
> -   if (driver->max_speed < USB_SPEED_FULL)
> -   dev_err(hsotg->dev, "%s: bad speed\n", __func__);
> -
> -   if (!driver->setup) {
> -   dev_err(hsotg->dev, "%s: missing entry points\n", __func__);
> -   return -EINVAL;

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-31 Thread John Youn 
On 10/31/2016 4:19 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Stultz  writes:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>
>> Ends up If we don't disconnect the gadget state on reset, the
>> fifo-map doesn't get cleared properly, which causes WARN_ON
>> messages and also results in the device not properly being
>> setup as a gadget every other time the OTG port is connected.
>>
>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>> reset path so the state is properly cleared.
>>
>> With it, the gadget interface initializes properly on every
>> plug in.
>>
>> I don't know if this is actually the right fix, but it seems
>> to work well. Feedback would be greatly appreciated!
>>
>> Cc: Wei Xu 
>> Cc: Guodong Xu 
>> Cc: Chen Yu 
>> Cc: Amit Pundir 
>> Cc: Rob Herring 
>> Cc: Mark Rutland 
>> Cc: John Youn 
>> Cc: Douglas Anderson 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-...@vger.kernel.org
>> Signed-off-by: John Stultz 
>> ---
>>  drivers/usb/dwc2/gadget.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 24fbebc..5505001 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
>> dwc2_hsotg *hsotg,
>>  
>>  /* Kill any ep0 requests as controller will be reinitialized */
>>  kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>> +/* Make sure everything is disconnected */
>> +dwc2_hsotg_disconnect(hsotg);
> 
> Dunno, seems like you're actually working around a different
> bug. Looking at USB Reset handler we have:
> 
>   if (gintsts & (GINTSTS_USBRST | GINTSTS_RESETDET)) {
> 
>   u32 usb_status = dwc2_readl(hsotg->regs + GOTGCTL);
>   u32 connected = hsotg->connected;
> 
>   dev_dbg(hsotg->dev, "%s: USBRst\n", __func__);
>   dev_dbg(hsotg->dev, "GNPTXSTS=%08x\n",
>   dwc2_readl(hsotg->regs + GNPTXSTS));
> 
>   dwc2_writel(GINTSTS_USBRST, hsotg->regs + GINTSTS);
> 
>   /* Report disconnection if it is not already done. */
>   dwc2_hsotg_disconnect(hsotg);
> 
>   if (usb_status & GOTGCTL_BSESVLD && connected)
>   dwc2_hsotg_core_init_disconnected(hsotg, true);
>   }
> 
> so, dwc2_hsotg_disconnect() is already called from Reset path. What
> you're doing here is that you could, potentially, call
> driver->disconnect() twice.
> 
> The real problem could be your implementation for ->pullup() which
> duplicates part of what ->udc_start() does:
> 
> static int dwc2_hsotg_pullup(struct usb_gadget *gadget, int is_on)
> {
>   struct dwc2_hsotg *hsotg = to_hsotg(gadget);
>   unsigned long flags = 0;
> 
>   dev_dbg(hsotg->dev, "%s: is_on: %d op_state: %d\n", __func__, is_on,
>   hsotg->op_state);
> 
>   /* Don't modify pullup state while in host mode */
>   if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
>   hsotg->enabled = is_on;
>   return 0;
>   }
> 
>   spin_lock_irqsave(>lock, flags);
>   if (is_on) {
>   hsotg->enabled = 1;
>   dwc2_hsotg_core_init_disconnected(hsotg, false);
>   dwc2_hsotg_core_connect(hsotg);
>   } else {
>   dwc2_hsotg_core_disconnect(hsotg);
>   dwc2_hsotg_disconnect(hsotg);
>   hsotg->enabled = 0;
>   }
> 
>   hsotg->gadget.speed = USB_SPEED_UNKNOWN;
>   spin_unlock_irqrestore(>lock, flags);
> 
>   return 0;
> }
> 
> Here's what I think dwc2_hsotg_pullup() and dwc2_hsotg_udc_start()
> should contain:
> 
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 9dc6c482b89e..dbe28947d3a9 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3388,6 +3388,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
>   dwc2_writel(0, hsotg->regs + DAINTMSK);
>  
>   /* Be in disconnected state until gadget is registered */
> + /* REVISIT This should be done in probe() */
>   __orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
>  
>   /* setup fifos */
> @@ -3428,26 +3429,6 @@ static int dwc2_hsotg_udc_start(struct usb_gadget 
> *gadget,
>   unsigned long flags;
>   int ret;
>  
> - if (!hsotg) {
> - pr_err("%s: called with no device\n", __func__);
> - return -ENODEV;

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-31 Thread John Youn 
On 10/31/2016 4:19 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Stultz  writes:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>
>> Ends up If we don't disconnect the gadget state on reset, the
>> fifo-map doesn't get cleared properly, which causes WARN_ON
>> messages and also results in the device not properly being
>> setup as a gadget every other time the OTG port is connected.
>>
>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>> reset path so the state is properly cleared.
>>
>> With it, the gadget interface initializes properly on every
>> plug in.
>>
>> I don't know if this is actually the right fix, but it seems
>> to work well. Feedback would be greatly appreciated!
>>
>> Cc: Wei Xu 
>> Cc: Guodong Xu 
>> Cc: Chen Yu 
>> Cc: Amit Pundir 
>> Cc: Rob Herring 
>> Cc: Mark Rutland 
>> Cc: John Youn 
>> Cc: Douglas Anderson 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-...@vger.kernel.org
>> Signed-off-by: John Stultz 
>> ---
>>  drivers/usb/dwc2/gadget.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 24fbebc..5505001 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
>> dwc2_hsotg *hsotg,
>>  
>>  /* Kill any ep0 requests as controller will be reinitialized */
>>  kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>> +/* Make sure everything is disconnected */
>> +dwc2_hsotg_disconnect(hsotg);
> 
> Dunno, seems like you're actually working around a different
> bug. Looking at USB Reset handler we have:
> 
>   if (gintsts & (GINTSTS_USBRST | GINTSTS_RESETDET)) {
> 
>   u32 usb_status = dwc2_readl(hsotg->regs + GOTGCTL);
>   u32 connected = hsotg->connected;
> 
>   dev_dbg(hsotg->dev, "%s: USBRst\n", __func__);
>   dev_dbg(hsotg->dev, "GNPTXSTS=%08x\n",
>   dwc2_readl(hsotg->regs + GNPTXSTS));
> 
>   dwc2_writel(GINTSTS_USBRST, hsotg->regs + GINTSTS);
> 
>   /* Report disconnection if it is not already done. */
>   dwc2_hsotg_disconnect(hsotg);
> 
>   if (usb_status & GOTGCTL_BSESVLD && connected)
>   dwc2_hsotg_core_init_disconnected(hsotg, true);
>   }
> 
> so, dwc2_hsotg_disconnect() is already called from Reset path. What
> you're doing here is that you could, potentially, call
> driver->disconnect() twice.
> 
> The real problem could be your implementation for ->pullup() which
> duplicates part of what ->udc_start() does:
> 
> static int dwc2_hsotg_pullup(struct usb_gadget *gadget, int is_on)
> {
>   struct dwc2_hsotg *hsotg = to_hsotg(gadget);
>   unsigned long flags = 0;
> 
>   dev_dbg(hsotg->dev, "%s: is_on: %d op_state: %d\n", __func__, is_on,
>   hsotg->op_state);
> 
>   /* Don't modify pullup state while in host mode */
>   if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
>   hsotg->enabled = is_on;
>   return 0;
>   }
> 
>   spin_lock_irqsave(>lock, flags);
>   if (is_on) {
>   hsotg->enabled = 1;
>   dwc2_hsotg_core_init_disconnected(hsotg, false);
>   dwc2_hsotg_core_connect(hsotg);
>   } else {
>   dwc2_hsotg_core_disconnect(hsotg);
>   dwc2_hsotg_disconnect(hsotg);
>   hsotg->enabled = 0;
>   }
> 
>   hsotg->gadget.speed = USB_SPEED_UNKNOWN;
>   spin_unlock_irqrestore(>lock, flags);
> 
>   return 0;
> }
> 
> Here's what I think dwc2_hsotg_pullup() and dwc2_hsotg_udc_start()
> should contain:
> 
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 9dc6c482b89e..dbe28947d3a9 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3388,6 +3388,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
>   dwc2_writel(0, hsotg->regs + DAINTMSK);
>  
>   /* Be in disconnected state until gadget is registered */
> + /* REVISIT This should be done in probe() */
>   __orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
>  
>   /* setup fifos */
> @@ -3428,26 +3429,6 @@ static int dwc2_hsotg_udc_start(struct usb_gadget 
> *gadget,
>   unsigned long flags;
>   int ret;
>  
> - if (!hsotg) {
> - pr_err("%s: called with no device\n", __func__);
> - return -ENODEV;
> - }
> -
> - if (!driver) {
> - dev_err(hsotg->dev, "%s: no driver\n", __func__);
> - return -EINVAL;
> - }
> -
> - if (driver->max_speed < USB_SPEED_FULL)
> - dev_err(hsotg->dev, "%s: bad speed\n",

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-31 Thread Felipe Balbi 

Hi,

John Stultz  writes:
> I had seen some odd behavior with HiKey's usb-gadget interface
> that I finally seemed to have chased down. Basically every other
> time I pluged in the OTG port, the gadget interface would
> properly initialize. The other times, I'd get a big WARN_ON
> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>
> Ends up If we don't disconnect the gadget state on reset, the
> fifo-map doesn't get cleared properly, which causes WARN_ON
> messages and also results in the device not properly being
> setup as a gadget every other time the OTG port is connected.
>
> So this patch adds a call to dwc2_hsotg_disconnect() in the
> reset path so the state is properly cleared.
>
> With it, the gadget interface initializes properly on every
> plug in.
>
> I don't know if this is actually the right fix, but it seems
> to work well. Feedback would be greatly appreciated!
>
> Cc: Wei Xu 
> Cc: Guodong Xu 
> Cc: Chen Yu 
> Cc: Amit Pundir 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: John Youn 
> Cc: Douglas Anderson 
> Cc: Greg Kroah-Hartman 
> Cc: linux-...@vger.kernel.org
> Signed-off-by: John Stultz 
> ---
>  drivers/usb/dwc2/gadget.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 24fbebc..5505001 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
> dwc2_hsotg *hsotg,
>  
>   /* Kill any ep0 requests as controller will be reinitialized */
>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
> + /* Make sure everything is disconnected */
> + dwc2_hsotg_disconnect(hsotg);

Dunno, seems like you're actually working around a different
bug. Looking at USB Reset handler we have:

if (gintsts & (GINTSTS_USBRST | GINTSTS_RESETDET)) {

u32 usb_status = dwc2_readl(hsotg->regs + GOTGCTL);
u32 connected = hsotg->connected;

dev_dbg(hsotg->dev, "%s: USBRst\n", __func__);
dev_dbg(hsotg->dev, "GNPTXSTS=%08x\n",
dwc2_readl(hsotg->regs + GNPTXSTS));

dwc2_writel(GINTSTS_USBRST, hsotg->regs + GINTSTS);

/* Report disconnection if it is not already done. */
dwc2_hsotg_disconnect(hsotg);

if (usb_status & GOTGCTL_BSESVLD && connected)
dwc2_hsotg_core_init_disconnected(hsotg, true);
}

so, dwc2_hsotg_disconnect() is already called from Reset path. What
you're doing here is that you could, potentially, call
driver->disconnect() twice.

The real problem could be your implementation for ->pullup() which
duplicates part of what ->udc_start() does:

static int dwc2_hsotg_pullup(struct usb_gadget *gadget, int is_on)
{
struct dwc2_hsotg *hsotg = to_hsotg(gadget);
unsigned long flags = 0;

dev_dbg(hsotg->dev, "%s: is_on: %d op_state: %d\n", __func__, is_on,
hsotg->op_state);

/* Don't modify pullup state while in host mode */
if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
hsotg->enabled = is_on;
return 0;
}

spin_lock_irqsave(>lock, flags);
if (is_on) {
hsotg->enabled = 1;
dwc2_hsotg_core_init_disconnected(hsotg, false);
dwc2_hsotg_core_connect(hsotg);
} else {
dwc2_hsotg_core_disconnect(hsotg);
dwc2_hsotg_disconnect(hsotg);
hsotg->enabled = 0;
}

hsotg->gadget.speed = USB_SPEED_UNKNOWN;
spin_unlock_irqrestore(>lock, flags);

return 0;
}

Here's what I think dwc2_hsotg_pullup() and dwc2_hsotg_udc_start()
should contain:


diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 9dc6c482b89e..dbe28947d3a9 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3388,6 +3388,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
dwc2_writel(0, hsotg->regs + DAINTMSK);
 
/* Be in disconnected state until gadget is registered */
+   /* REVISIT This should be done in probe() */
__orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
 
/* setup fifos */
@@ -3428,26 +3429,6 @@ static int dwc2_hsotg_udc_start(struct usb_gadget 
*gadget,
unsigned long flags;
int ret;
 
-   if (!hsotg) {
-   pr_err("%s: called with no device\n", __func__);
-   return -ENODEV;
-   }
-
-   if (!driver) {
-   dev_err(hsotg->dev, "%s: no driver\n", __func__);
-   return -EINVAL;
-   }
-
-   if

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-31 Thread Felipe Balbi 

Hi,

John Stultz  writes:
> I had seen some odd behavior with HiKey's usb-gadget interface
> that I finally seemed to have chased down. Basically every other
> time I pluged in the OTG port, the gadget interface would
> properly initialize. The other times, I'd get a big WARN_ON
> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>
> Ends up If we don't disconnect the gadget state on reset, the
> fifo-map doesn't get cleared properly, which causes WARN_ON
> messages and also results in the device not properly being
> setup as a gadget every other time the OTG port is connected.
>
> So this patch adds a call to dwc2_hsotg_disconnect() in the
> reset path so the state is properly cleared.
>
> With it, the gadget interface initializes properly on every
> plug in.
>
> I don't know if this is actually the right fix, but it seems
> to work well. Feedback would be greatly appreciated!
>
> Cc: Wei Xu 
> Cc: Guodong Xu 
> Cc: Chen Yu 
> Cc: Amit Pundir 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: John Youn 
> Cc: Douglas Anderson 
> Cc: Greg Kroah-Hartman 
> Cc: linux-...@vger.kernel.org
> Signed-off-by: John Stultz 
> ---
>  drivers/usb/dwc2/gadget.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 24fbebc..5505001 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
> dwc2_hsotg *hsotg,
>  
>   /* Kill any ep0 requests as controller will be reinitialized */
>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
> + /* Make sure everything is disconnected */
> + dwc2_hsotg_disconnect(hsotg);

Dunno, seems like you're actually working around a different
bug. Looking at USB Reset handler we have:

if (gintsts & (GINTSTS_USBRST | GINTSTS_RESETDET)) {

u32 usb_status = dwc2_readl(hsotg->regs + GOTGCTL);
u32 connected = hsotg->connected;

dev_dbg(hsotg->dev, "%s: USBRst\n", __func__);
dev_dbg(hsotg->dev, "GNPTXSTS=%08x\n",
dwc2_readl(hsotg->regs + GNPTXSTS));

dwc2_writel(GINTSTS_USBRST, hsotg->regs + GINTSTS);

/* Report disconnection if it is not already done. */
dwc2_hsotg_disconnect(hsotg);

if (usb_status & GOTGCTL_BSESVLD && connected)
dwc2_hsotg_core_init_disconnected(hsotg, true);
}

so, dwc2_hsotg_disconnect() is already called from Reset path. What
you're doing here is that you could, potentially, call
driver->disconnect() twice.

The real problem could be your implementation for ->pullup() which
duplicates part of what ->udc_start() does:

static int dwc2_hsotg_pullup(struct usb_gadget *gadget, int is_on)
{
struct dwc2_hsotg *hsotg = to_hsotg(gadget);
unsigned long flags = 0;

dev_dbg(hsotg->dev, "%s: is_on: %d op_state: %d\n", __func__, is_on,
hsotg->op_state);

/* Don't modify pullup state while in host mode */
if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
hsotg->enabled = is_on;
return 0;
}

spin_lock_irqsave(>lock, flags);
if (is_on) {
hsotg->enabled = 1;
dwc2_hsotg_core_init_disconnected(hsotg, false);
dwc2_hsotg_core_connect(hsotg);
} else {
dwc2_hsotg_core_disconnect(hsotg);
dwc2_hsotg_disconnect(hsotg);
hsotg->enabled = 0;
}

hsotg->gadget.speed = USB_SPEED_UNKNOWN;
spin_unlock_irqrestore(>lock, flags);

return 0;
}

Here's what I think dwc2_hsotg_pullup() and dwc2_hsotg_udc_start()
should contain:


diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 9dc6c482b89e..dbe28947d3a9 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3388,6 +3388,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
dwc2_writel(0, hsotg->regs + DAINTMSK);
 
/* Be in disconnected state until gadget is registered */
+   /* REVISIT This should be done in probe() */
__orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
 
/* setup fifos */
@@ -3428,26 +3429,6 @@ static int dwc2_hsotg_udc_start(struct usb_gadget 
*gadget,
unsigned long flags;
int ret;
 
-   if (!hsotg) {
-   pr_err("%s: called with no device\n", __func__);
-   return -ENODEV;
-   }
-
-   if (!driver) {
-   dev_err(hsotg->dev, "%s: no driver\n", __func__);
-   return -EINVAL;
-   }
-
-   if (driver->max_speed < USB_SPEED_FULL)
-   dev_err(hsotg->dev, "%s: bad speed\n", __func__);
-
-   if (!driver->setup) {
-   dev_err(hsotg->dev, "%s: missing entry points\n", __func__);
-   return -EINVAL;
-   }
-
-

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-25 Thread John Youn 
On 10/25/2016 2:56 PM, John Stultz wrote:
> On Tue, Oct 25, 2016 at 2:29 PM, John Youn  wrote:
>> On 10/19/2016 11:00 PM, John Stultz wrote:
>>> I had seen some odd behavior with HiKey's usb-gadget interface
>>> that I finally seemed to have chased down. Basically every other
>>> time I pluged in the OTG port, the gadget interface would
>>> properly initialize. The other times, I'd get a big WARN_ON
>>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>>
>>> Ends up If we don't disconnect the gadget state on reset, the
>>> fifo-map doesn't get cleared properly, which causes WARN_ON
>>> messages and also results in the device not properly being
>>> setup as a gadget every other time the OTG port is connected.
>>>
>>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>>> reset path so the state is properly cleared.
>>>
>>> With it, the gadget interface initializes properly on every
>>> plug in.
>>>
>>> I don't know if this is actually the right fix, but it seems
>>> to work well. Feedback would be greatly appreciated!
>>>
>>> Cc: Wei Xu 
>>> Cc: Guodong Xu 
>>> Cc: Chen Yu 
>>> Cc: Amit Pundir 
>>> Cc: Rob Herring 
>>> Cc: Mark Rutland 
>>> Cc: John Youn 
>>> Cc: Douglas Anderson 
>>> Cc: Greg Kroah-Hartman 
>>> Cc: linux-...@vger.kernel.org
>>> Signed-off-by: John Stultz 
>>> ---
>>>  drivers/usb/dwc2/gadget.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>>> index 24fbebc..5505001 100644
>>> --- a/drivers/usb/dwc2/gadget.c
>>> +++ b/drivers/usb/dwc2/gadget.c
>>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
>>> dwc2_hsotg *hsotg,
>>>
>>>   /* Kill any ep0 requests as controller will be reinitialized */
>>>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>>> + /* Make sure everything is disconnected */
>>> + dwc2_hsotg_disconnect(hsotg);
>>>
>>>   if (!is_usb_reset)
>>>   if (dwc2_core_reset(hsotg))
>>>
>>
>> Seems fine with our testing.
>>
>> Acked-by: John Youn 
> 
> Awesome! Thanks so much for the review and testing!
> 
> I'm curious, did you happen to have any thoughts or objections on the
> "dwc2: Force port resume on switching to device mode" patch
> (https://patchwork.kernel.org/patch/9375965/ ) as well?

Sorry, must have missed that one. We'll take a look.

Regards,
John

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-25 Thread John Youn 
On 10/25/2016 2:56 PM, John Stultz wrote:
> On Tue, Oct 25, 2016 at 2:29 PM, John Youn  wrote:
>> On 10/19/2016 11:00 PM, John Stultz wrote:
>>> I had seen some odd behavior with HiKey's usb-gadget interface
>>> that I finally seemed to have chased down. Basically every other
>>> time I pluged in the OTG port, the gadget interface would
>>> properly initialize. The other times, I'd get a big WARN_ON
>>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>>
>>> Ends up If we don't disconnect the gadget state on reset, the
>>> fifo-map doesn't get cleared properly, which causes WARN_ON
>>> messages and also results in the device not properly being
>>> setup as a gadget every other time the OTG port is connected.
>>>
>>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>>> reset path so the state is properly cleared.
>>>
>>> With it, the gadget interface initializes properly on every
>>> plug in.
>>>
>>> I don't know if this is actually the right fix, but it seems
>>> to work well. Feedback would be greatly appreciated!
>>>
>>> Cc: Wei Xu 
>>> Cc: Guodong Xu 
>>> Cc: Chen Yu 
>>> Cc: Amit Pundir 
>>> Cc: Rob Herring 
>>> Cc: Mark Rutland 
>>> Cc: John Youn 
>>> Cc: Douglas Anderson 
>>> Cc: Greg Kroah-Hartman 
>>> Cc: linux-...@vger.kernel.org
>>> Signed-off-by: John Stultz 
>>> ---
>>>  drivers/usb/dwc2/gadget.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>>> index 24fbebc..5505001 100644
>>> --- a/drivers/usb/dwc2/gadget.c
>>> +++ b/drivers/usb/dwc2/gadget.c
>>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
>>> dwc2_hsotg *hsotg,
>>>
>>>   /* Kill any ep0 requests as controller will be reinitialized */
>>>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>>> + /* Make sure everything is disconnected */
>>> + dwc2_hsotg_disconnect(hsotg);
>>>
>>>   if (!is_usb_reset)
>>>   if (dwc2_core_reset(hsotg))
>>>
>>
>> Seems fine with our testing.
>>
>> Acked-by: John Youn 
> 
> Awesome! Thanks so much for the review and testing!
> 
> I'm curious, did you happen to have any thoughts or objections on the
> "dwc2: Force port resume on switching to device mode" patch
> (https://patchwork.kernel.org/patch/9375965/ ) as well?

Sorry, must have missed that one. We'll take a look.

Regards,
John

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-25 Thread John Stultz 
On Tue, Oct 25, 2016 at 2:29 PM, John Youn  wrote:
> On 10/19/2016 11:00 PM, John Stultz wrote:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>
>> Ends up If we don't disconnect the gadget state on reset, the
>> fifo-map doesn't get cleared properly, which causes WARN_ON
>> messages and also results in the device not properly being
>> setup as a gadget every other time the OTG port is connected.
>>
>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>> reset path so the state is properly cleared.
>>
>> With it, the gadget interface initializes properly on every
>> plug in.
>>
>> I don't know if this is actually the right fix, but it seems
>> to work well. Feedback would be greatly appreciated!
>>
>> Cc: Wei Xu 
>> Cc: Guodong Xu 
>> Cc: Chen Yu 
>> Cc: Amit Pundir 
>> Cc: Rob Herring 
>> Cc: Mark Rutland 
>> Cc: John Youn 
>> Cc: Douglas Anderson 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-...@vger.kernel.org
>> Signed-off-by: John Stultz 
>> ---
>>  drivers/usb/dwc2/gadget.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 24fbebc..5505001 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
>> dwc2_hsotg *hsotg,
>>
>>   /* Kill any ep0 requests as controller will be reinitialized */
>>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>> + /* Make sure everything is disconnected */
>> + dwc2_hsotg_disconnect(hsotg);
>>
>>   if (!is_usb_reset)
>>   if (dwc2_core_reset(hsotg))
>>
>
> Seems fine with our testing.
>
> Acked-by: John Youn 

Awesome! Thanks so much for the review and testing!

I'm curious, did you happen to have any thoughts or objections on the
"dwc2: Force port resume on switching to device mode" patch
(https://patchwork.kernel.org/patch/9375965/ ) as well?

thanks
-john

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-25 Thread John Stultz 
On Tue, Oct 25, 2016 at 2:29 PM, John Youn  wrote:
> On 10/19/2016 11:00 PM, John Stultz wrote:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>
>> Ends up If we don't disconnect the gadget state on reset, the
>> fifo-map doesn't get cleared properly, which causes WARN_ON
>> messages and also results in the device not properly being
>> setup as a gadget every other time the OTG port is connected.
>>
>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>> reset path so the state is properly cleared.
>>
>> With it, the gadget interface initializes properly on every
>> plug in.
>>
>> I don't know if this is actually the right fix, but it seems
>> to work well. Feedback would be greatly appreciated!
>>
>> Cc: Wei Xu 
>> Cc: Guodong Xu 
>> Cc: Chen Yu 
>> Cc: Amit Pundir 
>> Cc: Rob Herring 
>> Cc: Mark Rutland 
>> Cc: John Youn 
>> Cc: Douglas Anderson 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-...@vger.kernel.org
>> Signed-off-by: John Stultz 
>> ---
>>  drivers/usb/dwc2/gadget.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 24fbebc..5505001 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
>> dwc2_hsotg *hsotg,
>>
>>   /* Kill any ep0 requests as controller will be reinitialized */
>>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>> + /* Make sure everything is disconnected */
>> + dwc2_hsotg_disconnect(hsotg);
>>
>>   if (!is_usb_reset)
>>   if (dwc2_core_reset(hsotg))
>>
>
> Seems fine with our testing.
>
> Acked-by: John Youn 

Awesome! Thanks so much for the review and testing!

I'm curious, did you happen to have any thoughts or objections on the
"dwc2: Force port resume on switching to device mode" patch
(https://patchwork.kernel.org/patch/9375965/ ) as well?

thanks
-john

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-25 Thread John Youn 
On 10/19/2016 11:00 PM, John Stultz wrote:
> I had seen some odd behavior with HiKey's usb-gadget interface
> that I finally seemed to have chased down. Basically every other
> time I pluged in the OTG port, the gadget interface would
> properly initialize. The other times, I'd get a big WARN_ON
> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
> 
> Ends up If we don't disconnect the gadget state on reset, the
> fifo-map doesn't get cleared properly, which causes WARN_ON
> messages and also results in the device not properly being
> setup as a gadget every other time the OTG port is connected.
> 
> So this patch adds a call to dwc2_hsotg_disconnect() in the
> reset path so the state is properly cleared.
> 
> With it, the gadget interface initializes properly on every
> plug in.
> 
> I don't know if this is actually the right fix, but it seems
> to work well. Feedback would be greatly appreciated!
> 
> Cc: Wei Xu 
> Cc: Guodong Xu 
> Cc: Chen Yu 
> Cc: Amit Pundir 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: John Youn 
> Cc: Douglas Anderson 
> Cc: Greg Kroah-Hartman 
> Cc: linux-...@vger.kernel.org
> Signed-off-by: John Stultz 
> ---
>  drivers/usb/dwc2/gadget.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 24fbebc..5505001 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
> dwc2_hsotg *hsotg,
>  
>   /* Kill any ep0 requests as controller will be reinitialized */
>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
> + /* Make sure everything is disconnected */
> + dwc2_hsotg_disconnect(hsotg);
>  
>   if (!is_usb_reset)
>   if (dwc2_core_reset(hsotg))
> 

Seems fine with our testing.

Acked-by: John Youn 

Regards,
John

Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset

2016-10-25 Thread John Youn 
On 10/19/2016 11:00 PM, John Stultz wrote:
> I had seen some odd behavior with HiKey's usb-gadget interface
> that I finally seemed to have chased down. Basically every other
> time I pluged in the OTG port, the gadget interface would
> properly initialize. The other times, I'd get a big WARN_ON
> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
> 
> Ends up If we don't disconnect the gadget state on reset, the
> fifo-map doesn't get cleared properly, which causes WARN_ON
> messages and also results in the device not properly being
> setup as a gadget every other time the OTG port is connected.
> 
> So this patch adds a call to dwc2_hsotg_disconnect() in the
> reset path so the state is properly cleared.
> 
> With it, the gadget interface initializes properly on every
> plug in.
> 
> I don't know if this is actually the right fix, but it seems
> to work well. Feedback would be greatly appreciated!
> 
> Cc: Wei Xu 
> Cc: Guodong Xu 
> Cc: Chen Yu 
> Cc: Amit Pundir 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: John Youn 
> Cc: Douglas Anderson 
> Cc: Greg Kroah-Hartman 
> Cc: linux-...@vger.kernel.org
> Signed-off-by: John Stultz 
> ---
>  drivers/usb/dwc2/gadget.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 24fbebc..5505001 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct 
> dwc2_hsotg *hsotg,
>  
>   /* Kill any ep0 requests as controller will be reinitialized */
>   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
> + /* Make sure everything is disconnected */
> + dwc2_hsotg_disconnect(hsotg);
>  
>   if (!is_usb_reset)
>   if (dwc2_core_reset(hsotg))
> 

Seems fine with our testing.

Acked-by: John Youn 

Regards,
John