* [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey
@ 2017-09-20 19:57 John Stultz
2017-09-20 19:57 ` [RESEND x2][PATCH 1/3] usb: dwc2: Improve gadget state disconnection handling John Stultz
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: John Stultz @ 2017-09-20 19:57 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 are 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.
I'd love to get some feedback on the patches and consideration
to be merged upstream.
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 | 8 ++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* [RESEND x2][PATCH 1/3] usb: dwc2: Improve gadget state disconnection handling 2017-09-20 19:57 [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Stultz @ 2017-09-20 19:57 ` John Stultz 2017-09-20 19:57 ` [RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode John Stultz ` (2 subsequent siblings) 3 siblings, 0 replies; 23+ messages in thread From: John Stultz @ 2017-09-20 19:57 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. The fix there was somewhat simple, as it just made sure to call dwc2_hsotg_disconnect() before we connected things up in OTG mode. 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> --- drivers/usb/dwc2/hcd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index c263114..df54428 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,13 @@ 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); + dwc2_hsotg_core_init_disconnected(hsotg, false); + 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] 23+ messages in thread
* [RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode 2017-09-20 19:57 [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Stultz 2017-09-20 19:57 ` [RESEND x2][PATCH 1/3] usb: dwc2: Improve gadget state disconnection handling John Stultz @ 2017-09-20 19:57 ` John Stultz 2017-10-03 10:02 ` Minas Harutyunyan 2017-09-20 19:57 ` [RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking John Stultz 2017-09-30 17:13 ` [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Youn 3 siblings, 1 reply; 23+ messages in thread From: John Stultz @ 2017-09-20 19:57 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 two previous patches mailed out earlier: https://lkml.org/lkml/2017/8/3/1008 https://lkml.org/lkml/2017/8/3/1010 ), 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 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] 23+ messages in thread
* Re: [RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode 2017-09-20 19:57 ` [RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode John Stultz @ 2017-10-03 10:02 ` Minas Harutyunyan 0 siblings, 0 replies; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-03 10:02 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 9/20/2017 11:57 PM, 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 two previous patches mailed out > earlier: > https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2017_8_3_1008&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=YbED3GZFyun_ID3-6Off3kdvd9xTUepWt4lzd2__ZSs&s=65BnVw7lagEfnyuD2WHnFlGTUnjvPHWyFiwL_F1vwfE&e= > https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2017_8_3_1010&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=YbED3GZFyun_ID3-6Off3kdvd9xTUepWt4lzd2__ZSs&s=MdrOhV0i6kRrV2mHK5zHIwE1eF21MsTkHIjvsV2k7uw&e= > ), 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 > 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); > Tested by: Minas Harutyunyan <hminas@synopsys.com> ^ permalink raw reply [flat|nested] 23+ messages in thread
* [RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking 2017-09-20 19:57 [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Stultz 2017-09-20 19:57 ` [RESEND x2][PATCH 1/3] usb: dwc2: Improve gadget state disconnection handling John Stultz 2017-09-20 19:57 ` [RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode John Stultz @ 2017-09-20 19:57 ` John Stultz 2017-10-03 10:02 ` Minas Harutyunyan 2017-09-30 17:13 ` [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Youn 3 siblings, 1 reply; 23+ messages in thread From: John Stultz @ 2017-09-20 19:57 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. 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 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] 23+ messages in thread
* Re: [RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking 2017-09-20 19:57 ` [RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking John Stultz @ 2017-10-03 10:02 ` Minas Harutyunyan 0 siblings, 0 replies; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-03 10:02 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 9/20/2017 11:57 PM, 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. > > 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 > 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); > } > > /** > Tested by: Minas Harutyunyan <hminas@synopsys.com> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-09-20 19:57 [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Stultz ` (2 preceding siblings ...) 2017-09-20 19:57 ` [RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking John Stultz @ 2017-09-30 17:13 ` John Youn 2017-10-03 9:58 ` Minas Harutyunyan 3 siblings, 1 reply; 23+ messages in thread From: John Youn @ 2017-09-30 17:13 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 09/20/2017 12:57 PM, John Stultz wrote: > So here are 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. > > I'd love to get some feedback on the patches and consideration > to be merged upstream. > > 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 | 8 ++++++-- > 2 files changed, 13 insertions(+), 2 deletions(-) > Hi John, I think we have something that fixes these issues. Minas, Could you take a look at this? I was not able to find the patches we talked about. If possible, please post them so that John can try them out. Thanks, John ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-09-30 17:13 ` [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Youn @ 2017-10-03 9:58 ` Minas Harutyunyan 2017-10-09 21:50 ` John Stultz 0 siblings, 1 reply; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-03 9:58 UTC (permalink / raw) To: John Youn, 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 9/30/2017 9:13 PM, John Youn wrote: > On 09/20/2017 12:57 PM, John Stultz wrote: >> So here are 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. >> >> I'd love to get some feedback on the patches and consideration >> to be merged upstream. >> >> 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 | 8 ++++++-- >> 2 files changed, 13 insertions(+), 2 deletions(-) >> > > Hi John, > > I think we have something that fixes these issues. > > Minas, > > Could you take a look at this? I was not able to find the patches we > talked about. If possible, please post them so that John can try them > out. > > Thanks, > John > Hi John Stultz, Could you please apply patch from Vardan Mikayelyan "usb: dwc2: Fix dwc2_hsotg_core_init_disconnected()" submitted at 02/25/2017 (https://marc.info/?l=linux-usb&m=148801589931039&w=2) instead of your "usb: dwc2: Improve gadget state disconnection handling" and test again failing scenario. Other 2 patches from series "[PATCH 0/3] dwc2 fixes for edge cases on hikey" are Ok. Thanks, Minas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-03 9:58 ` Minas Harutyunyan @ 2017-10-09 21:50 ` John Stultz 2017-10-12 7:59 ` Minas Harutyunyan 0 siblings, 1 reply; 23+ messages in thread From: John Stultz @ 2017-10-09 21:50 UTC (permalink / raw) To: Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On Tue, Oct 3, 2017 at 2:58 AM, Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote: > > Could you please apply patch from Vardan Mikayelyan "usb: dwc2: Fix > dwc2_hsotg_core_init_disconnected()" submitted at 02/25/2017 > (https://marc.info/?l=linux-usb&m=148801589931039&w=2) instead of your > "usb: dwc2: Improve gadget state disconnection handling" and test again > failing scenario. I may be misunderstanding htings, but I don't believe that patch addresses the same issue I'm trying to fix (I've tested with it, and it doesn't cause any trouble for me, but it also doesn't seem to address the corner-cases I'm hitting). My "Improve gadget state disconnection handling" patch handles the fact that when we switch from B/gadget mode to A/Host mode, we don't always go through a gadget disconnect path. So instead of calling the dwc2_hsotg_disconnect() path initially when switching to gadget mode (to avoid the state complaining that we set it up twice), we should instead be calling dwc2_hsotg_disconnect() when we are setting up the A/host mode. So for example, the follow-on fix to the UDC state won't properly work without my "Improve gadget state disconnection handling" patch, and "cat /sys/class/udc/f72c0000.usb/state" will always return configured (assuming gadget mode was used once) regardless of the gadget state, since we don't actually call dwc2_hsotg_disconnect when the otg plug is removed. > Other 2 patches from series "[PATCH 0/3] dwc2 fixes for edge cases on > hikey" are Ok. Thanks for the review/feedback! thanks -john ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-09 21:50 ` John Stultz @ 2017-10-12 7:59 ` Minas Harutyunyan 2017-10-12 18:06 ` John Stultz 0 siblings, 1 reply; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-12 7:59 UTC (permalink / raw) To: John Stultz, Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On 10/10/2017 1:50 AM, John Stultz wrote: > On Tue, Oct 3, 2017 at 2:58 AM, Minas Harutyunyan > <Minas.Harutyunyan@synopsys.com> wrote: >> >> Could you please apply patch from Vardan Mikayelyan "usb: dwc2: Fix >> dwc2_hsotg_core_init_disconnected()" submitted at 02/25/2017 >> (https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dusb-26m-3D148801589931039-26w-3D2&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=4y4_kSoJQIp-rJkvFNu8yXR67QxLLQrbFkjlyytMUCE&s=3Gmh7tVGk7ncQfBNUjkVdpRa1XX_jf7lWga7kR1O9bQ&e=) instead of your >> "usb: dwc2: Improve gadget state disconnection handling" and test again >> failing scenario. > > I may be misunderstanding htings, but I don't believe that patch > addresses the same issue I'm trying to fix (I've tested with it, and > it doesn't cause any trouble for me, but it also doesn't seem to > address the corner-cases I'm hitting). > > My "Improve gadget state disconnection handling" patch handles the > fact that when we switch from B/gadget mode to A/Host mode, we don't > always go through a gadget disconnect path. > > So instead of calling the dwc2_hsotg_disconnect() path initially when > switching to gadget mode (to avoid the state complaining that we set > it up twice), we should instead be calling dwc2_hsotg_disconnect() > when we are setting up the A/host mode. > > So for example, the follow-on fix to the UDC state won't properly work > without my "Improve gadget state disconnection handling" patch, and > "cat /sys/class/udc/f72c0000.usb/state" will always return configured > (assuming gadget mode was used once) regardless of the gadget state, > since we don't actually call dwc2_hsotg_disconnect when the otg plug > is removed. > >> Other 2 patches from series "[PATCH 0/3] dwc2 fixes for edge cases on >> hikey" are Ok. > > Thanks for the review/feedback! > > thanks > -john > Hi John Stultz, 1. Vardan's patch fixing issue when dwc2 switched from host to device mode. It's allow to make functional device after reconnecting without tracking UDC state. 2. I suppose that your patch "[RESEND x2][PATCH 1/3] usb: dwc2: Improve gadget state disconnection handling" not a good way to set correct UDC state. You added calling device mode functions dwc2_hsotg_disconnect() and dwc2_hsotg_core_init_disconnected() while core in Host mode and as result additional unwanted "mode mismatch" interrupts will be asserted. 3. Function dwc2_conn_id_status_change() called when connector ID status changed. This interrupt asserted only when A-plug connected or disconnected. Connecting/disconnecting B-plug doesn't assert this interrupt. Thanks, Minas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-12 7:59 ` Minas Harutyunyan @ 2017-10-12 18:06 ` John Stultz 2017-10-16 8:36 ` Minas Harutyunyan 0 siblings, 1 reply; 23+ messages in thread From: John Stultz @ 2017-10-12 18:06 UTC (permalink / raw) To: Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On Thu, Oct 12, 2017 at 12:59 AM, Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote: > > 1. Vardan's patch fixing issue when dwc2 switched from host to device > mode. It's allow to make functional device after reconnecting without > tracking UDC state. While I'm sure Vardan's patch is useful, I worry that its resolving a different issue then what I'm trying to address, as it doesn't seem to help the problems I'm seeing. > 2. I suppose that your patch "[RESEND x2][PATCH 1/3] usb: dwc2: Improve > gadget state disconnection handling" not a good way to set correct UDC > state. You added calling device mode functions dwc2_hsotg_disconnect() > and dwc2_hsotg_core_init_disconnected() while core in Host mode and as > result additional unwanted "mode mismatch" interrupts will be asserted. Apologies, I'm not sure I'm understanding you here. Forgive me if I'm misinterpreting your feedback. So, the "usb: dwc2: Improve gadget state disconnection handling" isn't itself doing the UDC state handling. Personally I see it as improving a previously applied fix (dad3f793f20f - usb: dwc2: Make sure we disconnect the gadget state). So instead of calling dwc2_hsotg_disconnect() in dwc2_conn_id_status_change() when transitioning INTO device/B mode, which was added due to earlier problems with state tracking (as when we unplug the gadget cable, nothing else triggers the hsotg_disconnect code), I'm instead suggesting we call dwc2_hsotg_disconnect() when we transition into host/A mode. This only allows us to do proper UDC state handling later, since we properly run the disconnect code for device when we switch into host mode. If I'm understanding you, you seem to be objecting to this, as calling dwc2_hsotg_disconnect() while we are transitioning to host mode can cause "mode mismatch" interrupts. I've not seen this in practice with this patch, but you know the logic better and it could be possible. Now, I'm of course open to other approaches, but it seems that we need *something* to call dwc2_hsotg_disconnect() when the otg cable is removed (which currently just doesn't happen). The earlier patch calling dwc2_hsotg_disconnect() when we are entering device/B mode avoids the state tracking warnings but, doesn't seem correct (nor does it allow for things like proper UDC state handling). > 3. Function dwc2_conn_id_status_change() called when connector ID status > changed. This interrupt asserted only when A-plug connected or > disconnected. Connecting/disconnecting B-plug doesn't assert this interrupt. Ok. What I'm seeing may be somewhat hardware specific then, as on HiKey, we have a switch that enables a on-board USB hub when the OTG plug is removed. This may be the root of the issue, but I guess I'm at a loss for how things should be handled here. When the b-plug is disconnected, we need to do something to signal to the core that we aren't connected, no? And it seems that your point that the conn_id_status_change logic only runs on the A-plug connect/disconnect mirrors the usage for the B plug (ie: if A is disconnected, we enter B mode, thus if A is connected, shouldn't we disable/disconnect B mode?). I suspect there's something more subtle to your statement though, so if you could expand a bit so I could better understand I'd appreciate it. thanks -john ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-12 18:06 ` John Stultz @ 2017-10-16 8:36 ` Minas Harutyunyan 2017-10-16 21:34 ` John Stultz 0 siblings, 1 reply; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-16 8:36 UTC (permalink / raw) To: John Stultz, Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On 10/12/2017 10:06 PM, John Stultz wrote: > On Thu, Oct 12, 2017 at 12:59 AM, Minas Harutyunyan > <Minas.Harutyunyan@synopsys.com> wrote: >> >> 1. Vardan's patch fixing issue when dwc2 switched from host to device >> mode. It's allow to make functional device after reconnecting without >> tracking UDC state. > > While I'm sure Vardan's patch is useful, I worry that its resolving a > different issue then what I'm trying to address, as it doesn't seem to > help the problems I'm seeing. > >> 2. I suppose that your patch "[RESEND x2][PATCH 1/3] usb: dwc2: Improve >> gadget state disconnection handling" not a good way to set correct UDC >> state. You added calling device mode functions dwc2_hsotg_disconnect() >> and dwc2_hsotg_core_init_disconnected() while core in Host mode and as >> result additional unwanted "mode mismatch" interrupts will be asserted. > > Apologies, I'm not sure I'm understanding you here. Forgive me if I'm > misinterpreting your feedback. > > So, the "usb: dwc2: Improve gadget state disconnection handling" isn't > itself doing the UDC state handling. > > Personally I see it as improving a previously applied fix > (dad3f793f20f - usb: dwc2: Make sure we disconnect the gadget state). > So instead of calling dwc2_hsotg_disconnect() in > dwc2_conn_id_status_change() when transitioning INTO device/B mode, > which was added due to earlier problems with state tracking (as when > we unplug the gadget cable, nothing else triggers the hsotg_disconnect > code), I'm instead suggesting we call dwc2_hsotg_disconnect() when we > transition into host/A mode. > > This only allows us to do proper UDC state handling later, since we > properly run the disconnect code for device when we switch into host > mode. > > If I'm understanding you, you seem to be objecting to this, as calling > dwc2_hsotg_disconnect() while we are transitioning to host mode can > cause "mode mismatch" interrupts. I've not seen this in practice with > this patch, but you know the logic better and it could be possible. > > Now, I'm of course open to other approaches, but it seems that we need > *something* to call dwc2_hsotg_disconnect() when the otg cable is > removed (which currently just doesn't happen). The earlier patch > calling dwc2_hsotg_disconnect() when we are entering device/B mode > avoids the state tracking warnings but, doesn't seem correct (nor does > it allow for things like proper UDC state handling). > >> 3. Function dwc2_conn_id_status_change() called when connector ID status >> changed. This interrupt asserted only when A-plug connected or >> disconnected. Connecting/disconnecting B-plug doesn't assert this interrupt. > > Ok. What I'm seeing may be somewhat hardware specific then, as on > HiKey, we have a switch that enables a on-board USB hub when the OTG > plug is removed. This may be the root of the issue, but I guess I'm > at a loss for how things should be handled here. > > When the b-plug is disconnected, we need to do something to signal to > the core that we aren't connected, no? > > And it seems that your point that the conn_id_status_change logic only > runs on the A-plug connect/disconnect mirrors the usage for the B plug > (ie: if A is disconnected, we enter B mode, thus if A is connected, > shouldn't we disable/disconnect B mode?). I suspect there's something > more subtle to your statement though, so if you could expand a bit so > I could better understand I'd appreciate it. > > thanks > -john > Hi John Stultz, On b-plug disconnect should asserted GOTGINT.SesEndDet interrupt. According previously sent by you register dump (GHWCFG2 = 0x23affc70) your core OTG_MODE=0. Bellow fragment from programming guide on Device disconnect: "7.3Device Disconnection The device session ends when the USB cable is disconnected or if the VBUS is switched off by the Host. The device disconnect flow varies depending on the value of the OTG_MODE configuration parameter. When OTG_MODE = 0,1, or 3 When OTG_MODE is set to 0,1, or 3, the device disconnect flow is as follows: 1. When the USB cable is unplugged or when the VBUS is switched off by the Host, the Device core trigger GINTSTS.OTGInt [bit 2] interrupt bit. 2. When the device application detects GINTSTS.OTGInt interrupt, it checks that the GOTGINT.SesEndDet (Session End Detected) bit is set to 1’b1." So, you should receive and handle "Session End Detected". In function dwc2_handle_otg_intr() on this interrupt (in device mode) calling dwc2_hsotg_disconnect() function. By adding your patch "[PATCH 3/3] usb: dwc2: Fix UDC state tracking" state changed to not attached as required. Thanks, Minas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-16 8:36 ` Minas Harutyunyan @ 2017-10-16 21:34 ` John Stultz 2017-10-17 8:41 ` Minas Harutyunyan 0 siblings, 1 reply; 23+ messages in thread From: John Stultz @ 2017-10-16 21:34 UTC (permalink / raw) To: Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On Mon, Oct 16, 2017 at 1:36 AM, Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote: > On b-plug disconnect should asserted GOTGINT.SesEndDet interrupt. > According previously sent by you register dump (GHWCFG2 = 0x23affc70) > your core OTG_MODE=0. > Bellow fragment from programming guide on Device disconnect: > > "7.3Device Disconnection > The device session ends when the USB cable is disconnected or if the > VBUS is switched off by the Host. The > device disconnect flow varies depending on the value of the OTG_MODE > configuration parameter. > > When OTG_MODE = 0,1, or 3 > When OTG_MODE is set to 0,1, or 3, the device disconnect flow is as follows: > 1. When the USB cable is unplugged or when the VBUS is switched off by > the Host, the Device core > trigger GINTSTS.OTGInt [bit 2] interrupt bit. > 2. When the device application detects GINTSTS.OTGInt interrupt, it > checks that the > GOTGINT.SesEndDet (Session End Detected) bit is set to 1’b1." > > So, you should receive and handle "Session End Detected". In function > dwc2_handle_otg_intr() on this interrupt (in device mode) calling > dwc2_hsotg_disconnect() function. By adding your patch "[PATCH 3/3] usb: > dwc2: Fix UDC state tracking" state changed to not attached as required. So, on the HiKey board (using 4.14-rc5 + Vardan's patch), I'm not seeing the GOTGINT_SES_END_DET in dwc2_handle_otg_intr() when I remove the USB OTG cable. In fact, I'm not seeing any calls to dwc2_handle_otg_intr()... which seems... odd maybe? Any clues as to what might be going wrong then? thanks -john ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-16 21:34 ` John Stultz @ 2017-10-17 8:41 ` Minas Harutyunyan 2017-10-19 6:46 ` Minas Harutyunyan 2017-10-19 20:06 ` John Stultz 0 siblings, 2 replies; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-17 8:41 UTC (permalink / raw) To: John Stultz, Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On 10/17/2017 1:34 AM, John Stultz wrote: > On Mon, Oct 16, 2017 at 1:36 AM, Minas Harutyunyan > <Minas.Harutyunyan@synopsys.com> wrote: >> On b-plug disconnect should asserted GOTGINT.SesEndDet interrupt. >> According previously sent by you register dump (GHWCFG2 = 0x23affc70) >> your core OTG_MODE=0. >> Bellow fragment from programming guide on Device disconnect: >> >> "7.3Device Disconnection >> The device session ends when the USB cable is disconnected or if the >> VBUS is switched off by the Host. The >> device disconnect flow varies depending on the value of the OTG_MODE >> configuration parameter. >> >> When OTG_MODE = 0,1, or 3 >> When OTG_MODE is set to 0,1, or 3, the device disconnect flow is as follows: >> 1. When the USB cable is unplugged or when the VBUS is switched off by >> the Host, the Device core >> trigger GINTSTS.OTGInt [bit 2] interrupt bit. >> 2. When the device application detects GINTSTS.OTGInt interrupt, it >> checks that the >> GOTGINT.SesEndDet (Session End Detected) bit is set to 1’b1." >> >> So, you should receive and handle "Session End Detected". In function >> dwc2_handle_otg_intr() on this interrupt (in device mode) calling >> dwc2_hsotg_disconnect() function. By adding your patch "[PATCH 3/3] usb: >> dwc2: Fix UDC state tracking" state changed to not attached as required. > > > So, on the HiKey board (using 4.14-rc5 + Vardan's patch), I'm not > seeing the GOTGINT_SES_END_DET in dwc2_handle_otg_intr() when I remove > the USB OTG cable. > > In fact, I'm not seeing any calls to dwc2_handle_otg_intr()... which > seems... odd maybe? Any clues as to what might be going wrong then? > > thanks > -john > Hi John Stultz, So, on Hikey board on unplug B connector GOTGINT.SesEndDet interrupt not asserted, instead asserted GINTSTS_CONIDSTSCHNG. Please, confirm. In this case without your patch "[PATCH 1/3] usb: dwc2: Improve gadget state disconnection handling" but by applying your patch "[PATCH 3/3] usb: dwc2: Fix UDC state tracking": 1. On B plug connect UDC state will be set to "configured" 2. On B plug disconnect - "not attached". Is it Ok for you? Meantime, I'll check with HW team why GOTGINT.SesEndDet interrupt not asserted on unplug B connector. Thanks, Minas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-17 8:41 ` Minas Harutyunyan @ 2017-10-19 6:46 ` Minas Harutyunyan 2017-10-19 20:20 ` John Stultz 2017-10-19 20:06 ` John Stultz 1 sibling, 1 reply; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-19 6:46 UTC (permalink / raw) To: John Stultz, Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On 10/17/2017 12:41 PM, Minas Harutyunyan wrote: > On 10/17/2017 1:34 AM, John Stultz wrote: >> On Mon, Oct 16, 2017 at 1:36 AM, Minas Harutyunyan >> <Minas.Harutyunyan@synopsys.com> wrote: >>> On b-plug disconnect should asserted GOTGINT.SesEndDet interrupt. >>> According previously sent by you register dump (GHWCFG2 = 0x23affc70) >>> your core OTG_MODE=0. >>> Bellow fragment from programming guide on Device disconnect: >>> >>> "7.3Device Disconnection >>> The device session ends when the USB cable is disconnected or if the >>> VBUS is switched off by the Host. The >>> device disconnect flow varies depending on the value of the OTG_MODE >>> configuration parameter. >>> >>> When OTG_MODE = 0,1, or 3 >>> When OTG_MODE is set to 0,1, or 3, the device disconnect flow is as follows: >>> 1. When the USB cable is unplugged or when the VBUS is switched off by >>> the Host, the Device core >>> trigger GINTSTS.OTGInt [bit 2] interrupt bit. >>> 2. When the device application detects GINTSTS.OTGInt interrupt, it >>> checks that the >>> GOTGINT.SesEndDet (Session End Detected) bit is set to 1’b1." >>> >>> So, you should receive and handle "Session End Detected". In function >>> dwc2_handle_otg_intr() on this interrupt (in device mode) calling >>> dwc2_hsotg_disconnect() function. By adding your patch "[PATCH 3/3] usb: >>> dwc2: Fix UDC state tracking" state changed to not attached as required. >> >> >> So, on the HiKey board (using 4.14-rc5 + Vardan's patch), I'm not >> seeing the GOTGINT_SES_END_DET in dwc2_handle_otg_intr() when I remove >> the USB OTG cable. >> >> In fact, I'm not seeing any calls to dwc2_handle_otg_intr()... which >> seems... odd maybe? Any clues as to what might be going wrong then? >> >> thanks >> -john >> > Hi John Stultz, > So, on Hikey board on unplug B connector GOTGINT.SesEndDet interrupt not > asserted, instead asserted GINTSTS_CONIDSTSCHNG. Please, confirm. > > In this case without your patch "[PATCH 1/3] usb: dwc2: Improve gadget > state disconnection handling" but by applying your patch "[PATCH 3/3] > usb: dwc2: Fix UDC state tracking": > 1. On B plug connect UDC state will be set to "configured" > 2. On B plug disconnect - "not attached". > Is it Ok for you? > > Meantime, I'll check with HW team why GOTGINT.SesEndDet interrupt not > asserted on unplug B connector. > > Thanks, > Minas > Hi John Stultz, Could you please apply this patch. Please not apply your patch series "[PATCH 0/3] dwc2 fixes for edge cases on hikey" to check only below patch. If you confirm that this patch fix your issue with "Transaction Error" and " ChHltd set, but reason is unknown" I'll submit to LKML as final patch. Then can be applied your patch series, without patch 1/3 (changing in connector id status change) to correctly set UDC states. diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index f4ef159b538e..7da22152df68 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -331,6 +331,9 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg) usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); usbcfg &= ~(GUSBCFG_HNPCAP | GUSBCFG_SRPCAP); + /* Set HS/FS Timeout Calibration */ + usbcfg |= GUSBCFG_TOUTCAL(7); + switch (hsotg->hw_params.op_mode) { case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: if (hsotg->params.otg_cap == Thanks, Minas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-19 6:46 ` Minas Harutyunyan @ 2017-10-19 20:20 ` John Stultz 2017-10-20 11:48 ` Minas Harutyunyan 0 siblings, 1 reply; 23+ messages in thread From: John Stultz @ 2017-10-19 20:20 UTC (permalink / raw) To: Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On Wed, Oct 18, 2017 at 11:46 PM, Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote: > Could you please apply this patch. Please not apply your patch series > "[PATCH 0/3] dwc2 fixes for edge cases on hikey" to check only below patch. > If you confirm that this patch fix your issue with "Transaction Error" > and " ChHltd set, but reason is unknown" I'll submit to LKML as final patch. > Then can be applied your patch series, without patch 1/3 (changing in > connector id status change) to correctly set UDC states. > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index > f4ef159b538e..7da22152df68 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -331,6 +331,9 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg) > usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); > usbcfg &= ~(GUSBCFG_HNPCAP | GUSBCFG_SRPCAP); > > + /* Set HS/FS Timeout Calibration */ > + usbcfg |= GUSBCFG_TOUTCAL(7); > + > switch (hsotg->hw_params.op_mode) { > case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: > if (hsotg->params.otg_cap == So while using this patch and the earlier one from Vardan, I don't see the "Transaction Error" and " ChHltd set, but reason is unknown" messages, but I do see: [ 272.290459] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290476] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290491] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290507] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290522] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290538] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290554] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290570] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290585] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290687] dwc2 f72c0000.usb: dwc2_hsotg_ep_stop_xfr: timeout DIEPINT.NAKEFF [ 272.290702] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode [ 272.290717] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode After which the USB eth device stops functioning. So its not really much functional change from without the patch from my perspective. Additionally, I'm still not seeing any disconnect calls when removing the B plug. Re-adding my three patches ontop of this change and Vardan's does fix both the UDC state handling and avoids usb devices from failing when the board is in host mode and the /dev/usb-ffs/adb/ep0 file is closed by adbd. thanks -john ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-19 20:20 ` John Stultz @ 2017-10-20 11:48 ` Minas Harutyunyan 2017-10-23 9:19 ` Minas Harutyunyan 0 siblings, 1 reply; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-20 11:48 UTC (permalink / raw) To: John Stultz, Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On 10/20/2017 12:20 AM, John Stultz wrote: > On Wed, Oct 18, 2017 at 11:46 PM, Minas Harutyunyan > <Minas.Harutyunyan@synopsys.com> wrote: >> Could you please apply this patch. Please not apply your patch series >> "[PATCH 0/3] dwc2 fixes for edge cases on hikey" to check only below patch. >> If you confirm that this patch fix your issue with "Transaction Error" >> and " ChHltd set, but reason is unknown" I'll submit to LKML as final patch. >> Then can be applied your patch series, without patch 1/3 (changing in >> connector id status change) to correctly set UDC states. >> >> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index >> f4ef159b538e..7da22152df68 100644 >> --- a/drivers/usb/dwc2/hcd.c >> +++ b/drivers/usb/dwc2/hcd.c >> @@ -331,6 +331,9 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg) >> usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); >> usbcfg &= ~(GUSBCFG_HNPCAP | GUSBCFG_SRPCAP); >> >> + /* Set HS/FS Timeout Calibration */ >> + usbcfg |= GUSBCFG_TOUTCAL(7); >> + >> switch (hsotg->hw_params.op_mode) { >> case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: >> if (hsotg->params.otg_cap == > > > So while using this patch and the earlier one from Vardan, I don't see > the "Transaction Error" > and " ChHltd set, but reason is unknown" messages, but I do see: > > [ 272.290459] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290476] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290491] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290507] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290522] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290538] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290554] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290570] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290585] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290687] dwc2 f72c0000.usb: dwc2_hsotg_ep_stop_xfr: timeout DIEPINT.NAKEFF > [ 272.290702] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > [ 272.290717] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently > in Host mode > > After which the USB eth device stops functioning. > > So its not really much functional change from without the patch from > my perspective. > > Additionally, I'm still not seeing any disconnect calls when removing > the B plug. > > Re-adding my three patches ontop of this change and Vardan's does fix > both the UDC state handling and avoids usb devices from failing when > the board is in host mode and the /dev/usb-ffs/adb/ep0 file is closed > by adbd. > > > thanks > -john > Hi John Stultz, It's good news that you not see more "Transaction error" and "ChHlt set, but reason is unknown"! To avoid "Mode Mismatch" interrupt please apply your patch 2/3. Additionally I see that in your setup used UTMI+ 8-bit PHY but GUSBCFG.USBTRDTIM set to 5, but should be 9. In device mode in function dwc2_hsotg_core_init_disconnected() GUSBCFG related fields TOUTCAL and USBTRDTIM setting again, but in Host mode these fields are not set at all. This can be reason why using your patch 1/3 you overcome TOUTCAL and USBTRDTIM issue. So, suggesting another patch for USBTRDTIM field bellow: @@ -2311,10 +2314,17 @@ static int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup) */ static void dwc2_core_host_init(struct dwc2_hsotg *hsotg) { - u32 hcfg, hfir, otgctl; + u32 hcfg, hfir, otgctl, usbcfg, trdtrim; dev_dbg(hsotg->dev, "%s(%p)\n", __func__, hsotg); + /* Set USBTrdTim value */ + usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); + usbcfg &= ~GUSBCFG_USBTRDTIM_MASK; + trdtrim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5; + usbcfg |= (trdtrim << GUSBCFG_USBTRDTIM_SHIFT); + dwc2_writel(usbcfg, hsotg->regs + GUSBCFG); + /* Restart the Phy Clock */ dwc2_writel(0, hsotg->regs + PCGCTL); Please apply follow patches: 1. Vardan's patch. 2. Patch for TOUTCAL. 3. Patch for USBTRDTIM (see above). 4. Your patch 2/3 to avoid "Mode Mismatch" interrupts. 5. Your patch 3/3 to set udc state to "not attached". Unfortunately, we haven't hikey board in our lab and can't fully test patches on our side. Thanks, Minas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-20 11:48 ` Minas Harutyunyan @ 2017-10-23 9:19 ` Minas Harutyunyan 2017-10-23 20:41 ` John Stultz 0 siblings, 1 reply; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-23 9:19 UTC (permalink / raw) To: John Stultz, Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On 10/20/2017 3:57 PM, Minas Harutyunyan wrote: > On 10/20/2017 12:20 AM, John Stultz wrote: >> On Wed, Oct 18, 2017 at 11:46 PM, Minas Harutyunyan >> <Minas.Harutyunyan@synopsys.com> wrote: >>> Could you please apply this patch. Please not apply your patch series >>> "[PATCH 0/3] dwc2 fixes for edge cases on hikey" to check only below patch. >>> If you confirm that this patch fix your issue with "Transaction Error" >>> and " ChHltd set, but reason is unknown" I'll submit to LKML as final patch. >>> Then can be applied your patch series, without patch 1/3 (changing in >>> connector id status change) to correctly set UDC states. >>> >>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index >>> f4ef159b538e..7da22152df68 100644 >>> --- a/drivers/usb/dwc2/hcd.c >>> +++ b/drivers/usb/dwc2/hcd.c >>> @@ -331,6 +331,9 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg) >>> usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); >>> usbcfg &= ~(GUSBCFG_HNPCAP | GUSBCFG_SRPCAP); >>> >>> + /* Set HS/FS Timeout Calibration */ >>> + usbcfg |= GUSBCFG_TOUTCAL(7); >>> + >>> switch (hsotg->hw_params.op_mode) { >>> case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: >>> if (hsotg->params.otg_cap == >> >> >> So while using this patch and the earlier one from Vardan, I don't see >> the "Transaction Error" >> and " ChHltd set, but reason is unknown" messages, but I do see: >> >> [ 272.290459] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290476] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290491] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290507] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290522] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290538] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290554] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290570] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290585] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290687] dwc2 f72c0000.usb: dwc2_hsotg_ep_stop_xfr: timeout DIEPINT.NAKEFF >> [ 272.290702] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290717] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> >> After which the USB eth device stops functioning. >> >> So its not really much functional change from without the patch from >> my perspective. >> >> Additionally, I'm still not seeing any disconnect calls when removing >> the B plug. >> >> Re-adding my three patches ontop of this change and Vardan's does fix >> both the UDC state handling and avoids usb devices from failing when >> the board is in host mode and the /dev/usb-ffs/adb/ep0 file is closed >> by adbd. >> >> >> thanks >> -john >> > > Hi John Stultz, > > It's good news that you not see more "Transaction error" and "ChHlt set, > but reason is unknown"! > > To avoid "Mode Mismatch" interrupt please apply your patch 2/3. > > Additionally I see that in your setup used UTMI+ 8-bit PHY but > GUSBCFG.USBTRDTIM set to 5, but should be 9. > In device mode in function dwc2_hsotg_core_init_disconnected() GUSBCFG > related fields TOUTCAL and USBTRDTIM setting again, but in Host mode > these fields are not set at all. This can be reason why using your patch > 1/3 you overcome TOUTCAL and USBTRDTIM issue. > > So, suggesting another patch for USBTRDTIM field bellow: > > @@ -2311,10 +2314,17 @@ static int dwc2_core_init(struct dwc2_hsotg > *hsotg, bool initial_setup) > */ > static void dwc2_core_host_init(struct dwc2_hsotg *hsotg) > { > - u32 hcfg, hfir, otgctl; > + u32 hcfg, hfir, otgctl, usbcfg, trdtrim; > > dev_dbg(hsotg->dev, "%s(%p)\n", __func__, hsotg); > > + /* Set USBTrdTim value */ > + usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); > + usbcfg &= ~GUSBCFG_USBTRDTIM_MASK; > + trdtrim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5; > + usbcfg |= (trdtrim << GUSBCFG_USBTRDTIM_SHIFT); > + dwc2_writel(usbcfg, hsotg->regs + GUSBCFG); > + > /* Restart the Phy Clock */ > dwc2_writel(0, hsotg->regs + PCGCTL); > > > Please apply follow patches: > 1. Vardan's patch. > 2. Patch for TOUTCAL. > 3. Patch for USBTRDTIM (see above). > 4. Your patch 2/3 to avoid "Mode Mismatch" interrupts. > 5. Your patch 3/3 to set udc state to "not attached". > > Unfortunately, we haven't hikey board in our lab and can't fully test > patches on our side. > > > Thanks, > Minas > Hi John Stultz, Could you please verify on your setup follow patches: 1. Vardan's patch. 2. Patch for TOUTCAL&USBTRDTIM programming (new version see below). 4. Your patch 2/3 to avoid "Mode Mismatch" interrupts. 5. Your patch 3/3 to set udc state to "not attached". 6. Your patch 1/3, but remove dwc2_hsotg_core_init_disconnected() function call from Host starting brnch, keep *only* dwc2_hsotg_disconnect() to change UDC state to "not attached". diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index f4ef159b538e..1f65464055b9 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -2311,10 +2311,17 @@ static int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup) */ static void dwc2_core_host_init(struct dwc2_hsotg *hsotg) { - u32 hcfg, hfir, otgctl; + u32 hcfg, hfir, otgctl, usbcfg, val; dev_dbg(hsotg->dev, "%s(%p)\n", __func__, hsotg); + /* Set HS/FS Timeout Calibration and USBTrdTim */ + usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); + usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_USBTRDTIM_MASK); + val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5; + usbcfg |= (GUSBCFG_TOUTCAL(7) | (val << GUSBCFG_USBTRDTIM_SHIFT)); + dwc2_writel(usbcfg, hsotg->regs + GUSBCFG); + /* Restart the Phy Clock */ dwc2_writel(0, hsotg->regs + PCGCTL); Thanks, Minas ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-23 9:19 ` Minas Harutyunyan @ 2017-10-23 20:41 ` John Stultz 2017-10-23 21:36 ` John Stultz 2017-10-24 9:47 ` Minas Harutyunyan 0 siblings, 2 replies; 23+ messages in thread From: John Stultz @ 2017-10-23 20:41 UTC (permalink / raw) To: Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On Mon, Oct 23, 2017 at 2:19 AM, Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote: > Could you please verify on your setup follow patches: > 1. Vardan's patch. > 2. Patch for TOUTCAL&USBTRDTIM programming (new version see below). > 4. Your patch 2/3 to avoid "Mode Mismatch" interrupts. > 5. Your patch 3/3 to set udc state to "not attached". > 6. Your patch 1/3, but remove dwc2_hsotg_core_init_disconnected() > function call from Host starting brnch, keep *only* > dwc2_hsotg_disconnect() to change UDC state to "not attached". So yes, this set does seem to work ok for me. Though neither Vardan's patch or the TOUTCAL/USBTRDTIM patch seem to have much effect either way (I need to do more testing just to be sure, but for the use cases I've had trouble with they don't seem to do much). I'm happy to rework my earlier patch #1/3 to remove dwc2_hsotg_core_init_disconnected() and resend. thanks -john ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-23 20:41 ` John Stultz @ 2017-10-23 21:36 ` John Stultz 2017-10-24 9:47 ` Minas Harutyunyan 1 sibling, 0 replies; 23+ messages in thread From: John Stultz @ 2017-10-23 21:36 UTC (permalink / raw) To: Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On Mon, Oct 23, 2017 at 1:41 PM, John Stultz <john.stultz@linaro.org> wrote: > On Mon, Oct 23, 2017 at 2:19 AM, Minas Harutyunyan > <Minas.Harutyunyan@synopsys.com> wrote: >> Could you please verify on your setup follow patches: >> 1. Vardan's patch. >> 2. Patch for TOUTCAL&USBTRDTIM programming (new version see below). >> 4. Your patch 2/3 to avoid "Mode Mismatch" interrupts. >> 5. Your patch 3/3 to set udc state to "not attached". >> 6. Your patch 1/3, but remove dwc2_hsotg_core_init_disconnected() >> function call from Host starting brnch, keep *only* >> dwc2_hsotg_disconnect() to change UDC state to "not attached". > > So yes, this set does seem to work ok for me. Though neither Vardan's > patch or the TOUTCAL/USBTRDTIM patch seem to have much effect either > way (I need to do more testing just to be sure, but for the use cases > I've had trouble with they don't seem to do much). > > I'm happy to rework my earlier patch #1/3 to remove > dwc2_hsotg_core_init_disconnected() and resend. Just resent the set of my changes with the above tweak. The stack including Vardan's and your fixes that I also tested with (but to little effect) can be found here: https://git.linaro.org/people/john.stultz/android-dev.git/log/?id=4302a823816e944a6f548b05049c4e4c006bbadd thanks -john ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-23 20:41 ` John Stultz 2017-10-23 21:36 ` John Stultz @ 2017-10-24 9:47 ` Minas Harutyunyan 1 sibling, 0 replies; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-24 9:47 UTC (permalink / raw) To: John Stultz, Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On 10/24/2017 12:41 AM, John Stultz wrote: > On Mon, Oct 23, 2017 at 2:19 AM, Minas Harutyunyan > <Minas.Harutyunyan@synopsys.com> wrote: >> Could you please verify on your setup follow patches: >> 1. Vardan's patch. >> 2. Patch for TOUTCAL&USBTRDTIM programming (new version see below). >> 4. Your patch 2/3 to avoid "Mode Mismatch" interrupts. >> 5. Your patch 3/3 to set udc state to "not attached". >> 6. Your patch 1/3, but remove dwc2_hsotg_core_init_disconnected() >> function call from Host starting brnch, keep *only* >> dwc2_hsotg_disconnect() to change UDC state to "not attached". > > So yes, this set does seem to work ok for me. Though neither Vardan's > patch or the TOUTCAL/USBTRDTIM patch seem to have much effect either > way (I need to do more testing just to be sure, but for the use cases > I've had trouble with they don't seem to do much). > > I'm happy to rework my earlier patch #1/3 to remove > dwc2_hsotg_core_init_disconnected() and resend. > > thanks > -john > Hi John Stultz, 1. Vardan's patch required for cases when core switching from host mode to device mode. On host disconnect hsotg->lx_state set to DWC2_L2 as result dwc2_hsotg_enqueue_setup() failed because enqueuing should be done in DWC2_L0 state. This patch set lx_state to DWC2_L0 before enqueuing setup transfer. 2. TOUTCAL&USBTRDTIM patch adding missing USBCFG programming in host mode. These fields even if was programmed in device mode (see function dwc2_hsotg_core_init_disconnected()) will be resetting to POR values after core soft reset applied. Programming of these fields to correct values allow fix issues with lot of transaction errors due to timeouts and turnarrounds on USB bus. Previously on TOUTCAL patch you wrote: "So while using this patch and the earlier one from Vardan, I don't see the "Transaction Error" and " ChHltd set, but reason is unknown" messages". Thanks, Minas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-17 8:41 ` Minas Harutyunyan 2017-10-19 6:46 ` Minas Harutyunyan @ 2017-10-19 20:06 ` John Stultz 2017-10-20 11:26 ` Minas Harutyunyan 1 sibling, 1 reply; 23+ messages in thread From: John Stultz @ 2017-10-19 20:06 UTC (permalink / raw) To: Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb On Tue, Oct 17, 2017 at 1:41 AM, Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote: > On 10/17/2017 1:34 AM, John Stultz wrote: >> On Mon, Oct 16, 2017 at 1:36 AM, Minas Harutyunyan >> <Minas.Harutyunyan@synopsys.com> wrote: >>> On b-plug disconnect should asserted GOTGINT.SesEndDet interrupt. >>> According previously sent by you register dump (GHWCFG2 = 0x23affc70) >>> your core OTG_MODE=0. >>> Bellow fragment from programming guide on Device disconnect: >>> >>> "7.3Device Disconnection >>> The device session ends when the USB cable is disconnected or if the >>> VBUS is switched off by the Host. The >>> device disconnect flow varies depending on the value of the OTG_MODE >>> configuration parameter. >>> >>> When OTG_MODE = 0,1, or 3 >>> When OTG_MODE is set to 0,1, or 3, the device disconnect flow is as follows: >>> 1. When the USB cable is unplugged or when the VBUS is switched off by >>> the Host, the Device core >>> trigger GINTSTS.OTGInt [bit 2] interrupt bit. >>> 2. When the device application detects GINTSTS.OTGInt interrupt, it >>> checks that the >>> GOTGINT.SesEndDet (Session End Detected) bit is set to 1’b1." >>> >>> So, you should receive and handle "Session End Detected". In function >>> dwc2_handle_otg_intr() on this interrupt (in device mode) calling >>> dwc2_hsotg_disconnect() function. By adding your patch "[PATCH 3/3] usb: >>> dwc2: Fix UDC state tracking" state changed to not attached as required. >> >> >> So, on the HiKey board (using 4.14-rc5 + Vardan's patch), I'm not >> seeing the GOTGINT_SES_END_DET in dwc2_handle_otg_intr() when I remove >> the USB OTG cable. >> >> In fact, I'm not seeing any calls to dwc2_handle_otg_intr()... which >> seems... odd maybe? Any clues as to what might be going wrong then? >> >> thanks >> -john >> > Hi John Stultz, > So, on Hikey board on unplug B connector GOTGINT.SesEndDet interrupt not > asserted, instead asserted GINTSTS_CONIDSTSCHNG. Please, confirm. Correct. On B unplug, I see: dwc2_handle_conn_id_status_change_intr: ++Connector ID Status Change Interrupt++ (Host) And I never see any calls to dwc2_handle_otg_intr(). > In this case without your patch "[PATCH 1/3] usb: dwc2: Improve gadget > state disconnection handling" but by applying your patch "[PATCH 3/3] > usb: dwc2: Fix UDC state tracking": > 1. On B plug connect UDC state will be set to "configured" > 2. On B plug disconnect - "not attached". > Is it Ok for you? So this is what I expect, but I don't see it. Since without my patch, nothing seems to call disconenct when the B plug is disconnected, I still see: # cat /sys/class/udc/f72c0000.usb/state configured thanks -john ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey 2017-10-19 20:06 ` John Stultz @ 2017-10-20 11:26 ` Minas Harutyunyan 0 siblings, 0 replies; 23+ messages in thread From: Minas Harutyunyan @ 2017-10-20 11:26 UTC (permalink / raw) To: John Stultz, Minas Harutyunyan Cc: John Youn, lkml, Wei Xu, Guodong Xu, Amit Pundir, YongQin Liu, Douglas Anderson, Chen Yu, Felipe Balbi, Greg Kroah-Hartman, linux-usb Hi John Stultz, On 10/20/2017 12:06 AM, John Stultz wrote: > On Tue, Oct 17, 2017 at 1:41 AM, Minas Harutyunyan > <Minas.Harutyunyan@synopsys.com> wrote: >> On 10/17/2017 1:34 AM, John Stultz wrote: >>> On Mon, Oct 16, 2017 at 1:36 AM, Minas Harutyunyan >>> <Minas.Harutyunyan@synopsys.com> wrote: >>>> On b-plug disconnect should asserted GOTGINT.SesEndDet interrupt. >>>> According previously sent by you register dump (GHWCFG2 = 0x23affc70) >>>> your core OTG_MODE=0. >>>> Bellow fragment from programming guide on Device disconnect: >>>> >>>> "7.3Device Disconnection >>>> The device session ends when the USB cable is disconnected or if the >>>> VBUS is switched off by the Host. The >>>> device disconnect flow varies depending on the value of the OTG_MODE >>>> configuration parameter. >>>> >>>> When OTG_MODE = 0,1, or 3 >>>> When OTG_MODE is set to 0,1, or 3, the device disconnect flow is as follows: >>>> 1. When the USB cable is unplugged or when the VBUS is switched off by >>>> the Host, the Device core >>>> trigger GINTSTS.OTGInt [bit 2] interrupt bit. >>>> 2. When the device application detects GINTSTS.OTGInt interrupt, it >>>> checks that the >>>> GOTGINT.SesEndDet (Session End Detected) bit is set to 1’b1." >>>> >>>> So, you should receive and handle "Session End Detected". In function >>>> dwc2_handle_otg_intr() on this interrupt (in device mode) calling >>>> dwc2_hsotg_disconnect() function. By adding your patch "[PATCH 3/3] usb: >>>> dwc2: Fix UDC state tracking" state changed to not attached as required. >>> >>> >>> So, on the HiKey board (using 4.14-rc5 + Vardan's patch), I'm not >>> seeing the GOTGINT_SES_END_DET in dwc2_handle_otg_intr() when I remove >>> the USB OTG cable. >>> >>> In fact, I'm not seeing any calls to dwc2_handle_otg_intr()... which >>> seems... odd maybe? Any clues as to what might be going wrong then? >>> >>> thanks >>> -john >>> >> Hi John Stultz, >> So, on Hikey board on unplug B connector GOTGINT.SesEndDet interrupt not >> asserted, instead asserted GINTSTS_CONIDSTSCHNG. Please, confirm. > > Correct. On B unplug, I see: > dwc2_handle_conn_id_status_change_intr: ++Connector ID Status Change > Interrupt++ (Host) I can't understand how on B unplug connector id status changed to host! More probable it's board design issue. Please investigate hikey design. Electrically Connector ID pin status changed when plugged A connector and when unplugged A connector because of in A connector internally ConnID pin connected to ground. In B connector ConnID pin floating. > > And I never see any calls to dwc2_handle_otg_intr(). > >> In this case without your patch "[PATCH 1/3] usb: dwc2: Improve gadget >> state disconnection handling" but by applying your patch "[PATCH 3/3] >> usb: dwc2: Fix UDC state tracking": >> 1. On B plug connect UDC state will be set to "configured" >> 2. On B plug disconnect - "not attached". >> Is it Ok for you? > On our setup I tested your patch 3/3 without patches 1/3 and 2/3 and its changed state correctly. > So this is what I expect, but I don't see it. Since without my patch, > nothing seems to call disconenct when the B plug is disconnected, I > still see: > # cat /sys/class/udc/f72c0000.usb/state > configured > > thanks > -john > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-10-24 9:47 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-20 19:57 [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Stultz 2017-09-20 19:57 ` [RESEND x2][PATCH 1/3] usb: dwc2: Improve gadget state disconnection handling John Stultz 2017-09-20 19:57 ` [RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode John Stultz 2017-10-03 10:02 ` Minas Harutyunyan 2017-09-20 19:57 ` [RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking John Stultz 2017-10-03 10:02 ` Minas Harutyunyan 2017-09-30 17:13 ` [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey John Youn 2017-10-03 9:58 ` Minas Harutyunyan 2017-10-09 21:50 ` John Stultz 2017-10-12 7:59 ` Minas Harutyunyan 2017-10-12 18:06 ` John Stultz 2017-10-16 8:36 ` Minas Harutyunyan 2017-10-16 21:34 ` John Stultz 2017-10-17 8:41 ` Minas Harutyunyan 2017-10-19 6:46 ` Minas Harutyunyan 2017-10-19 20:20 ` John Stultz 2017-10-20 11:48 ` Minas Harutyunyan 2017-10-23 9:19 ` Minas Harutyunyan 2017-10-23 20:41 ` John Stultz 2017-10-23 21:36 ` John Stultz 2017-10-24 9:47 ` Minas Harutyunyan 2017-10-19 20:06 ` John Stultz 2017-10-20 11:26 ` 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.