Re: [PATCH v3] fsl/usb: Workarourd for USB erratum-A005697

[Alan Stern]

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.huang@xxxxxxx>
> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@xxxxxxx>
> ---
> 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