| Message ID | 3df661ea3e29360def73170ca3a55e7c7fb41ad2.1549353978.git.hminas@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: dwc2: gadget: Consider VBUS discharge time in disconnect flow | expand |
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.
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.
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)) {
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(+)