Re: [PATCH v4] fsl/usb: Workarourd for USB erratum-A005697

[Alan Stern]

On Tue, 29 Nov 2016, Changming Huang wrote:

> The EHCI specification states the following in the SUSP bit description:
> In the Suspend state, the port is sensitive to resume detection.
> Note that the bit status does not change until the port is suspended and
> that there may be a delay in suspending a port if there is a transaction
> currently in progress on the USB.
> 
> However, in NXP USBDR controller, the PORTSCx[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 make sure this port has entered
> suspended state before initiating this port resume using the Force Port
> Resume bit. This bit is for NXP controller, not EHCI compatible.
> 
> Signed-off-by: Changming Huang 
> Signed-off-by: Ramneek Mehresh 
> ---
> Changes in v4:
>   - release spinlock before sleeping
> Changes in v3:
>   - add 10ms delay in function ehci_hub_control
>   - fix typos
> Changes in v2:
>   - move sleep out of spin-lock
>   - add more comment for this workaround

Acked-by: Alan Stern 

Re: [PATCH v4] fsl/usb: Workarourd for USB erratum-A005697

[Alan Stern]

On Tue, 29 Nov 2016, Changming Huang wrote:

> The EHCI specification states the following in the SUSP bit description:
> In the Suspend state, the port is sensitive to resume detection.
> Note that the bit status does not change until the port is suspended and
> that there may be a delay in suspending a port if there is a transaction
> currently in progress on the USB.
> 
> However, in NXP USBDR controller, the PORTSCx[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 make sure this port has entered
> suspended state before initiating this port resume using the Force Port
> Resume bit. This bit is for NXP controller, not EHCI compatible.
> 
> Signed-off-by: Changming Huang 
> Signed-off-by: Ramneek Mehresh 
> ---
> Changes in v4:
>   - release spinlock before sleeping
> Changes in v3:
>   - add 10ms delay in function ehci_hub_control
>   - fix typos
> Changes in v2:
>   - move sleep out of spin-lock
>   - add more comment for this workaround

Acked-by: Alan Stern 

[PATCH v4] fsl/usb: Workarourd for USB erratum-A005697

[Changming Huang]

The EHCI specification states the following in the SUSP bit description:
In the Suspend state, the port is sensitive to resume detection.
Note that the bit status does not change until the port is suspended and
that there may be a delay in suspending a port if there is a transaction
currently in progress on the USB.

However, in NXP USBDR controller, the PORTSCx[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 make sure this port has entered
suspended state before initiating this port resume using the Force Port
Resume bit. This bit is for NXP controller, not EHCI compatible.

Signed-off-by: Changming Huang 
Signed-off-by: Ramneek Mehresh 
---
Changes in v4:
  - release spinlock before sleeping
Changes in v3:
  - add 10ms delay in function ehci_hub_control
  - fix typos
Changes in v2:
  - move sleep out of spin-lock
  - add more comment for this workaround

 drivers/usb/host/ehci-fsl.c  |3 +++
 drivers/usb/host/ehci-hub.c  |   14 ++
 drivers/usb/host/ehci.h  |8 
 drivers/usb/host/fsl-mph-dr-of.c |2 ++
 include/linux/fsl_devices.h  |1 +
 5 files changed, 28 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..df169c8 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -310,6 +310,14 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
}
spin_unlock_irq(>lock);
 
+   if (changed && ehci_has_fsl_susp_errata(ehci))
+   /*
+* Wait for at least 10 millisecondes to ensure the controller
+* enter the suspend status before initiating a port resume
+* using the Force Port Resume bit (Not-EHCI compatible).
+*/
+   usleep_range(1, 2);
+
if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
/*
 * Wait for HCD to enter low-power mode or for the bus
@@ -1200,6 +1208,12 @@ int ehci_hub_control(
wIndex, (temp1 & HOSTPC_PHCD) ?
"succeeded" : "failed");
}
+   if (ehci_has_fsl_susp_errata(ehci)) {
+   /* 10ms for HCD enter suspend */
+   spin_unlock_irqrestore(>lock, flags);
+   usleep_range(1, 2);
+   spin_lock_irqsave(>lock, flags);
+   }
set_bit(wIndex, >suspended_ports);
break;
case USB_PORT_FEAT_POWER:
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 3f3b74a..a8e3617 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -219,6 +219,7 @@ struct ehci_hcd {   /* one per controller */
unsignedno_selective_suspend:1;
unsignedhas_fsl_port_bug:1; /* FreeScale */
unsignedhas_fsl_hs_errata:1;/* Freescale HS quirk */
+   unsignedhas_fsl_susp_errata:1;  /* NXP SUSP quirk */
unsignedbig_endian_mmio:1;
unsignedbig_endian_desc:1;
unsignedbig_endian_capbase:1;
@@ -710,6 +711,13 @@ struct ehci_tt {
 #endif
 
 /*
+ * Some Freescale/NXP processors have an erratum (USB A-005697)
+ * in which we need to wait for 10ms for bus to enter suspend mode
+ * after setting SUSP bit.
+ */
+#define ehci_has_fsl_susp_errata(e)((e)->has_fsl_susp_errata)
+
+/*
  * While most USB host controllers implement their registers in
  * little-endian format, a minority (celleb companion chip) implement
  * them in big endian format.
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 =
   

[PATCH v4] fsl/usb: Workarourd for USB erratum-A005697

[Changming Huang]

The EHCI specification states the following in the SUSP bit description:
In the Suspend state, the port is sensitive to resume detection.
Note that the bit status does not change until the port is suspended and
that there may be a delay in suspending a port if there is a transaction
currently in progress on the USB.

However, in NXP USBDR controller, the PORTSCx[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 make sure this port has entered
suspended state before initiating this port resume using the Force Port
Resume bit. This bit is for NXP controller, not EHCI compatible.

Signed-off-by: Changming Huang 
Signed-off-by: Ramneek Mehresh 
---
Changes in v4:
  - release spinlock before sleeping
Changes in v3:
  - add 10ms delay in function ehci_hub_control
  - fix typos
Changes in v2:
  - move sleep out of spin-lock
  - add more comment for this workaround

 drivers/usb/host/ehci-fsl.c  |3 +++
 drivers/usb/host/ehci-hub.c  |   14 ++
 drivers/usb/host/ehci.h  |8 
 drivers/usb/host/fsl-mph-dr-of.c |2 ++
 include/linux/fsl_devices.h  |1 +
 5 files changed, 28 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..df169c8 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -310,6 +310,14 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
}
spin_unlock_irq(>lock);
 
+   if (changed && ehci_has_fsl_susp_errata(ehci))
+   /*
+* Wait for at least 10 millisecondes to ensure the controller
+* enter the suspend status before initiating a port resume
+* using the Force Port Resume bit (Not-EHCI compatible).
+*/
+   usleep_range(1, 2);
+
if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
/*
 * Wait for HCD to enter low-power mode or for the bus
@@ -1200,6 +1208,12 @@ int ehci_hub_control(
wIndex, (temp1 & HOSTPC_PHCD) ?
"succeeded" : "failed");
}
+   if (ehci_has_fsl_susp_errata(ehci)) {
+   /* 10ms for HCD enter suspend */
+   spin_unlock_irqrestore(>lock, flags);
+   usleep_range(1, 2);
+   spin_lock_irqsave(>lock, flags);
+   }
set_bit(wIndex, >suspended_ports);
break;
case USB_PORT_FEAT_POWER:
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 3f3b74a..a8e3617 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -219,6 +219,7 @@ struct ehci_hcd {   /* one per controller */
unsignedno_selective_suspend:1;
unsignedhas_fsl_port_bug:1; /* FreeScale */
unsignedhas_fsl_hs_errata:1;/* Freescale HS quirk */
+   unsignedhas_fsl_susp_errata:1;  /* NXP SUSP quirk */
unsignedbig_endian_mmio:1;
unsignedbig_endian_desc:1;
unsignedbig_endian_capbase:1;
@@ -710,6 +711,13 @@ struct ehci_tt {
 #endif
 
 /*
+ * Some Freescale/NXP processors have an erratum (USB A-005697)
+ * in which we need to wait for 10ms for bus to enter suspend mode
+ * after setting SUSP bit.
+ */
+#define ehci_has_fsl_susp_errata(e)((e)->has_fsl_susp_errata)
+
+/*
  * While most USB host controllers implement their registers in
  * little-endian format, a minority (celleb companion chip) implement
  * them in big endian format.
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,