usb: dwc2: gadget: Consider VBUS discharge time in disconnect flow

usb: dwc2: gadget: Consider VBUS discharge time in disconnect flow

[John Keeping]

On Tue, 5 Feb 2019 12:08:48 +0400
Minas Harutyunyan <minas.harutyunyan@synopsys.com> wrote:

> If VBUS discharge time > 3ms then on device cable disconnect, core
> asserted Early Suspend then Suspend interrupts which no need to
> handle.
> 
> VBUS discharge time depend on PHY schematic implementation.
> Can be up to 1 sec.
> 
> To unambiguous recognize disconnect event on Early_Suspend interrupt
> added delay to allow VBUS fully discharge and as result
> GOTGCTL_BSESVLD goes to 0. If GOTGCTL_BSESVLD after delay still set
> to 1 that mean Suspend initiated by host, not by disconnect.
> In case of disconnect after Early_Suspend core asserting Suspend,
> Reset Detect and Reset interrupts which no any meaning, so these
> interrupts no need to handle and resetting in GINTSTS.

I have just tried this and it makes enumeration much slower.  The host
system shows:

[1495332.243080] usb 1-2.4.7: new high-speed USB device number 89 using xhci_hcd
[1495337.399713] usb 1-2.4.7: device descriptor read/64, error -110
[1495347.779878] xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command
[1495348.004513] usb 1-2.4.7: New USB device found, idVendor=0001, idProduct=0001, bcdDevice= 1.01

My guess is that the added delay is triggering in cases when it
shouldn't and delaying the response to the setup device command.

usb: dwc2: gadget: Consider VBUS discharge time in disconnect flow

[Felipe Balbi]

Hi,

Minas Harutyunyan <minas.harutyunyan@synopsys.com> writes:
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 68ad75a7460d..eab1da4c0beb 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3689,6 +3689,14 @@ static irqreturn_t dwc2_hsotg_irq(int irq, void *pw)
>  	if (gintsts & GINTSTS_ERLYSUSP) {
>  		dev_dbg(hsotg->dev, "GINTSTS_ErlySusp\n");
>  		dwc2_writel(hsotg, GINTSTS_ERLYSUSP, GINTSTS);
> +
> +		mdelay(hsotg->params.vbus_discharge_time);

<snip>

> +	p->vbus_discharge_time = 1000;

A whole second?? Wow. Perhaps you need to rethink this patch a little.

usb: dwc2: gadget: Consider VBUS discharge time in disconnect flow

[Minas Harutyunyan]

If VBUS discharge time > 3ms then on device cable disconnect, core
asserted Early Suspend then Suspend interrupts which no need to handle.

VBUS discharge time depend on PHY schematic implementation.
Can be up to 1 sec.

To unambiguous recognize disconnect event on Early_Suspend interrupt
added delay to allow VBUS fully discharge and as result GOTGCTL_BSESVLD
goes to 0. If GOTGCTL_BSESVLD after delay still set to 1 that mean
Suspend initiated by host, not by disconnect.
In case of disconnect after Early_Suspend core asserting Suspend, Reset
Detect and Reset interrupts which no any meaning, so these interrupts
no need to handle and resetting in GINTSTS.

Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>
---
 drivers/usb/dwc2/core.h    | 4 ++++
 drivers/usb/dwc2/debugfs.c | 2 ++
 drivers/usb/dwc2/gadget.c  | 8 ++++++++
 drivers/usb/dwc2/params.c  | 1 +
 4 files changed, 15 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 30bab8463c96..4a4a196ef214 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -433,6 +433,8 @@ enum dwc2_ep0_state {
  * @service_interval:   Enable service interval based scheduling.
  *                      0 - No
  *                      1 - Yes
+ * @vbus_discharge_time: VBUS discharge time (msec) depend on PHY
+ *                       implementation and can be up to 1 sec.
  *
  * The following parameters may be specified when starting the module. These
  * parameters define how the DWC_otg controller should be configured. A
@@ -511,6 +513,8 @@ struct dwc2_core_params {
 	u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
 
 	bool change_speed_quirk;
+
+	u32 vbus_discharge_time;
 };
 
 /**
diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
index 7f62f4cdc265..a0c62511b481 100644
--- a/drivers/usb/dwc2/debugfs.c
+++ b/drivers/usb/dwc2/debugfs.c
@@ -707,6 +707,8 @@ static int params_show(struct seq_file *seq, void *v)
 	print_param(seq, p, g_dma_desc);
 	print_param(seq, p, g_rx_fifo_size);
 	print_param(seq, p, g_np_tx_fifo_size);
+	print_param(seq, p, vbus_discharge_time);
+
 
 	for (i = 0; i < MAX_EPS_CHANNELS; i++) {
 		char str[32];
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 68ad75a7460d..eab1da4c0beb 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3689,6 +3689,14 @@ static irqreturn_t dwc2_hsotg_irq(int irq, void *pw)
 	if (gintsts & GINTSTS_ERLYSUSP) {
 		dev_dbg(hsotg->dev, "GINTSTS_ErlySusp\n");
 		dwc2_writel(hsotg, GINTSTS_ERLYSUSP, GINTSTS);
+
+		mdelay(hsotg->params.vbus_discharge_time);
+		if (!(dwc2_readl(hsotg, GOTGCTL) & GOTGCTL_BSESVLD)) {
+			/* Clear Suspend, Reset Detect and Reset interrupts */
+			dwc2_writel(hsotg, GINTSTS_USBSUSP | GINTSTS_RESETDET |
+				    GINTSTS_USBRST, GINTSTS);
+			dwc2_hsotg_disconnect(hsotg);
+		}
 	}
 
 	/*
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 24ff5f21cb25..c1912627a032 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -315,6 +315,7 @@ static void dwc2_set_default_params(struct dwc2_hsotg *hsotg)
 	p->ahbcfg = GAHBCFG_HBSTLEN_INCR << GAHBCFG_HBSTLEN_SHIFT;
 	p->ref_clk_per = 33333;
 	p->sof_cnt_wkup_alert = 100;
+	p->vbus_discharge_time = 1000;
 
 	if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
 	    (hsotg->dr_mode == USB_DR_MODE_OTG)) {