* [PATCH] fsl/usb: Workarourd for USB erratum-A005697
@ 2016-11-24 9:18 Changming Huang
2016-11-24 11:17 ` Sriram Dash
0 siblings, 1 reply; 5+ messages in thread
From: Changming Huang @ 2016-11-24 9:18 UTC (permalink / raw)
To: stern, gregkh
Cc: sriram.dash, linux-usb, linux-kernel, julia.lawall,
Changming Huang, Ramneek Mehresh
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);
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
2016-11-24 9:18 [PATCH] fsl/usb: Workarourd for USB erratum-A005697 Changming Huang
@ 2016-11-24 11:17 ` Sriram Dash
2016-11-24 16:53 ` Alan Stern
2016-11-25 2:53 ` Jerry Huang
0 siblings, 2 replies; 5+ messages in thread
From: Sriram Dash @ 2016-11-24 11:17 UTC (permalink / raw)
To: Jerry Huang, stern, gregkh
Cc: linux-usb, linux-kernel, julia.lawall, Jerry Huang,
Ramneek Mehresh, Suresh Gupta
>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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
2016-11-24 11:17 ` Sriram Dash
@ 2016-11-24 16:53 ` Alan Stern
2016-11-25 2:52 ` Jerry Huang
2016-11-25 2:53 ` Jerry Huang
1 sibling, 1 reply; 5+ messages in thread
From: Alan Stern @ 2016-11-24 16:53 UTC (permalink / raw)
To: Sriram Dash
Cc: Jerry Huang, gregkh, linux-usb, linux-kernel, julia.lawall,
Ramneek Mehresh, Suresh Gupta
On Thu, 24 Nov 2016, Sriram Dash wrote:
> >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
The USB core guarantees that there won't be any data transactions in
progress when a root hub is suspended. There might possibly be an SOF
transaction, but that doesn't take more than a few microseconds at
most. Certainly nowhere near 10 ms!
Given that we already perform a 150-us delay, is this change really
needed?
Alan Stern
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
2016-11-24 16:53 ` Alan Stern
@ 2016-11-25 2:52 ` Jerry Huang
0 siblings, 0 replies; 5+ messages in thread
From: Jerry Huang @ 2016-11-25 2:52 UTC (permalink / raw)
To: Alan Stern, Sriram Dash
Cc: gregkh, linux-usb, linux-kernel, julia.lawall, Ramneek Mehresh,
Suresh Gupta
Hi, Alan,
Maybe other USB controller does not need this delay.
However, our silicon errata point out, in the USBDR controller, the PORTCx[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 ensure the port has entered SUSPEND status before initiating this port resume using the Force Port Resume (which is one bit for NXP silicon, not-EHCI compatible).
I will add more comment to understand why we need this delay in next version.
Best Regards
Jerry Huang
-----Original Message-----
From: Alan Stern [mailto:stern@rowland.harvard.edu]
Sent: Friday, November 25, 2016 12:54 AM
To: Sriram Dash <sriram.dash@nxp.com>
Cc: Jerry Huang <jerry.huang@nxp.com>; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; 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
On Thu, 24 Nov 2016, Sriram Dash wrote:
> >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
The USB core guarantees that there won't be any data transactions in progress when a root hub is suspended. There might possibly be an SOF transaction, but that doesn't take more than a few microseconds at most. Certainly nowhere near 10 ms!
Given that we already perform a 150-us delay, is this change really needed?
Alan Stern
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
2016-11-24 11:17 ` Sriram Dash
2016-11-24 16:53 ` Alan Stern
@ 2016-11-25 2:53 ` Jerry Huang
1 sibling, 0 replies; 5+ messages in thread
From: Jerry Huang @ 2016-11-25 2:53 UTC (permalink / raw)
To: Sriram Dash, stern, gregkh
Cc: linux-usb, linux-kernel, julia.lawall, Ramneek Mehresh, Suresh Gupta
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-25 13:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-24 9:18 [PATCH] fsl/usb: Workarourd for USB erratum-A005697 Changming Huang
2016-11-24 11:17 ` Sriram Dash
2016-11-24 16:53 ` Alan Stern
2016-11-25 2:52 ` Jerry Huang
2016-11-25 2:53 ` Jerry Huang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).