[RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Stultz]

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

[RESEND x2][PATCH 1/3] usb: dwc2: Improve gadget state disconnection handling

[John Stultz]

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

[RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode

[John Stultz]

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

[RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking

[John Stultz]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Youn]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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

Re: [RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode

[Minas Harutyunyan]

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>

Re: [RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking

[Minas Harutyunyan]

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>

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Stultz]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Stultz]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Stultz]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Stultz]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Stultz]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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
> 

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Stultz]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[John Stultz]

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

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

[Minas Harutyunyan]

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