Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932875AbcK1P5w (ORCPT ); Mon, 28 Nov 2016 10:57:52 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:54034 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S932373AbcK1P5m (ORCPT ); Mon, 28 Nov 2016 10:57:42 -0500 Date: Mon, 28 Nov 2016 10:57:41 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Changming Huang cc: gregkh@linuxfoundation.org, , , , , Subject: Re: [PATCH v3] fsl/usb: Workarourd for USB erratum-A005697 In-Reply-To: <1480320310-9511-1-git-send-email-jerry.huang@nxp.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2164 Lines: 57 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 > Signed-off-by: Ramneek Mehresh > --- > 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