On Mon, 27 Jul 2015, Nikhil Badola wrote:
> As per USB specification, in the Suspend state, the status bit does
> not change until the port is suspended. However, there may be a delay
> in suspending a port if there is a transaction currently in progress
> on the bus.
>
> In the USBDR controller, the PORTSCx[SUSP] bit changes immediately when
> the application sets it and not when the port is actually suspended
>
> Workaround for this issue involves waiting for a minimum of 10ms to
> allow the controller to go into SUSPEND state before proceeding ahead
Why is this delay needed? When ehci_bus_suspend() is called there are
never any transactions in progress, except possibly for a SOF
transaction (which takes less than 1 us, nowhere near 10 ms).
> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@xxxxxxxxxxxxx>
> Signed-off-by: Nikhil Badola <nikhil.badola@xxxxxxxxxxxxx>
> ---
> drivers/usb/host/ehci-fsl.c | 3 +++
> drivers/usb/host/ehci-hub.c | 2 ++
> drivers/usb/host/ehci.h | 12 ++++++++++++
> drivers/usb/host/fsl-mph-dr-of.c | 5 +++++
> include/linux/fsl_devices.h | 1 +
> 5 files changed, 23 insertions(+)
>
> diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
> index 202dafb..8904aae 100644
> --- a/drivers/usb/host/ehci-fsl.c
> +++ b/drivers/usb/host/ehci-fsl.c
> @@ -278,6 +278,9 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci)
> out_be32(non_ehci + FSL_SOC_USB_SNOOP2, 0x80000000 | SNOOP_SIZE_2GB);
> }
>
> + if (pdata->has_fsl_erratum_a005697 == 1)
> + ehci->has_fsl_susp_errata = 1;
> +
> if ((pdata->operating_mode == FSL_USB2_DR_HOST) ||
> (pdata->operating_mode == FSL_USB2_DR_OTG))
> if (ehci_fsl_setup_phy(hcd, pdata->phy_mode, 0))
> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> index 22abb68..7eac923 100644
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -303,6 +303,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
> USB_PORT_STAT_HIGH_SPEED)
> fs_idle_delay = true;
> ehci_writel(ehci, t2, reg);
> + if (ehci_has_fsl_susp_errata(ehci))
> + usleep_range(10000, 20000);
> changed = 1;
> }
> }
This is wrong for two reasons. First, you must not sleep while holding
a spinlock. Second, your sleep is in the wrong place. It should occur
later, at this point:
if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
/*
* Wait for HCD to enter low-power mode or for the bus
* to switch to full-speed idle.
*/
usleep_range(5000, 5500);
}
You can check your quirk flag here and increase the length of this
sleep to 10 ms.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
