Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1529183 > unrolled thread
| Started by | Sriram Dash <sriram.dash@nxp.com> |
|---|---|
| First post | 2016-11-24 12:20 +0100 |
| Last post | 2016-11-25 09:30 +0100 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697 Sriram Dash <sriram.dash@nxp.com> - 2016-11-24 12:20 +0100
RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697 Alan Stern <stern@rowland.harvard.edu> - 2016-11-24 18:00 +0100
RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697 Jerry Huang <jerry.huang@nxp.com> - 2016-11-25 09:30 +0100
| From | Sriram Dash <sriram.dash@nxp.com> |
|---|---|
| Date | 2016-11-24 12:20 +0100 |
| Subject | RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697 |
| Message-ID | <sH0el-1UP-9@gated-at.bofh.it> |
>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
[toc] | [next] | [standalone]
| From | Alan Stern <stern@rowland.harvard.edu> |
|---|---|
| Date | 2016-11-24 18:00 +0100 |
| Message-ID | <sH5xo-5lp-23@gated-at.bofh.it> |
| In reply to | #1529183 |
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
[toc] | [prev] | [next] | [standalone]
| From | Jerry Huang <jerry.huang@nxp.com> |
|---|---|
| Date | 2016-11-25 09:30 +0100 |
| Message-ID | <sHk3o-6BP-17@gated-at.bofh.it> |
| In reply to | #1529183 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web