Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753935AbbG0Ol4 (ORCPT ); Mon, 27 Jul 2015 10:41:56 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:55363 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750931AbbG0Oly (ORCPT ); Mon, 27 Jul 2015 10:41:54 -0400 Date: Mon, 27 Jul 2015 10:41:53 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Nikhil Badola cc: linux-kernel@vger.kernel.org, , , Ramneek Mehresh Subject: Re: [PATCH] fsl/usb: Workaround for USB erratum-A005697 In-Reply-To: <1437997264-31219-1-git-send-email-nikhil.badola@freescale.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: 2910 Lines: 77 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 > Signed-off-by: Nikhil Badola > --- > 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/