All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] dwc2 fixes for edge cases on hikey
@ 2017-10-23 21:32 John Stultz
  2017-10-23 21:32 ` [PATCH 1/3 v2] usb: dwc2: Improve gadget state disconnection handling John Stultz
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: John Stultz @ 2017-10-23 21:32 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu,
	John Youn, Minas Harutyunyan, Douglas Anderson, Chen Yu,
	Felipe Balbi, Greg Kroah-Hartman, linux-usb

So here is v2 of a few dwc2 fixes that I've been using with HiKey.
I'm not totally sure these are all ideal, but they avoid edge case
issues that we have been running into with switching between
gadget mode and host mode.

I'd guess the first two are potentially -stable material, and
the last might be worth sending to -stable too, as its a relatively
simple fix, but to my understanding the UDC state tracking has
always been broken so its not really a regression. But still.

thanks
-john

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: YongQin Liu <yongqin.liu@linaro.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org

John Stultz (3):
  usb: dwc2: Improve gadget state disconnection handling
  usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode
  usb: dwc2: Fix UDC state tracking

 drivers/usb/dwc2/gadget.c | 7 +++++++
 drivers/usb/dwc2/hcd.c    | 7 +++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3 v2] usb: dwc2: Improve gadget state disconnection handling
  2017-10-23 21:32 [PATCH v2 0/3] dwc2 fixes for edge cases on hikey John Stultz
@ 2017-10-23 21:32 ` John Stultz
  2017-10-24  8:59   ` Minas Harutyunyan
  2017-10-23 21:32 ` [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode John Stultz
  2017-10-23 21:32 ` [PATCH 3/3 v2] usb: dwc2: Fix UDC state tracking John Stultz
  2 siblings, 1 reply; 7+ messages in thread
From: John Stultz @ 2017-10-23 21:32 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu,
	John Youn, Minas Harutyunyan, Douglas Anderson, Chen Yu,
	Felipe Balbi, Greg Kroah-Hartman, linux-usb

In the earlier commit dad3f793f20f ("usb: dwc2: Make sure we
disconnect the gadget state"), I was trying to fix up the
fact that we somehow weren't disconnecting the gadget state,
so that when the OTG port was plugged in the second time we
would get warnings about the state tracking being wrong.

(This seems to be due to a quirk of the HiKey board where
we do not ever get any otg interrupts, particularly the session
end detected signal. Instead we only see status change
interrupt.)

The fix there was somewhat simple, as it just made sure to
call dwc2_hsotg_disconnect() before we connected things up
in OTG mode, ensuring the state handling didn't throw errors.

But in looking at a different issue I was seeing with UDC
state handling, I realized that it would be much better
to call dwc2_hsotg_disconnect when we get the state change
signal moving to host mode.

Thus, this patch removes the earlier disconnect call I added
and moves it (and the needed locking) to the host mode
transition.

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: YongQin Liu <yongqin.liu@linaro.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2: Remove the extra dwc2_hsotg_core_init_disconnected() call
    I had added, as suggested by Minas.
---
 drivers/usb/dwc2/hcd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index c263114..9bd60ec 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3277,7 +3277,6 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
 		dwc2_core_init(hsotg, false);
 		dwc2_enable_global_interrupts(hsotg);
 		spin_lock_irqsave(&hsotg->lock, flags);
-		dwc2_hsotg_disconnect(hsotg);
 		dwc2_hsotg_core_init_disconnected(hsotg, false);
 		spin_unlock_irqrestore(&hsotg->lock, flags);
 		dwc2_hsotg_core_connect(hsotg);
@@ -3296,8 +3295,12 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
 		if (count > 250)
 			dev_err(hsotg->dev,
 				"Connection id status change timed out\n");
-		hsotg->op_state = OTG_STATE_A_HOST;
 
+		spin_lock_irqsave(&hsotg->lock, flags);
+		dwc2_hsotg_disconnect(hsotg);
+		spin_unlock_irqrestore(&hsotg->lock, flags);
+
+		hsotg->op_state = OTG_STATE_A_HOST;
 		/* Initialize the Core for Host mode */
 		dwc2_core_init(hsotg, false);
 		dwc2_enable_global_interrupts(hsotg);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode
  2017-10-23 21:32 [PATCH v2 0/3] dwc2 fixes for edge cases on hikey John Stultz
  2017-10-23 21:32 ` [PATCH 1/3 v2] usb: dwc2: Improve gadget state disconnection handling John Stultz
@ 2017-10-23 21:32 ` John Stultz
  2017-10-24  8:33   ` Minas Harutyunyan
  2017-10-23 21:32 ` [PATCH 3/3 v2] usb: dwc2: Fix UDC state tracking John Stultz
  2 siblings, 1 reply; 7+ messages in thread
From: John Stultz @ 2017-10-23 21:32 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu,
	John Youn, Minas Harutyunyan, Douglas Anderson, Chen Yu,
	Felipe Balbi, Greg Kroah-Hartman, linux-usb

We've found that while in host mode, using Android, if one runs
the command:
  stop adbd

The existing usb devices being utilized in host mode are disconnected.
This is most visible with usb networking devices.

This seems to be due to adbd closing the file:
  /dev/usb-ffs/adb/ep0
Which calls ffs_ep0_release() and the following backtrace:

[<ffffff800875a430>] dwc2_hsotg_ep_disable+0x148/0x150
[<ffffff800875a498>] dwc2_hsotg_udc_stop+0x60/0x110
[<ffffff8008787950>] usb_gadget_remove_driver+0x58/0x78
[<ffffff80087879e4>] usb_gadget_unregister_driver+0x74/0xe8
[<ffffff80087850c0>] unregister_gadget+0x28/0x58
[<ffffff800878511c>] unregister_gadget_item+0x2c/0x40
[<ffffff8008790ea8>] ffs_data_clear+0xe8/0xf8
[<ffffff8008790ed8>] ffs_data_reset+0x20/0x58
[<ffffff8008793218>] ffs_data_closed+0x98/0xe8
[<ffffff80087932d8>] ffs_ep0_release+0x20/0x30

Then when dwc2_hsotg_ep_disable() is called, we call
kill_all_requests() which causes a bunch of the following
messages:

dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
init: Service 'adbd' (pid 1915) killed by signal 9
init: Sending signal 9 to service 'adbd' (pid 1915) process group...
init: Successfully killed process cgroup uid 0 pid 1915 in 0ms
init: processing action (init.svc.adbd=stopped) from (/init.usb.configfs.rc:15)
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 8 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 12 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 15 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_update_urb_state_abn(): trimming xfer length

And the usb devices connected are basically hung at this point.

It seems like if we're in host mode, we probably shouldn't run
the dwc2_hostg_ep_disable logic, so this patch returns an error
in that case.

With this patch (along with the previous patch in this set), we avoid
the mismatched interrupts and connected usb devices continue to function.

I'm not sure if some other solution would be better here, but this seems
to work, so I wanted to send it out for input on what the right approach
should be.

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: YongQin Liu <yongqin.liu@linaro.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Tested by: Minas Harutyunyan <hminas@synopsys.com>
Reported-by: YongQin Liu <yongqin.liu@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/usb/dwc2/gadget.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0d8e09c..7fd0e38 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4004,6 +4004,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
 		return -EINVAL;
 	}
 
+	if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
+		dev_err(hsotg->dev, "%s: called in host mode?\n", __func__);
+		return -EINVAL;
+	}
+
 	epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
 
 	spin_lock_irqsave(&hsotg->lock, flags);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3 v2] usb: dwc2: Fix UDC state tracking
  2017-10-23 21:32 [PATCH v2 0/3] dwc2 fixes for edge cases on hikey John Stultz
  2017-10-23 21:32 ` [PATCH 1/3 v2] usb: dwc2: Improve gadget state disconnection handling John Stultz
  2017-10-23 21:32 ` [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode John Stultz
@ 2017-10-23 21:32 ` John Stultz
  2017-10-24  8:33   ` Minas Harutyunyan
  2 siblings, 1 reply; 7+ messages in thread
From: John Stultz @ 2017-10-23 21:32 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu,
	John Youn, Minas Harutyunyan, Douglas Anderson, Chen Yu,
	Felipe Balbi, Greg Kroah-Hartman, linux-usb

It has been noticed that the dwc2 udc state reporting doesn't
seem to work (at least on HiKey boards). Where after the initial
setup, the sysfs /sys/class/udc/f72c0000.usb/state file would
report "configured" no matter the state of the OTG port.

This patch adds a call so that we report to the UDC layer when
the gadget device is disconnected.

This patch does depend on the previous patch ("usb: dwc2:
Improve gadget state disconnection handling") in this patch set
in order to properly work.

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: YongQin Liu <yongqin.liu@linaro.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Tested by: Minas Harutyunyan <hminas@synopsys.com>
Reported-by: Amit Pundir <amit.pundir@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 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 7fd0e38..603c216 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3202,6 +3202,8 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg)
 
 	call_gadget(hsotg, disconnect);
 	hsotg->lx_state = DWC2_L3;
+
+	usb_gadget_set_state(&hsotg->gadget, USB_STATE_NOTATTACHED);
 }
 
 /**
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3 v2] usb: dwc2: Fix UDC state tracking
  2017-10-23 21:32 ` [PATCH 3/3 v2] usb: dwc2: Fix UDC state tracking John Stultz
@ 2017-10-24  8:33   ` Minas Harutyunyan
  0 siblings, 0 replies; 7+ messages in thread
From: Minas Harutyunyan @ 2017-10-24  8:33 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, John Youn,
	Minas Harutyunyan, Douglas Anderson, Chen Yu, Felipe Balbi,
	Greg Kroah-Hartman, linux-usb

On 10/24/2017 1:33 AM, John Stultz wrote:
> It has been noticed that the dwc2 udc state reporting doesn't
> seem to work (at least on HiKey boards). Where after the initial
> setup, the sysfs /sys/class/udc/f72c0000.usb/state file would
> report "configured" no matter the state of the OTG port.
> 
> This patch adds a call so that we report to the UDC layer when
> the gadget device is disconnected.
> 
> This patch does depend on the previous patch ("usb: dwc2:
> Improve gadget state disconnection handling") in this patch set
> in order to properly work.
> 
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Amit Pundir <amit.pundir@linaro.org>
> Cc: YongQin Liu <yongqin.liu@linaro.org>
> Cc: John Youn <johnyoun@synopsys.com>
> Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Chen Yu <chenyu56@huawei.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Tested by: Minas Harutyunyan <hminas@synopsys.com>
> Reported-by: Amit Pundir <amit.pundir@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   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 7fd0e38..603c216 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3202,6 +3202,8 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg)
>   
>   	call_gadget(hsotg, disconnect);
>   	hsotg->lx_state = DWC2_L3;
> +
> +	usb_gadget_set_state(&hsotg->gadget, USB_STATE_NOTATTACHED);
>   }
>   
>   /**
> 
Acked-by: Minas Harutyunyan <hminas@synopsys.com>
Tested-by: Minas Harutyunyan <hminas@synopsys.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode
  2017-10-23 21:32 ` [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode John Stultz
@ 2017-10-24  8:33   ` Minas Harutyunyan
  0 siblings, 0 replies; 7+ messages in thread
From: Minas Harutyunyan @ 2017-10-24  8:33 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, John Youn,
	Minas Harutyunyan, Douglas Anderson, Chen Yu, Felipe Balbi,
	Greg Kroah-Hartman, linux-usb

On 10/24/2017 1:33 AM, John Stultz wrote:
> We've found that while in host mode, using Android, if one runs
> the command:
>    stop adbd
> 
> The existing usb devices being utilized in host mode are disconnected.
> This is most visible with usb networking devices.
> 
> This seems to be due to adbd closing the file:
>    /dev/usb-ffs/adb/ep0
> Which calls ffs_ep0_release() and the following backtrace:
> 
> [<ffffff800875a430>] dwc2_hsotg_ep_disable+0x148/0x150
> [<ffffff800875a498>] dwc2_hsotg_udc_stop+0x60/0x110
> [<ffffff8008787950>] usb_gadget_remove_driver+0x58/0x78
> [<ffffff80087879e4>] usb_gadget_unregister_driver+0x74/0xe8
> [<ffffff80087850c0>] unregister_gadget+0x28/0x58
> [<ffffff800878511c>] unregister_gadget_item+0x2c/0x40
> [<ffffff8008790ea8>] ffs_data_clear+0xe8/0xf8
> [<ffffff8008790ed8>] ffs_data_reset+0x20/0x58
> [<ffffff8008793218>] ffs_data_closed+0x98/0xe8
> [<ffffff80087932d8>] ffs_ep0_release+0x20/0x30
> 
> Then when dwc2_hsotg_ep_disable() is called, we call
> kill_all_requests() which causes a bunch of the following
> messages:
> 
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> init: Service 'adbd' (pid 1915) killed by signal 9
> init: Sending signal 9 to service 'adbd' (pid 1915) process group...
> init: Successfully killed process cgroup uid 0 pid 1915 in 0ms
> init: processing action (init.svc.adbd=stopped) from (/init.usb.configfs.rc:15)
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 8 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 12 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 15 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_update_urb_state_abn(): trimming xfer length
> 
> And the usb devices connected are basically hung at this point.
> 
> It seems like if we're in host mode, we probably shouldn't run
> the dwc2_hostg_ep_disable logic, so this patch returns an error
> in that case.
> 
> With this patch (along with the previous patch in this set), we avoid
> the mismatched interrupts and connected usb devices continue to function.
> 
> I'm not sure if some other solution would be better here, but this seems
> to work, so I wanted to send it out for input on what the right approach
> should be.
> 
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Amit Pundir <amit.pundir@linaro.org>
> Cc: YongQin Liu <yongqin.liu@linaro.org>
> Cc: John Youn <johnyoun@synopsys.com>
> Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Chen Yu <chenyu56@huawei.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Tested by: Minas Harutyunyan <hminas@synopsys.com>
> Reported-by: YongQin Liu <yongqin.liu@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   drivers/usb/dwc2/gadget.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0d8e09c..7fd0e38 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4004,6 +4004,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
>   		return -EINVAL;
>   	}
>   
> +	if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
> +		dev_err(hsotg->dev, "%s: called in host mode?\n", __func__);
> +		return -EINVAL;
> +	}
> +
>   	epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
>   
>   	spin_lock_irqsave(&hsotg->lock, flags);
> 
Acked-by: Minas Harutyunyan <hminas@synopsys.com>
Tested-by: Minas Harutyunyan <hminas@synopsys.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3 v2] usb: dwc2: Improve gadget state disconnection handling
  2017-10-23 21:32 ` [PATCH 1/3 v2] usb: dwc2: Improve gadget state disconnection handling John Stultz
@ 2017-10-24  8:59   ` Minas Harutyunyan
  0 siblings, 0 replies; 7+ messages in thread
From: Minas Harutyunyan @ 2017-10-24  8:59 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, John Youn,
	Minas Harutyunyan, Douglas Anderson, Chen Yu, Felipe Balbi,
	Greg Kroah-Hartman, linux-usb

On 10/24/2017 1:33 AM, John Stultz wrote:
> In the earlier commit dad3f793f20f ("usb: dwc2: Make sure we
> disconnect the gadget state"), I was trying to fix up the
> fact that we somehow weren't disconnecting the gadget state,
> so that when the OTG port was plugged in the second time we
> would get warnings about the state tracking being wrong.
> 
> (This seems to be due to a quirk of the HiKey board where
> we do not ever get any otg interrupts, particularly the session
> end detected signal. Instead we only see status change
> interrupt.)
> 
> The fix there was somewhat simple, as it just made sure to
> call dwc2_hsotg_disconnect() before we connected things up
> in OTG mode, ensuring the state handling didn't throw errors.
> 
> But in looking at a different issue I was seeing with UDC
> state handling, I realized that it would be much better
> to call dwc2_hsotg_disconnect when we get the state change
> signal moving to host mode.
> 
> Thus, this patch removes the earlier disconnect call I added
> and moves it (and the needed locking) to the host mode
> transition.
> 
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Amit Pundir <amit.pundir@linaro.org>
> Cc: YongQin Liu <yongqin.liu@linaro.org>
> Cc: John Youn <johnyoun@synopsys.com>
> Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Chen Yu <chenyu56@huawei.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2: Remove the extra dwc2_hsotg_core_init_disconnected() call
>      I had added, as suggested by Minas.
> ---
>   drivers/usb/dwc2/hcd.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index c263114..9bd60ec 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -3277,7 +3277,6 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
>   		dwc2_core_init(hsotg, false);
>   		dwc2_enable_global_interrupts(hsotg);
>   		spin_lock_irqsave(&hsotg->lock, flags);
> -		dwc2_hsotg_disconnect(hsotg);
>   		dwc2_hsotg_core_init_disconnected(hsotg, false);
>   		spin_unlock_irqrestore(&hsotg->lock, flags);
>   		dwc2_hsotg_core_connect(hsotg);
> @@ -3296,8 +3295,12 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
>   		if (count > 250)
>   			dev_err(hsotg->dev,
>   				"Connection id status change timed out\n");
> -		hsotg->op_state = OTG_STATE_A_HOST;
>   
> +		spin_lock_irqsave(&hsotg->lock, flags);
> +		dwc2_hsotg_disconnect(hsotg);
> +		spin_unlock_irqrestore(&hsotg->lock, flags);
> +
> +		hsotg->op_state = OTG_STATE_A_HOST;
>   		/* Initialize the Core for Host mode */
>   		dwc2_core_init(hsotg, false);
>   		dwc2_enable_global_interrupts(hsotg);
> 

This patch is required for the HiKey platform, because the assertion of 
the "Connector ID status change" interrupt is different: asserting on B 
connector unplug and goes to Host mode.
No any side effect on SNPS HAPS-DX platform where ConnIDStsChng 
assertion is correct.

Acked-by: Minas Harutyunyan <hminas@synopsys.com>
Tested-by: Minas Harutyunyan <hminas@synopsys.com>

Thanks,
Minas

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-10-24  9:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-23 21:32 [PATCH v2 0/3] dwc2 fixes for edge cases on hikey John Stultz
2017-10-23 21:32 ` [PATCH 1/3 v2] usb: dwc2: Improve gadget state disconnection handling John Stultz
2017-10-24  8:59   ` Minas Harutyunyan
2017-10-23 21:32 ` [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode John Stultz
2017-10-24  8:33   ` Minas Harutyunyan
2017-10-23 21:32 ` [PATCH 3/3 v2] usb: dwc2: Fix UDC state tracking John Stultz
2017-10-24  8:33   ` Minas Harutyunyan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.