On Fri, 25 Nov 2016, Changming Huang wrote:
> The EHCI specification states the following in the SUSP bit description:
> In the Suspend state, the port is senstive to resume detection.
> Note that the bit status does not change untile the port is suspended and
> that there may be a delay in susupending 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>
> ---
> Change in v2:
> - move sleep out of spin-lock and add more comment for this workaround
This patch is incomplete.
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -310,6 +310,13 @@ 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 Fore Port Resume bit (Not-EHCI compatible).
> + */
The proper style for multi-line comments is:
/*
* Wait for ...
* ...
*/
Also, "Force" is misspelled.
> + 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
The patch does not change the code around line 1190 in the original
file:
/* After above check the port must be connected.
* Set appropriate bit thus could put phy into low power
* mode if we have tdi_phy_lpm feature
*/
temp &= ~PORT_WKCONN_E;
temp |= PORT_WKDISC_E | PORT_WKOC_E;
ehci_writel(ehci, temp | PORT_SUSPEND, status_reg);
This code sets the PORT_SUSPEND bit but does not have a 10-ms delay.
You need to add a delay here.
Alan Stern
