From: Jerry Huang <jerry.huang@nxp.com>
To: Sriram Dash <sriram.dash@nxp.com>,
        "stern@rowland.harvard.edu" <stern@rowland.harvard.edu>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
CC: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "julia.lawall@lip6.fr" <julia.lawall@lip6.fr>,
        Ramneek Mehresh <ramneek.mehresh@nxp.com>,
        Suresh Gupta <suresh.gupta@nxp.com>
Subject: RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
Thread-Topic: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
Thread-Index: AQHSRjXotONUOBA9ekapQ3gxU3f+DaDn/CwAgAEFbZA=
Date: Fri, 25 Nov 2016 02:53:22 +0000
Message-ID: <DB5PR0401MB18133989D0F268A754FC4110FE890@DB5PR0401MB1813.eurprd04.prod.outlook.com>
References: <1479979089-4715-1-git-send-email-jerry.huang@nxp.com>
 <DB5PR0401MB19252FA473A25F79DC864D15F5B60@DB5PR0401MB1925.eurprd04.prod.outlook.com>
In-Reply-To: <DB5PR0401MB19252FA473A25F79DC864D15F5B60@DB5PR0401MB1925.eurprd04.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Nov 2016 02:53:22.3309
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB5PR0401MB1928
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uAP8RI0E030905
Content-Length: 4800
Lines: 129

Thanks, Sriram,
It is better to move this delay out of spin-lock.

Best Regards
Jerry Huang


-----Original Message-----
From: Sriram Dash 
Sent: Thursday, November 24, 2016 7:17 PM
To: Jerry Huang <jerry.huang@nxp.com>; stern@rowland.harvard.edu; gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; julia.lawall@lip6.fr; Jerry Huang <jerry.huang@nxp.com>; Ramneek Mehresh <ramneek.mehresh@nxp.com>; Suresh Gupta <suresh.gupta@nxp.com>
Subject: RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697

>From: Changming Huang [mailto:jerry.huang@nxp.com] 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
>
>Signed-off-by: Changming Huang <jerry.huang@nxp.com>
>Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
>---
> drivers/usb/host/ehci-fsl.c      |    3 +++
> drivers/usb/host/ehci-hub.c      |    2 ++
> drivers/usb/host/ehci.h          |    6 ++++++
> drivers/usb/host/fsl-mph-dr-of.c |    2 ++
> include/linux/fsl_devices.h      |    1 +
> 5 files changed, 14 insertions(+)
>
>diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c 
>index 9f5ffb6..91701cc 100644
>--- a/drivers/usb/host/ehci-fsl.c
>+++ b/drivers/usb/host/ehci-fsl.c
>@@ -286,6 +286,9 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci)
> 	if (pdata->has_fsl_erratum_a005275 == 1)
> 		ehci->has_fsl_hs_errata = 1;
>
>+	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 
>74f62d6..86d154e 100644
>--- a/drivers/usb/host/ehci-hub.c
>+++ b/drivers/usb/host/ehci-hub.c
>@@ -305,6 +305,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))
>+				mdelay(10);

Hi Jerry,

Move the delay out of the spin lock. Other than that, it looks fine to me.

> 			changed = 1;
> 		}
> 	}
>diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index 
>3f3b74a..7706e4a 100644
>--- a/drivers/usb/host/ehci.h
>+++ b/drivers/usb/host/ehci.h
>@@ -219,6 +219,7 @@ struct ehci_hcd {			/* one per controller */
> 	unsigned		no_selective_suspend:1;
> 	unsigned		has_fsl_port_bug:1; /* FreeScale */
> 	unsigned		has_fsl_hs_errata:1;	/* Freescale HS quirk */
>+	unsigned		has_fsl_susp_errata:1;	/*Freescale SUSP quirk*/
> 	unsigned		big_endian_mmio:1;
> 	unsigned		big_endian_desc:1;
> 	unsigned		big_endian_capbase:1;
>@@ -703,10 +704,15 @@ struct ehci_tt {
> #if defined(CONFIG_PPC_85xx)
> /* Some Freescale processors have an erratum (USB A-005275) in which
>  * incoming packets get corrupted in HS mode
>+ * Some Freescale processors have an erratum (USB A-005697) in which
>+ * we need to wait for 10ms for bus to fo into suspend mode after
>+ * setting SUSP bit
>  */
> #define ehci_has_fsl_hs_errata(e)	((e)->has_fsl_hs_errata)
>+#define ehci_has_fsl_susp_errata(e)	((e)->has_fsl_susp_errata)
> #else
> #define ehci_has_fsl_hs_errata(e)	(0)
>+#define ehci_has_fsl_susp_errata(e)	(0)
> #endif
>
> /*
>diff --git a/drivers/usb/host/fsl-mph-dr-of.c 
>b/drivers/usb/host/fsl-mph-dr-of.c
>index f07ccb2..e90ddb5 100644
>--- a/drivers/usb/host/fsl-mph-dr-of.c
>+++ b/drivers/usb/host/fsl-mph-dr-of.c
>@@ -226,6 +226,8 @@ static int fsl_usb2_mph_dr_of_probe(struct 
>platform_device *ofdev)
> 		of_property_read_bool(np, "fsl,usb-erratum-a007792");
> 	pdata->has_fsl_erratum_a005275 =
> 		of_property_read_bool(np, "fsl,usb-erratum-a005275");
>+	pdata->has_fsl_erratum_a005697 =
>+		of_property_read_bool(np, "fsl,usb_erratum-a005697");
>
> 	/*
> 	 * Determine whether phy_clk_valid needs to be checked diff --git 
>a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h index 
>f291291..60cef82
>100644
>--- a/include/linux/fsl_devices.h
>+++ b/include/linux/fsl_devices.h
>@@ -100,6 +100,7 @@ struct fsl_usb2_platform_data {
> 	unsigned	already_suspended:1;
> 	unsigned        has_fsl_erratum_a007792:1;
> 	unsigned        has_fsl_erratum_a005275:1;
>+	unsigned	has_fsl_erratum_a005697:1;
> 	unsigned        check_phy_clk_valid:1;
>
> 	/* register save area for suspend/resume */
>--
>1.7.9.5


Regards,
Sriram