Thanks a lot, Alan. That is my fault, will fix it and send one new version.
Best Regards Jerry Huang -----Original Message----- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Monday, November 28, 2016 11:58 PM To: Jerry Huang <jerry.hu...@nxp.com> Cc: gre...@linuxfoundation.org; Ramneek Mehresh <ramneek.mehr...@nxp.com>; julia.law...@lip6.fr; Sriram Dash <sriram.d...@nxp.com>; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] fsl/usb: Workarourd for USB erratum-A005697 On Mon, 28 Nov 2016, Changming Huang wrote: > The EHCI specification states the following in the SUSP bit description: > In the Suspend state, the port is sensitive to resume detection. > Note that the bit status does not change until the port is suspended > and that there may be a delay in suspending a port if there is a > transaction currently in progress on the USB. > > However, in NXP USBDR controller, the PORTSCx[SUSP] bit changes > immediately when the application sets it and not when the port is actually > suspended. > > So the application must wait for at least 10 milliseconds after a port > indicates that it is suspended, to make sure this port has entered > suspended state before initiating this port resume using the Force > Port Resume bit. This bit is for NXP controller, not EHCI compatible. > > Signed-off-by: Changming Huang <jerry.hu...@nxp.com> > Signed-off-by: Ramneek Mehresh <ramneek.mehr...@nxp.com> > --- > Changes in v3: > - add 10ms delay in function ehci_hub_control > - fix typos > Changes in v2: > - move sleep out of spin-lock > - add more comment for this workaround > --- a/drivers/usb/host/ehci-hub.c > +++ b/drivers/usb/host/ehci-hub.c > @@ -310,6 +310,14 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) > } > spin_unlock_irq(&ehci->lock); > > + if (changed && ehci_has_fsl_susp_errata(ehci)) > + /* > + * Wait for at least 10 millisecondes to ensure the controller > + * enter the suspend status before initiating a port resume > + * using the Force Port Resume bit (Not-EHCI compatible). > + */ > + usleep_range(10000, 20000); > + > if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) { > /* > * Wait for HCD to enter low-power mode or for the bus @@ > -1200,6 > +1208,9 @@ int ehci_hub_control( > wIndex, (temp1 & HOSTPC_PHCD) ? > "succeeded" : "failed"); > } > + if (ehci_has_fsl_susp_errata(ehci)) > + /* 10ms for HCD enter suspend */ > + usleep_range(10000, 20000); > set_bit(wIndex, &ehci->suspended_ports); > break; Just like before, you have to release the spinlock before sleeping. Look at the code 10 lines above this. Alan Stern
