All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/3] usb: host: Implement workaround for Erratum A-007463
@ 2017-12-19 10:16 ` yinbo.zhu
  0 siblings, 0 replies; 12+ messages in thread
From: yinbo.zhu @ 2017-12-19 10:16 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1, yinbo . zhu

From: yinbo.zhu <yinbo.zhu@nxp.com>

When a transaction error (defined in Section 4.10.2.3, "USB
Transaction Error" of the xHCI Specification) occurs on the
USB, the host controller reports this through a transfer
event with the completion code "USB Transaction Error". When
this happens, the endpoint is placed in the Halted state. In
response, software must issue a Reset Endpoint command to
transition the endpoint to the Stopped state. In order to
restart the transfer, the driver can perform either of the
following:
a) Ring the doorbell again, which restarts the transfer from
   where it stopped, or
b) Issue a Set TR (Transfer Ring) Dequeue Pointer command
   for the endpoint to start the transfer form a different
   Transfer Ring pointer Consider the following
   scenario:
1. The xHCI driver prepares a control transfer read to one
   of the device's control endpoints;
2. During the IN data stage, a transaction error occurs on
   the USB, causing a transfer event with the completion
   code "USB Transaction Error";
3. The driver issues a Reset Endpoint command;
4. The driver rings the doorbell of the control endpoint to
   resume the transfer. In this scenario, the controller
   may reverse the direction of the data stage from IN to OUT.
   Instead of sending an ACK to the endpoint to poll for read
   data, it sends a Data Packet (DP) to the endpoint. It
   fetches the data from the data stage Transfer Request
   Block (TRB) that is being resumed, even though the data
   buffer is setup to receive data and not transmit it.
NOTE
This issue occurs only if the transaction error happens
during an IN data stage. There is no issue if the transaction
error happens during an OUT data stage.

Impact: When this issue occurs, the device likely responds in
one of the following ways:
a) The device responds with a STALL because the data stage
has unexpectedly changed directions. The controller then
generates a Stall Error transfer event, to which software
must issue a Reset Endpoint command followed by a Set TR
Dequeue Pointer command pointing to a new Setup TRB to clear
the STALL condition.
b) The device does not respond to the inverted data stage and
the transaction times out. The controller generates another
USB Transaction Error transfer event, to which software
likely performs a USB Reset to the device because it is
unresponsive. It is not expected that any of these recovery
steps will cause instability in the system because this
recovery is part of a standard xHCI driver and could happen
regardless of the defect. Another possible system-level
impact is that the controller attempts to read from the
memory location pointed at by the Data Stage TRB or a Normal
TRB chained to it. associated with this TRB is intended to be
written by the controller, but the controller reads from it
instead. Normally, this does not cause a problem. However, if
the system has some type of memory protection where this
unexpected read is treated as a bus error,
a problem. However, if the system has some type of memory
it may cause the system to become unstable or to crash.

Workaround: If a USB Transaction Error occurs during the IN
data phase of a control transfer, the driver must use the
Set TR Dequeue Pointer command to either restart the data
Phase or restart the entire control transfer from the
Setup phase.

Configs Affected:
LS1021-20-22A-R1.0, LS1021-20-22A-R2.007463

Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
---
Change in v4:
		Remove the point in "yinbo.zhu"
 drivers/usb/dwc3/core.c      |    2 ++
 drivers/usb/dwc3/core.h      |    2 ++
 drivers/usb/dwc3/host.c      |    3 +++
 drivers/usb/host/xhci-plat.c |    3 +++
 drivers/usb/host/xhci-ring.c |   28 +++++++++++++++++++++++-----
 drivers/usb/host/xhci.h      |    3 ++-
 6 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0783250..6613bc0 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1055,6 +1055,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 
 	dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
 				"snps,tx_de_emphasis_quirk");
+	dwc->quirk_reverse_in_out = device_property_read_bool(dev,
+				"snps,quirk_reverse_in_out");
 	device_property_read_u8(dev, "snps,tx_de_emphasis",
 				&tx_de_emphasis);
 	device_property_read_string(dev, "snps,hsphy_interface",
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 4a4a4c9..a263fdc 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -857,6 +857,7 @@ struct dwc3_scratchpad_array {
  * 	1	- -3.5dB de-emphasis
  * 	2	- No de-emphasis
  * 	3	- Reserved
+ * @quirk_reverse_in_out: prevent tx fifo reverse the data direction
  * @imod_interval: set the interrupt moderation interval in 250ns
  *                 increments or 0 to disable.
  */
@@ -1009,6 +1010,7 @@ struct dwc3 {
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
+	unsigned                quirk_reverse_in_out:1;
 
 	u16			imod_interval;
 };
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 1a3878a..dab5f49 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -90,6 +90,9 @@ int dwc3_host_init(struct dwc3 *dwc)
 
 	memset(props, 0, sizeof(struct property_entry) * ARRAY_SIZE(props));
 
+	if (dwc->quirk_reverse_in_out)
+		props[prop_idx++].name = "quirk-reverse-in-out";
+
 	if (dwc->usb3_lpm_capable)
 		props[prop_idx++].name = "usb3-lpm-capable";
 
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 6f03830..fe71b92 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -266,6 +266,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
 		xhci->quirks |= XHCI_LPM_SUPPORT;
 
+	if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
+		xhci->quirks |= XHCI_REVERSE_IN_OUT;
+
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c629a0b..cac355a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1925,10 +1925,12 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	union xhci_trb *ep_trb, struct xhci_transfer_event *event,
 	struct xhci_virt_ep *ep, int *status)
 {
+	struct xhci_dequeue_state deq_state;
 	struct xhci_virt_device *xdev;
 	struct xhci_ep_ctx *ep_ctx;
 	struct xhci_ring *ep_ring;
 	unsigned int slot_id;
+	u32 remaining;
 	u32 trb_comp_code;
 	int ep_index;
 
@@ -1951,14 +1953,30 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	if (trb_comp_code == COMP_STALL_ERROR ||
 		xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
 						trb_comp_code)) {
-		/* Issue a reset endpoint command to clear the host side
-		 * halt, followed by a set dequeue command to move the
-		 * dequeue pointer past the TD.
-		 * The class driver clears the device side halt later.
+		/*erratum A-007463:
+		 *After transaction error, controller switches control transfer
+		 *data stage from IN to OUT direction.
 		 */
-		xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
+		remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
+		if (remaining && xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
+				trb_comp_code) &&
+				(xhci->quirks & XHCI_REVERSE_IN_OUT)) {
+			memset(&deq_state, 0, sizeof(deq_state));
+			xhci_find_new_dequeue_state(xhci, slot_id,
+				ep_index, td->urb->stream_id, td, &deq_state);
+			xhci_queue_new_dequeue_state(xhci, slot_id, ep_index,
+						&deq_state);
+			xhci_ring_cmd_db(xhci);
+		} else {
+			/* Issue a reset endpoint command to clear the host side
+			 * halt, followed by a set dequeue command to move the
+			 * dequeue pointer past the TD.
+			 * The class driver clears the device side halt later.
+			 */
+			xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
 					ep_ring->stream_id, td, ep_trb,
 					EP_HARD_RESET);
+		}
 	} else {
 		/* Update ring dequeue pointer */
 		while (ep_ring->dequeue != td->last_trb)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 96099a2..9f133a9 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1783,7 +1783,7 @@ struct xhci_hcd {
 #define XHCI_STATE_DYING	(1 << 0)
 #define XHCI_STATE_HALTED	(1 << 1)
 #define XHCI_STATE_REMOVING	(1 << 2)
-	unsigned int		quirks;
+	u64		quirks;
 #define	XHCI_LINK_TRB_QUIRK	(1 << 0)
 #define XHCI_RESET_EP_QUIRK	(1 << 1)
 #define XHCI_NEC_HOST		(1 << 2)
@@ -1819,6 +1819,7 @@ struct xhci_hcd {
 #define XHCI_SSIC_PORT_UNUSED	(1 << 22)
 #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
 #define XHCI_MISSING_CAS	(1 << 24)
+#define XHCI_REVERSE_IN_OUT     BIT(32)
 /* For controller with a broken Port Disable implementation */
 #define XHCI_BROKEN_PORT_PED	(1 << 25)
 #define XHCI_LIMIT_ENDPOINT_INTERVAL_7	(1 << 26)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [v4,1/3] usb: host: Implement workaround for Erratum A-007463
@ 2017-12-19 10:16 ` yinbo.zhu
  0 siblings, 0 replies; 12+ messages in thread
From: yinbo.zhu @ 2017-12-19 10:16 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1, yinbo . zhu

From: yinbo.zhu <yinbo.zhu@nxp.com>

When a transaction error (defined in Section 4.10.2.3, "USB
Transaction Error" of the xHCI Specification) occurs on the
USB, the host controller reports this through a transfer
event with the completion code "USB Transaction Error". When
this happens, the endpoint is placed in the Halted state. In
response, software must issue a Reset Endpoint command to
transition the endpoint to the Stopped state. In order to
restart the transfer, the driver can perform either of the
following:
a) Ring the doorbell again, which restarts the transfer from
   where it stopped, or
b) Issue a Set TR (Transfer Ring) Dequeue Pointer command
   for the endpoint to start the transfer form a different
   Transfer Ring pointer Consider the following
   scenario:
1. The xHCI driver prepares a control transfer read to one
   of the device's control endpoints;
2. During the IN data stage, a transaction error occurs on
   the USB, causing a transfer event with the completion
   code "USB Transaction Error";
3. The driver issues a Reset Endpoint command;
4. The driver rings the doorbell of the control endpoint to
   resume the transfer. In this scenario, the controller
   may reverse the direction of the data stage from IN to OUT.
   Instead of sending an ACK to the endpoint to poll for read
   data, it sends a Data Packet (DP) to the endpoint. It
   fetches the data from the data stage Transfer Request
   Block (TRB) that is being resumed, even though the data
   buffer is setup to receive data and not transmit it.
NOTE
This issue occurs only if the transaction error happens
during an IN data stage. There is no issue if the transaction
error happens during an OUT data stage.

Impact: When this issue occurs, the device likely responds in
one of the following ways:
a) The device responds with a STALL because the data stage
has unexpectedly changed directions. The controller then
generates a Stall Error transfer event, to which software
must issue a Reset Endpoint command followed by a Set TR
Dequeue Pointer command pointing to a new Setup TRB to clear
the STALL condition.
b) The device does not respond to the inverted data stage and
the transaction times out. The controller generates another
USB Transaction Error transfer event, to which software
likely performs a USB Reset to the device because it is
unresponsive. It is not expected that any of these recovery
steps will cause instability in the system because this
recovery is part of a standard xHCI driver and could happen
regardless of the defect. Another possible system-level
impact is that the controller attempts to read from the
memory location pointed at by the Data Stage TRB or a Normal
TRB chained to it. associated with this TRB is intended to be
written by the controller, but the controller reads from it
instead. Normally, this does not cause a problem. However, if
the system has some type of memory protection where this
unexpected read is treated as a bus error,
a problem. However, if the system has some type of memory
it may cause the system to become unstable or to crash.

Workaround: If a USB Transaction Error occurs during the IN
data phase of a control transfer, the driver must use the
Set TR Dequeue Pointer command to either restart the data
Phase or restart the entire control transfer from the
Setup phase.

Configs Affected:
LS1021-20-22A-R1.0, LS1021-20-22A-R2.007463

Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
---
Change in v4:
		Remove the point in "yinbo.zhu"
 drivers/usb/dwc3/core.c      |    2 ++
 drivers/usb/dwc3/core.h      |    2 ++
 drivers/usb/dwc3/host.c      |    3 +++
 drivers/usb/host/xhci-plat.c |    3 +++
 drivers/usb/host/xhci-ring.c |   28 +++++++++++++++++++++++-----
 drivers/usb/host/xhci.h      |    3 ++-
 6 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0783250..6613bc0 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1055,6 +1055,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 
 	dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
 				"snps,tx_de_emphasis_quirk");
+	dwc->quirk_reverse_in_out = device_property_read_bool(dev,
+				"snps,quirk_reverse_in_out");
 	device_property_read_u8(dev, "snps,tx_de_emphasis",
 				&tx_de_emphasis);
 	device_property_read_string(dev, "snps,hsphy_interface",
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 4a4a4c9..a263fdc 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -857,6 +857,7 @@ struct dwc3_scratchpad_array {
  * 	1	- -3.5dB de-emphasis
  * 	2	- No de-emphasis
  * 	3	- Reserved
+ * @quirk_reverse_in_out: prevent tx fifo reverse the data direction
  * @imod_interval: set the interrupt moderation interval in 250ns
  *                 increments or 0 to disable.
  */
@@ -1009,6 +1010,7 @@ struct dwc3 {
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
+	unsigned                quirk_reverse_in_out:1;
 
 	u16			imod_interval;
 };
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 1a3878a..dab5f49 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -90,6 +90,9 @@ int dwc3_host_init(struct dwc3 *dwc)
 
 	memset(props, 0, sizeof(struct property_entry) * ARRAY_SIZE(props));
 
+	if (dwc->quirk_reverse_in_out)
+		props[prop_idx++].name = "quirk-reverse-in-out";
+
 	if (dwc->usb3_lpm_capable)
 		props[prop_idx++].name = "usb3-lpm-capable";
 
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 6f03830..fe71b92 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -266,6 +266,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
 		xhci->quirks |= XHCI_LPM_SUPPORT;
 
+	if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
+		xhci->quirks |= XHCI_REVERSE_IN_OUT;
+
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c629a0b..cac355a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1925,10 +1925,12 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	union xhci_trb *ep_trb, struct xhci_transfer_event *event,
 	struct xhci_virt_ep *ep, int *status)
 {
+	struct xhci_dequeue_state deq_state;
 	struct xhci_virt_device *xdev;
 	struct xhci_ep_ctx *ep_ctx;
 	struct xhci_ring *ep_ring;
 	unsigned int slot_id;
+	u32 remaining;
 	u32 trb_comp_code;
 	int ep_index;
 
@@ -1951,14 +1953,30 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	if (trb_comp_code == COMP_STALL_ERROR ||
 		xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
 						trb_comp_code)) {
-		/* Issue a reset endpoint command to clear the host side
-		 * halt, followed by a set dequeue command to move the
-		 * dequeue pointer past the TD.
-		 * The class driver clears the device side halt later.
+		/*erratum A-007463:
+		 *After transaction error, controller switches control transfer
+		 *data stage from IN to OUT direction.
 		 */
-		xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
+		remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
+		if (remaining && xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
+				trb_comp_code) &&
+				(xhci->quirks & XHCI_REVERSE_IN_OUT)) {
+			memset(&deq_state, 0, sizeof(deq_state));
+			xhci_find_new_dequeue_state(xhci, slot_id,
+				ep_index, td->urb->stream_id, td, &deq_state);
+			xhci_queue_new_dequeue_state(xhci, slot_id, ep_index,
+						&deq_state);
+			xhci_ring_cmd_db(xhci);
+		} else {
+			/* Issue a reset endpoint command to clear the host side
+			 * halt, followed by a set dequeue command to move the
+			 * dequeue pointer past the TD.
+			 * The class driver clears the device side halt later.
+			 */
+			xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
 					ep_ring->stream_id, td, ep_trb,
 					EP_HARD_RESET);
+		}
 	} else {
 		/* Update ring dequeue pointer */
 		while (ep_ring->dequeue != td->last_trb)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 96099a2..9f133a9 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1783,7 +1783,7 @@ struct xhci_hcd {
 #define XHCI_STATE_DYING	(1 << 0)
 #define XHCI_STATE_HALTED	(1 << 1)
 #define XHCI_STATE_REMOVING	(1 << 2)
-	unsigned int		quirks;
+	u64		quirks;
 #define	XHCI_LINK_TRB_QUIRK	(1 << 0)
 #define XHCI_RESET_EP_QUIRK	(1 << 1)
 #define XHCI_NEC_HOST		(1 << 2)
@@ -1819,6 +1819,7 @@ struct xhci_hcd {
 #define XHCI_SSIC_PORT_UNUSED	(1 << 22)
 #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
 #define XHCI_MISSING_CAS	(1 << 24)
+#define XHCI_REVERSE_IN_OUT     BIT(32)
 /* For controller with a broken Port Disable implementation */
 #define XHCI_BROKEN_PORT_PED	(1 << 25)
 #define XHCI_LIMIT_ENDPOINT_INTERVAL_7	(1 << 26)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 2/3] usb: host: Implement workaround for Erratum A-009611
@ 2017-12-19 10:16   ` yinbo.zhu
  0 siblings, 0 replies; 12+ messages in thread
From: yinbo.zhu @ 2017-12-19 10:16 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1, yinbo . zhu

From: yinbo.zhu <yinbo.zhu@nxp.com>

This is a occasional problem where the software issues an End
Transfer command while a USB transfer is in progress,
resulting in the TxFIFO  being flushed when the lower layer is
waiting for data, causing the super speed (ss) transmit to get
blocked. If the End Transfer command is issued on an IN
endpoint to flush out the pending transfers when the same IN
endpoint is doing transfers on the USB, then depending upon
the timing of the End Transfer (and the resulting internal
flush),the lower layer (U3PTL/U3MAC) could get stuck waiting
for data indefinitely. This blocks the transmission path on
the SS, and no DP/ACK/ERDY/DEVNOTIF packets can be sent from
the device. Impact: If this issue happens and the transmission
gets blocked, then the USB host aborts and
resets/re-enumerates the device. This unblocks the transmitt
engine and the device functions normally.

Workaround: Software must wait for all existing TRBs to
complete before issuing End transfer command.

Configs Affected:
LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0,
LS2088-48A-R1.1, LX2160-2120-2080A-R1.

Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
---
Change in v4:
                Remove the point in "yinbo.zhu"
 drivers/usb/dwc3/core.c      |    2 ++
 drivers/usb/dwc3/core.h      |    3 +++
 drivers/usb/dwc3/host.c      |    3 +++
 drivers/usb/host/xhci-plat.c |    4 ++++
 drivers/usb/host/xhci.c      |   25 +++++++++++++++++++------
 drivers/usb/host/xhci.h      |    1 +
 6 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 6613bc0..863f2c0 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1057,6 +1057,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				"snps,tx_de_emphasis_quirk");
 	dwc->quirk_reverse_in_out = device_property_read_bool(dev,
 				"snps,quirk_reverse_in_out");
+	dwc->quirk_stop_transfer_in_block = device_property_read_bool(dev,
+				"snps,quirk_stop_transfer_in_block");
 	device_property_read_u8(dev, "snps,tx_de_emphasis",
 				&tx_de_emphasis);
 	device_property_read_string(dev, "snps,hsphy_interface",
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index a263fdc..6276678 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -858,6 +858,8 @@ struct dwc3_scratchpad_array {
  * 	2	- No de-emphasis
  * 	3	- Reserved
  * @quirk_reverse_in_out: prevent tx fifo reverse the data direction
+ * @quirk_stop_transfer_in_block: prevent block transmission from being
+ *				  interrupted
  * @imod_interval: set the interrupt moderation interval in 250ns
  *                 increments or 0 to disable.
  */
@@ -1011,6 +1013,7 @@ struct dwc3 {
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
 	unsigned                quirk_reverse_in_out:1;
+	unsigned                quirk_stop_transfer_in_block:1;
 
 	u16			imod_interval;
 };
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index dab5f49..78cb7bb 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -93,6 +93,9 @@ int dwc3_host_init(struct dwc3 *dwc)
 	if (dwc->quirk_reverse_in_out)
 		props[prop_idx++].name = "quirk-reverse-in-out";
 
+	if (dwc->quirk_stop_transfer_in_block)
+		props[prop_idx++].name = "quirk-stop-transfer-in-block";
+
 	if (dwc->usb3_lpm_capable)
 		props[prop_idx++].name = "usb3-lpm-capable";
 
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index fe71b92..35e0fc8 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
 		xhci->quirks |= XHCI_REVERSE_IN_OUT;
 
+	if (device_property_read_bool(&pdev->dev,
+				"quirk-stop-transfer-in-block"))
+		xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
+
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 05104bd..5141856 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1501,13 +1501,26 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 			ret = -ENOMEM;
 			goto done;
 		}
-		ep->ep_state |= EP_STOP_CMD_PENDING;
-		ep->stop_cmd_timer.expires = jiffies +
+		/*
+		 *erratum A-009611: Issuing an End Transfer command on an IN
+		 *endpoint. when a transfer is in progress on USB blocks the
+		 *transmission.
+		 *Workaround: Software must wait for all existing TRBs to
+		 *complete before issuing End transfer command.
+		 */
+		if ((ep_ring->enqueue == ep_ring->dequeue &&
+				(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) ||
+				!(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) {
+			ep->ep_state |= EP_STOP_CMD_PENDING;
+			ep->stop_cmd_timer.expires = jiffies +
 			XHCI_STOP_EP_CMD_TIMEOUT * HZ;
-		add_timer(&ep->stop_cmd_timer);
-		xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id,
-					 ep_index, 0);
-		xhci_ring_cmd_db(xhci);
+			add_timer(&ep->stop_cmd_timer);
+			xhci_queue_stop_endpoint(xhci, command,
+					urb->dev->slot_id,
+					ep_index, 0);
+			xhci_ring_cmd_db(xhci);
+		}
+
 	}
 done:
 	spin_unlock_irqrestore(&xhci->lock, flags);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 9f133a9..db10ee4 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1820,6 +1820,7 @@ struct xhci_hcd {
 #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
 #define XHCI_MISSING_CAS	(1 << 24)
 #define XHCI_REVERSE_IN_OUT     BIT(32)
+#define XHCI_STOP_TRANSFER_IN_BLOCK   BIT(33)
 /* For controller with a broken Port Disable implementation */
 #define XHCI_BROKEN_PORT_PED	(1 << 25)
 #define XHCI_LIMIT_ENDPOINT_INTERVAL_7	(1 << 26)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [v4,2/3] usb: host: Implement workaround for Erratum A-009611
@ 2017-12-19 10:16   ` yinbo.zhu
  0 siblings, 0 replies; 12+ messages in thread
From: yinbo.zhu @ 2017-12-19 10:16 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1, yinbo . zhu

From: yinbo.zhu <yinbo.zhu@nxp.com>

This is a occasional problem where the software issues an End
Transfer command while a USB transfer is in progress,
resulting in the TxFIFO  being flushed when the lower layer is
waiting for data, causing the super speed (ss) transmit to get
blocked. If the End Transfer command is issued on an IN
endpoint to flush out the pending transfers when the same IN
endpoint is doing transfers on the USB, then depending upon
the timing of the End Transfer (and the resulting internal
flush),the lower layer (U3PTL/U3MAC) could get stuck waiting
for data indefinitely. This blocks the transmission path on
the SS, and no DP/ACK/ERDY/DEVNOTIF packets can be sent from
the device. Impact: If this issue happens and the transmission
gets blocked, then the USB host aborts and
resets/re-enumerates the device. This unblocks the transmitt
engine and the device functions normally.

Workaround: Software must wait for all existing TRBs to
complete before issuing End transfer command.

Configs Affected:
LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0,
LS2088-48A-R1.1, LX2160-2120-2080A-R1.

Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
---
Change in v4:
                Remove the point in "yinbo.zhu"
 drivers/usb/dwc3/core.c      |    2 ++
 drivers/usb/dwc3/core.h      |    3 +++
 drivers/usb/dwc3/host.c      |    3 +++
 drivers/usb/host/xhci-plat.c |    4 ++++
 drivers/usb/host/xhci.c      |   25 +++++++++++++++++++------
 drivers/usb/host/xhci.h      |    1 +
 6 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 6613bc0..863f2c0 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1057,6 +1057,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				"snps,tx_de_emphasis_quirk");
 	dwc->quirk_reverse_in_out = device_property_read_bool(dev,
 				"snps,quirk_reverse_in_out");
+	dwc->quirk_stop_transfer_in_block = device_property_read_bool(dev,
+				"snps,quirk_stop_transfer_in_block");
 	device_property_read_u8(dev, "snps,tx_de_emphasis",
 				&tx_de_emphasis);
 	device_property_read_string(dev, "snps,hsphy_interface",
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index a263fdc..6276678 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -858,6 +858,8 @@ struct dwc3_scratchpad_array {
  * 	2	- No de-emphasis
  * 	3	- Reserved
  * @quirk_reverse_in_out: prevent tx fifo reverse the data direction
+ * @quirk_stop_transfer_in_block: prevent block transmission from being
+ *				  interrupted
  * @imod_interval: set the interrupt moderation interval in 250ns
  *                 increments or 0 to disable.
  */
@@ -1011,6 +1013,7 @@ struct dwc3 {
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
 	unsigned                quirk_reverse_in_out:1;
+	unsigned                quirk_stop_transfer_in_block:1;
 
 	u16			imod_interval;
 };
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index dab5f49..78cb7bb 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -93,6 +93,9 @@ int dwc3_host_init(struct dwc3 *dwc)
 	if (dwc->quirk_reverse_in_out)
 		props[prop_idx++].name = "quirk-reverse-in-out";
 
+	if (dwc->quirk_stop_transfer_in_block)
+		props[prop_idx++].name = "quirk-stop-transfer-in-block";
+
 	if (dwc->usb3_lpm_capable)
 		props[prop_idx++].name = "usb3-lpm-capable";
 
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index fe71b92..35e0fc8 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
 		xhci->quirks |= XHCI_REVERSE_IN_OUT;
 
+	if (device_property_read_bool(&pdev->dev,
+				"quirk-stop-transfer-in-block"))
+		xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
+
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 05104bd..5141856 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1501,13 +1501,26 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 			ret = -ENOMEM;
 			goto done;
 		}
-		ep->ep_state |= EP_STOP_CMD_PENDING;
-		ep->stop_cmd_timer.expires = jiffies +
+		/*
+		 *erratum A-009611: Issuing an End Transfer command on an IN
+		 *endpoint. when a transfer is in progress on USB blocks the
+		 *transmission.
+		 *Workaround: Software must wait for all existing TRBs to
+		 *complete before issuing End transfer command.
+		 */
+		if ((ep_ring->enqueue == ep_ring->dequeue &&
+				(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) ||
+				!(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) {
+			ep->ep_state |= EP_STOP_CMD_PENDING;
+			ep->stop_cmd_timer.expires = jiffies +
 			XHCI_STOP_EP_CMD_TIMEOUT * HZ;
-		add_timer(&ep->stop_cmd_timer);
-		xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id,
-					 ep_index, 0);
-		xhci_ring_cmd_db(xhci);
+			add_timer(&ep->stop_cmd_timer);
+			xhci_queue_stop_endpoint(xhci, command,
+					urb->dev->slot_id,
+					ep_index, 0);
+			xhci_ring_cmd_db(xhci);
+		}
+
 	}
 done:
 	spin_unlock_irqrestore(&xhci->lock, flags);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 9f133a9..db10ee4 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1820,6 +1820,7 @@ struct xhci_hcd {
 #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
 #define XHCI_MISSING_CAS	(1 << 24)
 #define XHCI_REVERSE_IN_OUT     BIT(32)
+#define XHCI_STOP_TRANSFER_IN_BLOCK   BIT(33)
 /* For controller with a broken Port Disable implementation */
 #define XHCI_BROKEN_PORT_PED	(1 << 25)
 #define XHCI_LIMIT_ENDPOINT_INTERVAL_7	(1 << 26)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 3/3] usb: host: Implement workaround for Erratum A-009668
@ 2017-12-19 10:16   ` yinbo.zhu
  0 siblings, 0 replies; 12+ messages in thread
From: yinbo.zhu @ 2017-12-19 10:16 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1, yinbo . zhu

From: yinbo.zhu <yinbo.zhu@nxp.com>

This issue is observed in USB 2.0 mode when the USB 3.0 host
controller is connected to a FS/LS device via a hub. The
host controller issues start-split (SSPLIT) and (CSPLIT)
tokens to accomplish a split-transaction. A
split-transaction consists of a SSPLIT token, token/data
consists of a SSPLIT token, token/data packets, CSPLIT token
and token/data/handshake packets. A SSPLIT token is issued
by the host controller to the hub followed by token/data/
handshake packets. The hub then relays the token/data/
handshake packets to the FS /LS device. Sometime later, the
host controller issues a CSPLIT token followed by the same
token/data/handshake packets to the hub to complete the
split-transaction. As an example scenario, when the xHCI
driver issues an Address device command with BSR=0, the
host controller sends SETUP(SET_ADDRESS) tokens on the USB
as part of splittransactions. If the host controller
receives a NYET response from the hub for the CSPLIT SETUP,
it means that the split-transaction has not yet been
completed or the hub is not able to handle the split
transaction. In such a case, the host controller keeps
retrying the splittransactions until such time an ACK
response is received from the hub for the CSPLIT SETUP token
. If the split-transactions do not complete in a time bound
manner, the xHCI driver may issue a Stop Endpoint Command.
The host controller does not service the Stop Endpoint
Command and eventually the xHCI driver times out waiting for
the Stop Endpoint Command to complete.

Impact: Stop Endpoint Command does not complete.
Workaround: Instead of issuing a Stop Endpoint Command,
issue a Disable Slot Command with the corresponding slot
ID. Alternately, you can issue an Address Device Command
with BSR=1.

Configs Affected:
LS1012A-R1.0, LS1012A-R2.0, LS1021-20-22A-R1.0,
LS1021-20-22A-R2.0, LS1043-23A-R1.0, LS1043-23A-R1.1,
LS1046-26A-R1.0, LS1088-48A-R1.0, LS2080-40A-R1.0,
LS2081A-R1.1, LS2085-45A-R1.0, LS2088-48A-R1.0,
LS2088-48A-R1.1, LX2160-2120-2080A-R1.0.

Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
---
Change in v4:
                Remove the point in "yinbo.zhu"
 drivers/usb/dwc3/core.c      |    2 ++
 drivers/usb/dwc3/core.h      |    2 ++
 drivers/usb/dwc3/host.c      |    3 +++
 drivers/usb/host/xhci-plat.c |    3 +++
 drivers/usb/host/xhci.c      |   12 ++++++++++++
 drivers/usb/host/xhci.h      |    1 +
 6 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 863f2c0..90b097c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1059,6 +1059,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				"snps,quirk_reverse_in_out");
 	dwc->quirk_stop_transfer_in_block = device_property_read_bool(dev,
 				"snps,quirk_stop_transfer_in_block");
+	dwc->quirk_stop_ep_in_u1 = device_property_read_bool(dev,
+				"snps,quirk_stop_ep_in_u1");
 	device_property_read_u8(dev, "snps,tx_de_emphasis",
 				&tx_de_emphasis);
 	device_property_read_string(dev, "snps,hsphy_interface",
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6276678..b55a443 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -860,6 +860,7 @@ struct dwc3_scratchpad_array {
  * @quirk_reverse_in_out: prevent tx fifo reverse the data direction
  * @quirk_stop_transfer_in_block: prevent block transmission from being
  *				  interrupted
+ * @quirk_stop_ep_in_u1: replace stop commad with disable slot command
  * @imod_interval: set the interrupt moderation interval in 250ns
  *                 increments or 0 to disable.
  */
@@ -1014,6 +1015,7 @@ struct dwc3 {
 	unsigned		tx_de_emphasis:2;
 	unsigned                quirk_reverse_in_out:1;
 	unsigned                quirk_stop_transfer_in_block:1;
+	unsigned                quirk_stop_ep_in_u1:1;
 
 	u16			imod_interval;
 };
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 78cb7bb..0a34274 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -96,6 +96,9 @@ int dwc3_host_init(struct dwc3 *dwc)
 	if (dwc->quirk_stop_transfer_in_block)
 		props[prop_idx++].name = "quirk-stop-transfer-in-block";
 
+	if (dwc->quirk_stop_ep_in_u1)
+		props[prop_idx++].name = "quirk-stop-ep-in-u1";
+
 	if (dwc->usb3_lpm_capable)
 		props[prop_idx++].name = "usb3-lpm-capable";
 
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 35e0fc8..49d6cb4 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -273,6 +273,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 				"quirk-stop-transfer-in-block"))
 		xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
 
+	if (device_property_read_bool(&pdev->dev, "quirk-stop-ep-in-u1"))
+		xhci->quirks |= XHCI_STOP_EP_IN_U1;
+
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5141856..20c9af4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1521,6 +1521,18 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 			xhci_ring_cmd_db(xhci);
 		}
 
+		/*
+		 *erratum A-009668: Stop Endpoint Command does not complete.
+		 *Workaround: Instead of issuing a Stop Endpoint Command,
+		 *issue a Disable Slot Command with the corresponding slot ID.
+		 *Alternately, you can issue an Address Device Command with
+		 *BSR=1
+		 */
+		if ((urb->dev->speed <= USB_SPEED_HIGH) &&
+					(xhci->quirks & XHCI_STOP_EP_IN_U1)) {
+			xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
+				urb->dev->slot_id);
+		}
 	}
 done:
 	spin_unlock_irqrestore(&xhci->lock, flags);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index db10ee4..22ba752 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1821,6 +1821,7 @@ struct xhci_hcd {
 #define XHCI_MISSING_CAS	(1 << 24)
 #define XHCI_REVERSE_IN_OUT     BIT(32)
 #define XHCI_STOP_TRANSFER_IN_BLOCK   BIT(33)
+#define XHCI_STOP_EP_IN_U1     BIT(34)
 /* For controller with a broken Port Disable implementation */
 #define XHCI_BROKEN_PORT_PED	(1 << 25)
 #define XHCI_LIMIT_ENDPOINT_INTERVAL_7	(1 << 26)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [v4,3/3] usb: host: Implement workaround for Erratum A-009668
@ 2017-12-19 10:16   ` yinbo.zhu
  0 siblings, 0 replies; 12+ messages in thread
From: yinbo.zhu @ 2017-12-19 10:16 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1, yinbo . zhu

From: yinbo.zhu <yinbo.zhu@nxp.com>

This issue is observed in USB 2.0 mode when the USB 3.0 host
controller is connected to a FS/LS device via a hub. The
host controller issues start-split (SSPLIT) and (CSPLIT)
tokens to accomplish a split-transaction. A
split-transaction consists of a SSPLIT token, token/data
consists of a SSPLIT token, token/data packets, CSPLIT token
and token/data/handshake packets. A SSPLIT token is issued
by the host controller to the hub followed by token/data/
handshake packets. The hub then relays the token/data/
handshake packets to the FS /LS device. Sometime later, the
host controller issues a CSPLIT token followed by the same
token/data/handshake packets to the hub to complete the
split-transaction. As an example scenario, when the xHCI
driver issues an Address device command with BSR=0, the
host controller sends SETUP(SET_ADDRESS) tokens on the USB
as part of splittransactions. If the host controller
receives a NYET response from the hub for the CSPLIT SETUP,
it means that the split-transaction has not yet been
completed or the hub is not able to handle the split
transaction. In such a case, the host controller keeps
retrying the splittransactions until such time an ACK
response is received from the hub for the CSPLIT SETUP token
. If the split-transactions do not complete in a time bound
manner, the xHCI driver may issue a Stop Endpoint Command.
The host controller does not service the Stop Endpoint
Command and eventually the xHCI driver times out waiting for
the Stop Endpoint Command to complete.

Impact: Stop Endpoint Command does not complete.
Workaround: Instead of issuing a Stop Endpoint Command,
issue a Disable Slot Command with the corresponding slot
ID. Alternately, you can issue an Address Device Command
with BSR=1.

Configs Affected:
LS1012A-R1.0, LS1012A-R2.0, LS1021-20-22A-R1.0,
LS1021-20-22A-R2.0, LS1043-23A-R1.0, LS1043-23A-R1.1,
LS1046-26A-R1.0, LS1088-48A-R1.0, LS2080-40A-R1.0,
LS2081A-R1.1, LS2085-45A-R1.0, LS2088-48A-R1.0,
LS2088-48A-R1.1, LX2160-2120-2080A-R1.0.

Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
---
Change in v4:
                Remove the point in "yinbo.zhu"
 drivers/usb/dwc3/core.c      |    2 ++
 drivers/usb/dwc3/core.h      |    2 ++
 drivers/usb/dwc3/host.c      |    3 +++
 drivers/usb/host/xhci-plat.c |    3 +++
 drivers/usb/host/xhci.c      |   12 ++++++++++++
 drivers/usb/host/xhci.h      |    1 +
 6 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 863f2c0..90b097c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1059,6 +1059,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				"snps,quirk_reverse_in_out");
 	dwc->quirk_stop_transfer_in_block = device_property_read_bool(dev,
 				"snps,quirk_stop_transfer_in_block");
+	dwc->quirk_stop_ep_in_u1 = device_property_read_bool(dev,
+				"snps,quirk_stop_ep_in_u1");
 	device_property_read_u8(dev, "snps,tx_de_emphasis",
 				&tx_de_emphasis);
 	device_property_read_string(dev, "snps,hsphy_interface",
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6276678..b55a443 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -860,6 +860,7 @@ struct dwc3_scratchpad_array {
  * @quirk_reverse_in_out: prevent tx fifo reverse the data direction
  * @quirk_stop_transfer_in_block: prevent block transmission from being
  *				  interrupted
+ * @quirk_stop_ep_in_u1: replace stop commad with disable slot command
  * @imod_interval: set the interrupt moderation interval in 250ns
  *                 increments or 0 to disable.
  */
@@ -1014,6 +1015,7 @@ struct dwc3 {
 	unsigned		tx_de_emphasis:2;
 	unsigned                quirk_reverse_in_out:1;
 	unsigned                quirk_stop_transfer_in_block:1;
+	unsigned                quirk_stop_ep_in_u1:1;
 
 	u16			imod_interval;
 };
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 78cb7bb..0a34274 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -96,6 +96,9 @@ int dwc3_host_init(struct dwc3 *dwc)
 	if (dwc->quirk_stop_transfer_in_block)
 		props[prop_idx++].name = "quirk-stop-transfer-in-block";
 
+	if (dwc->quirk_stop_ep_in_u1)
+		props[prop_idx++].name = "quirk-stop-ep-in-u1";
+
 	if (dwc->usb3_lpm_capable)
 		props[prop_idx++].name = "usb3-lpm-capable";
 
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 35e0fc8..49d6cb4 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -273,6 +273,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 				"quirk-stop-transfer-in-block"))
 		xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
 
+	if (device_property_read_bool(&pdev->dev, "quirk-stop-ep-in-u1"))
+		xhci->quirks |= XHCI_STOP_EP_IN_U1;
+
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5141856..20c9af4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1521,6 +1521,18 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 			xhci_ring_cmd_db(xhci);
 		}
 
+		/*
+		 *erratum A-009668: Stop Endpoint Command does not complete.
+		 *Workaround: Instead of issuing a Stop Endpoint Command,
+		 *issue a Disable Slot Command with the corresponding slot ID.
+		 *Alternately, you can issue an Address Device Command with
+		 *BSR=1
+		 */
+		if ((urb->dev->speed <= USB_SPEED_HIGH) &&
+					(xhci->quirks & XHCI_STOP_EP_IN_U1)) {
+			xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
+				urb->dev->slot_id);
+		}
 	}
 done:
 	spin_unlock_irqrestore(&xhci->lock, flags);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index db10ee4..22ba752 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1821,6 +1821,7 @@ struct xhci_hcd {
 #define XHCI_MISSING_CAS	(1 << 24)
 #define XHCI_REVERSE_IN_OUT     BIT(32)
 #define XHCI_STOP_TRANSFER_IN_BLOCK   BIT(33)
+#define XHCI_STOP_EP_IN_U1     BIT(34)
 /* For controller with a broken Port Disable implementation */
 #define XHCI_BROKEN_PORT_PED	(1 << 25)
 #define XHCI_LIMIT_ENDPOINT_INTERVAL_7	(1 << 26)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/3] usb: host: Implement workaround for Erratum A-007463
@ 2018-01-04 13:38   ` Mathias Nyman
  0 siblings, 0 replies; 12+ messages in thread
From: Mathias Nyman @ 2018-01-04 13:38 UTC (permalink / raw)
  To: yinbo.zhu, Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1

On 19.12.2017 12:16, yinbo.zhu@nxp.com wrote:
> From: yinbo.zhu <yinbo.zhu@nxp.com>
> 
> When a transaction error (defined in Section 4.10.2.3, "USB
> Transaction Error" of the xHCI Specification) occurs on the
> USB, the host controller reports this through a transfer
> event with the completion code "USB Transaction Error". When
> this happens, the endpoint is placed in the Halted state. In
> response, software must issue a Reset Endpoint command to
> transition the endpoint to the Stopped state. In order to
> restart the transfer, the driver can perform either of the
> following:
> a) Ring the doorbell again, which restarts the transfer from
>     where it stopped, or
> b) Issue a Set TR (Transfer Ring) Dequeue Pointer command
>     for the endpoint to start the transfer form a different
>     Transfer Ring pointer Consider the following
>     scenario:
> 1. The xHCI driver prepares a control transfer read to one
>     of the device's control endpoints;
> 2. During the IN data stage, a transaction error occurs on
>     the USB, causing a transfer event with the completion
>     code "USB Transaction Error";
> 3. The driver issues a Reset Endpoint command;
> 4. The driver rings the doorbell of the control endpoint to
>     resume the transfer. In this scenario, the controller
>     may reverse the direction of the data stage from IN to OUT.

The xhci driver should not ring the doorbell without first issuing a
Set TR Dequeue Pointer command in the USB Transaction error case.

code flow:

handle_tx_event()
   case COMP_USB_TRANSACTION_ERROR:
     status = -EPROTO;
   process_ctrl_td()
     switch (trb_comp_code)
     default:
       if (!xhci_requires_manual_halt_cleanup()  // FALSE
         break;
       /* else fall through */
       case COMP_STALL_ERROR:
         finish_td()
           if (xhci_requires_manual_halt_cleanup())   // TRUE
             xhci_cleanup_halted_endpoint(...,EP_HARD_RESET)
               xhci_queue_reset_ep()
               xhci_cleanup_stalled_ring()
                 xhci_find_new_dequeue_state()
                 xhci_queue_new_dequeue_state()

If you can see the driver ringing the doorbell before the
Set TR Dequeue Pointer command then there is some race going
on in the driver

>     Instead of sending an ACK to the endpoint to poll for read
>     data, it sends a Data Packet (DP) to the endpoint. It
>     fetches the data from the data stage Transfer Request
>     Block (TRB) that is being resumed, even though the data
>     buffer is setup to receive data and not transmit it.
> NOTE
> This issue occurs only if the transaction error happens
> during an IN data stage. There is no issue if the transaction
> error happens during an OUT data stage.

Have you been able to trigger this issue?

> 
> Impact: When this issue occurs, the device likely responds in
> one of the following ways:
> a) The device responds with a STALL because the data stage
> has unexpectedly changed directions. The controller then
> generates a Stall Error transfer event, to which software
> must issue a Reset Endpoint command followed by a Set TR
> Dequeue Pointer command pointing to a new Setup TRB to clear
> the STALL condition.
> b) The device does not respond to the inverted data stage and
> the transaction times out. The controller generates another
> USB Transaction Error transfer event, to which software
> likely performs a USB Reset to the device because it is
> unresponsive. It is not expected that any of these recovery
> steps will cause instability in the system because this
> recovery is part of a standard xHCI driver and could happen
> regardless of the defect. Another possible system-level
> impact is that the controller attempts to read from the
> memory location pointed at by the Data Stage TRB or a Normal
> TRB chained to it. associated with this TRB is intended to be
> written by the controller, but the controller reads from it
> instead. Normally, this does not cause a problem. However, if
> the system has some type of memory protection where this
> unexpected read is treated as a bus error,
> a problem. However, if the system has some type of memory
> it may cause the system to become unstable or to crash.
> 
> Workaround: If a USB Transaction Error occurs during the IN
> data phase of a control transfer, the driver must use the
> Set TR Dequeue Pointer command to either restart the data
> Phase or restart the entire control transfer from the
> Setup phase.

The Set TR Dequeue pointer command should already be the default way
the xhci driver handles this now.

> 
> Configs Affected:
> LS1021-20-22A-R1.0, LS1021-20-22A-R2.007463
> 
> Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
> ---


> @@ -1951,14 +1953,30 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
>   	if (trb_comp_code == COMP_STALL_ERROR ||
>   		xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
>   						trb_comp_code)) {
> -		/* Issue a reset endpoint command to clear the host side
> -		 * halt, followed by a set dequeue command to move the
> -		 * dequeue pointer past the TD.
> -		 * The class driver clears the device side halt later.
> +		/*erratum A-007463:
> +		 *After transaction error, controller switches control transfer
> +		 *data stage from IN to OUT direction.
>   		 */
> -		xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
> +		remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
> +		if (remaining && xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
> +				trb_comp_code) &&
> +				(xhci->quirks & XHCI_REVERSE_IN_OUT)) {
> +			memset(&deq_state, 0, sizeof(deq_state));
> +			xhci_find_new_dequeue_state(xhci, slot_id,
> +				ep_index, td->urb->stream_id, td, &deq_state);
> +			xhci_queue_new_dequeue_state(xhci, slot_id, ep_index,
> +						&deq_state);
> +			xhci_ring_cmd_db(xhci);

Even if all these changes should be unnecessary the endpoint is still halted
here, and we would need a reset endpoint command here as well, right?

Has this new codepath been tested?

> +		} else {
> +			/* Issue a reset endpoint command to clear the host side
> +			 * halt, followed by a set dequeue command to move the
> +			 * dequeue pointer past the TD.
> +			 * The class driver clears the device side halt later.
> +			 */
> +			xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
>   					ep_ring->stream_id, td, ep_trb,
>   					EP_HARD_RESET);

-Mathias

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [v4,1/3] usb: host: Implement workaround for Erratum A-007463
@ 2018-01-04 13:38   ` Mathias Nyman
  0 siblings, 0 replies; 12+ messages in thread
From: Mathias Nyman @ 2018-01-04 13:38 UTC (permalink / raw)
  To: yinbo.zhu, Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1

On 19.12.2017 12:16, yinbo.zhu@nxp.com wrote:
> From: yinbo.zhu <yinbo.zhu@nxp.com>
> 
> When a transaction error (defined in Section 4.10.2.3, "USB
> Transaction Error" of the xHCI Specification) occurs on the
> USB, the host controller reports this through a transfer
> event with the completion code "USB Transaction Error". When
> this happens, the endpoint is placed in the Halted state. In
> response, software must issue a Reset Endpoint command to
> transition the endpoint to the Stopped state. In order to
> restart the transfer, the driver can perform either of the
> following:
> a) Ring the doorbell again, which restarts the transfer from
>     where it stopped, or
> b) Issue a Set TR (Transfer Ring) Dequeue Pointer command
>     for the endpoint to start the transfer form a different
>     Transfer Ring pointer Consider the following
>     scenario:
> 1. The xHCI driver prepares a control transfer read to one
>     of the device's control endpoints;
> 2. During the IN data stage, a transaction error occurs on
>     the USB, causing a transfer event with the completion
>     code "USB Transaction Error";
> 3. The driver issues a Reset Endpoint command;
> 4. The driver rings the doorbell of the control endpoint to
>     resume the transfer. In this scenario, the controller
>     may reverse the direction of the data stage from IN to OUT.

The xhci driver should not ring the doorbell without first issuing a
Set TR Dequeue Pointer command in the USB Transaction error case.

code flow:

handle_tx_event()
   case COMP_USB_TRANSACTION_ERROR:
     status = -EPROTO;
   process_ctrl_td()
     switch (trb_comp_code)
     default:
       if (!xhci_requires_manual_halt_cleanup()  // FALSE
         break;
       /* else fall through */
       case COMP_STALL_ERROR:
         finish_td()
           if (xhci_requires_manual_halt_cleanup())   // TRUE
             xhci_cleanup_halted_endpoint(...,EP_HARD_RESET)
               xhci_queue_reset_ep()
               xhci_cleanup_stalled_ring()
                 xhci_find_new_dequeue_state()
                 xhci_queue_new_dequeue_state()

If you can see the driver ringing the doorbell before the
Set TR Dequeue Pointer command then there is some race going
on in the driver

>     Instead of sending an ACK to the endpoint to poll for read
>     data, it sends a Data Packet (DP) to the endpoint. It
>     fetches the data from the data stage Transfer Request
>     Block (TRB) that is being resumed, even though the data
>     buffer is setup to receive data and not transmit it.
> NOTE
> This issue occurs only if the transaction error happens
> during an IN data stage. There is no issue if the transaction
> error happens during an OUT data stage.

Have you been able to trigger this issue?

> 
> Impact: When this issue occurs, the device likely responds in
> one of the following ways:
> a) The device responds with a STALL because the data stage
> has unexpectedly changed directions. The controller then
> generates a Stall Error transfer event, to which software
> must issue a Reset Endpoint command followed by a Set TR
> Dequeue Pointer command pointing to a new Setup TRB to clear
> the STALL condition.
> b) The device does not respond to the inverted data stage and
> the transaction times out. The controller generates another
> USB Transaction Error transfer event, to which software
> likely performs a USB Reset to the device because it is
> unresponsive. It is not expected that any of these recovery
> steps will cause instability in the system because this
> recovery is part of a standard xHCI driver and could happen
> regardless of the defect. Another possible system-level
> impact is that the controller attempts to read from the
> memory location pointed at by the Data Stage TRB or a Normal
> TRB chained to it. associated with this TRB is intended to be
> written by the controller, but the controller reads from it
> instead. Normally, this does not cause a problem. However, if
> the system has some type of memory protection where this
> unexpected read is treated as a bus error,
> a problem. However, if the system has some type of memory
> it may cause the system to become unstable or to crash.
> 
> Workaround: If a USB Transaction Error occurs during the IN
> data phase of a control transfer, the driver must use the
> Set TR Dequeue Pointer command to either restart the data
> Phase or restart the entire control transfer from the
> Setup phase.

The Set TR Dequeue pointer command should already be the default way
the xhci driver handles this now.

> 
> Configs Affected:
> LS1021-20-22A-R1.0, LS1021-20-22A-R2.007463
> 
> Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
> ---


> @@ -1951,14 +1953,30 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
>   	if (trb_comp_code == COMP_STALL_ERROR ||
>   		xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
>   						trb_comp_code)) {
> -		/* Issue a reset endpoint command to clear the host side
> -		 * halt, followed by a set dequeue command to move the
> -		 * dequeue pointer past the TD.
> -		 * The class driver clears the device side halt later.
> +		/*erratum A-007463:
> +		 *After transaction error, controller switches control transfer
> +		 *data stage from IN to OUT direction.
>   		 */
> -		xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
> +		remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
> +		if (remaining && xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
> +				trb_comp_code) &&
> +				(xhci->quirks & XHCI_REVERSE_IN_OUT)) {
> +			memset(&deq_state, 0, sizeof(deq_state));
> +			xhci_find_new_dequeue_state(xhci, slot_id,
> +				ep_index, td->urb->stream_id, td, &deq_state);
> +			xhci_queue_new_dequeue_state(xhci, slot_id, ep_index,
> +						&deq_state);
> +			xhci_ring_cmd_db(xhci);

Even if all these changes should be unnecessary the endpoint is still halted
here, and we would need a reset endpoint command here as well, right?

Has this new codepath been tested?

> +		} else {
> +			/* Issue a reset endpoint command to clear the host side
> +			 * halt, followed by a set dequeue command to move the
> +			 * dequeue pointer past the TD.
> +			 * The class driver clears the device side halt later.
> +			 */
> +			xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
>   					ep_ring->stream_id, td, ep_trb,
>   					EP_HARD_RESET);

-Mathias
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 2/3] usb: host: Implement workaround for Erratum A-009611
@ 2018-01-04 14:32     ` Mathias Nyman
  0 siblings, 0 replies; 12+ messages in thread
From: Mathias Nyman @ 2018-01-04 14:32 UTC (permalink / raw)
  To: yinbo.zhu, Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1

On 19.12.2017 12:16, yinbo.zhu@nxp.com wrote:
> From: yinbo.zhu <yinbo.zhu@nxp.com>
> 
> This is a occasional problem where the software issues an End
> Transfer command while a USB transfer is in progress,
> resulting in the TxFIFO  being flushed when the lower layer is
> waiting for data, causing the super speed (ss) transmit to get
> blocked. If the End Transfer command is issued on an IN
> endpoint to flush out the pending transfers when the same IN
> endpoint is doing transfers on the USB, then depending upon
> the timing of the End Transfer (and the resulting internal
> flush),the lower layer (U3PTL/U3MAC) could get stuck waiting
> for data indefinitely. This blocks the transmission path on
> the SS, and no DP/ACK/ERDY/DEVNOTIF packets can be sent from
> the device. Impact: If this issue happens and the transmission
> gets blocked, then the USB host aborts and
> resets/re-enumerates the device. This unblocks the transmitt
> engine and the device functions normally.
> 
> Workaround: Software must wait for all existing TRBs to
> complete before issuing End transfer command.

Are you referring to the "Stop endpoint command" when you say
End transfer command?
The Stop endpoint command is used when we want to cancel pending URBs.
So usually there will be TRBs pending when it is called.

This workaround sounds like it could cause more issues than the
occasional problem the Erratum explains. If we don't stop the
endpoint then it will continue to try and process the TRBs that
were marked to be canceled. It the URB was canceled because it
timed out then we are stuck as nothing will be done to remove it.

> 
> Configs Affected:
> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0,
> LS2088-48A-R1.1, LX2160-2120-2080A-R1.
> 
> Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
> ---
>   
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index fe71b92..35e0fc8 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   	if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
>   		xhci->quirks |= XHCI_REVERSE_IN_OUT;
>   
> +	if (device_property_read_bool(&pdev->dev,
> +				"quirk-stop-transfer-in-block"))
> +		xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
> +
>   	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
>   		xhci->quirks |= XHCI_BROKEN_PORT_PED;
>   
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 05104bd..5141856 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1501,13 +1501,26 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
>   			ret = -ENOMEM;
>   			goto done;
>   		}
> -		ep->ep_state |= EP_STOP_CMD_PENDING;
> -		ep->stop_cmd_timer.expires = jiffies +
> +		/*
> +		 *erratum A-009611: Issuing an End Transfer command on an IN
> +		 *endpoint. when a transfer is in progress on USB blocks the
> +		 *transmission.
> +		 *Workaround: Software must wait for all existing TRBs to
> +		 *complete before issuing End transfer command.
> +		 */
> +		if ((ep_ring->enqueue == ep_ring->dequeue &&
> +				(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) ||
> +				!(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) {

If you really can't issue a stop endpoint command then this should be narrowed to
when really needed, i.e.

if (has_quirk && ring_not_empty && is_superspeed && endpoint_direction_is_in)
	goto done;

Has this workaround been tested? have you tried it with a usb camera switching camera modes?

> +			ep->ep_state |= EP_STOP_CMD_PENDING;
> +			ep->stop_cmd_timer.expires = jiffies +
>   			XHCI_STOP_EP_CMD_TIMEOUT * HZ;
> -		add_timer(&ep->stop_cmd_timer);
> -		xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id,
> -					 ep_index, 0);
> -		xhci_ring_cmd_db(xhci);
> +			add_timer(&ep->stop_cmd_timer);
> +			xhci_queue_stop_endpoint(xhci, command,
> +					urb->dev->slot_id,
> +					ep_index, 0);
> +			xhci_ring_cmd_db(xhci);
> +		}
> +
>   	}
>   done:
>   	spin_unlock_irqrestore(&xhci->lock, flags);
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 9f133a9..db10ee4 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1820,6 +1820,7 @@ struct xhci_hcd {
>   #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
>   #define XHCI_MISSING_CAS	(1 << 24)
>   #define XHCI_REVERSE_IN_OUT     BIT(32)
> +#define XHCI_STOP_TRANSFER_IN_BLOCK   BIT(33)

Need to make sure we have that many bits available in the quirk variables

-Mathias

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [v4,2/3] usb: host: Implement workaround for Erratum A-009611
@ 2018-01-04 14:32     ` Mathias Nyman
  0 siblings, 0 replies; 12+ messages in thread
From: Mathias Nyman @ 2018-01-04 14:32 UTC (permalink / raw)
  To: yinbo.zhu, Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1

On 19.12.2017 12:16, yinbo.zhu@nxp.com wrote:
> From: yinbo.zhu <yinbo.zhu@nxp.com>
> 
> This is a occasional problem where the software issues an End
> Transfer command while a USB transfer is in progress,
> resulting in the TxFIFO  being flushed when the lower layer is
> waiting for data, causing the super speed (ss) transmit to get
> blocked. If the End Transfer command is issued on an IN
> endpoint to flush out the pending transfers when the same IN
> endpoint is doing transfers on the USB, then depending upon
> the timing of the End Transfer (and the resulting internal
> flush),the lower layer (U3PTL/U3MAC) could get stuck waiting
> for data indefinitely. This blocks the transmission path on
> the SS, and no DP/ACK/ERDY/DEVNOTIF packets can be sent from
> the device. Impact: If this issue happens and the transmission
> gets blocked, then the USB host aborts and
> resets/re-enumerates the device. This unblocks the transmitt
> engine and the device functions normally.
> 
> Workaround: Software must wait for all existing TRBs to
> complete before issuing End transfer command.

Are you referring to the "Stop endpoint command" when you say
End transfer command?
The Stop endpoint command is used when we want to cancel pending URBs.
So usually there will be TRBs pending when it is called.

This workaround sounds like it could cause more issues than the
occasional problem the Erratum explains. If we don't stop the
endpoint then it will continue to try and process the TRBs that
were marked to be canceled. It the URB was canceled because it
timed out then we are stuck as nothing will be done to remove it.

> 
> Configs Affected:
> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0,
> LS2088-48A-R1.1, LX2160-2120-2080A-R1.
> 
> Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
> ---
>   
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index fe71b92..35e0fc8 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   	if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
>   		xhci->quirks |= XHCI_REVERSE_IN_OUT;
>   
> +	if (device_property_read_bool(&pdev->dev,
> +				"quirk-stop-transfer-in-block"))
> +		xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
> +
>   	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
>   		xhci->quirks |= XHCI_BROKEN_PORT_PED;
>   
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 05104bd..5141856 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1501,13 +1501,26 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
>   			ret = -ENOMEM;
>   			goto done;
>   		}
> -		ep->ep_state |= EP_STOP_CMD_PENDING;
> -		ep->stop_cmd_timer.expires = jiffies +
> +		/*
> +		 *erratum A-009611: Issuing an End Transfer command on an IN
> +		 *endpoint. when a transfer is in progress on USB blocks the
> +		 *transmission.
> +		 *Workaround: Software must wait for all existing TRBs to
> +		 *complete before issuing End transfer command.
> +		 */
> +		if ((ep_ring->enqueue == ep_ring->dequeue &&
> +				(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) ||
> +				!(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) {

If you really can't issue a stop endpoint command then this should be narrowed to
when really needed, i.e.

if (has_quirk && ring_not_empty && is_superspeed && endpoint_direction_is_in)
	goto done;

Has this workaround been tested? have you tried it with a usb camera switching camera modes?

> +			ep->ep_state |= EP_STOP_CMD_PENDING;
> +			ep->stop_cmd_timer.expires = jiffies +
>   			XHCI_STOP_EP_CMD_TIMEOUT * HZ;
> -		add_timer(&ep->stop_cmd_timer);
> -		xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id,
> -					 ep_index, 0);
> -		xhci_ring_cmd_db(xhci);
> +			add_timer(&ep->stop_cmd_timer);
> +			xhci_queue_stop_endpoint(xhci, command,
> +					urb->dev->slot_id,
> +					ep_index, 0);
> +			xhci_ring_cmd_db(xhci);
> +		}
> +
>   	}
>   done:
>   	spin_unlock_irqrestore(&xhci->lock, flags);
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 9f133a9..db10ee4 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1820,6 +1820,7 @@ struct xhci_hcd {
>   #define XHCI_NO_64BIT_SUPPORT	(1 << 23)
>   #define XHCI_MISSING_CAS	(1 << 24)
>   #define XHCI_REVERSE_IN_OUT     BIT(32)
> +#define XHCI_STOP_TRANSFER_IN_BLOCK   BIT(33)

Need to make sure we have that many bits available in the quirk variables

-Mathias
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 3/3] usb: host: Implement workaround for Erratum A-009668
@ 2018-01-04 14:58     ` Mathias Nyman
  0 siblings, 0 replies; 12+ messages in thread
From: Mathias Nyman @ 2018-01-04 14:58 UTC (permalink / raw)
  To: yinbo.zhu, Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1

On 19.12.2017 12:16, yinbo.zhu@nxp.com wrote:
> From: yinbo.zhu <yinbo.zhu@nxp.com>
> 
> This issue is observed in USB 2.0 mode when the USB 3.0 host
> controller is connected to a FS/LS device via a hub. The
> host controller issues start-split (SSPLIT) and (CSPLIT)
> tokens to accomplish a split-transaction. A
> split-transaction consists of a SSPLIT token, token/data
> consists of a SSPLIT token, token/data packets, CSPLIT token
> and token/data/handshake packets. A SSPLIT token is issued
> by the host controller to the hub followed by token/data/
> handshake packets. The hub then relays the token/data/
> handshake packets to the FS /LS device. Sometime later, the
> host controller issues a CSPLIT token followed by the same
> token/data/handshake packets to the hub to complete the
> split-transaction. As an example scenario, when the xHCI
> driver issues an Address device command with BSR=0, the
> host controller sends SETUP(SET_ADDRESS) tokens on the USB
> as part of splittransactions. If the host controller
> receives a NYET response from the hub for the CSPLIT SETUP,
> it means that the split-transaction has not yet been
> completed or the hub is not able to handle the split
> transaction. In such a case, the host controller keeps
> retrying the splittransactions until such time an ACK
> response is received from the hub for the CSPLIT SETUP token
> . If the split-transactions do not complete in a time bound
> manner, the xHCI driver may issue a Stop Endpoint Command.
> The host controller does not service the Stop Endpoint
> Command and eventually the xHCI driver times out waiting for
> the Stop Endpoint Command to complete.
> 
> Impact: Stop Endpoint Command does not complete.
> Workaround: Instead of issuing a Stop Endpoint Command,
> issue a Disable Slot Command with the corresponding slot
> ID. Alternately, you can issue an Address Device Command
> with BSR=1.
> 
> Configs Affected:
> LS1012A-R1.0, LS1012A-R2.0, LS1021-20-22A-R1.0,
> LS1021-20-22A-R2.0, LS1043-23A-R1.0, LS1043-23A-R1.1,
> LS1046-26A-R1.0, LS1088-48A-R1.0, LS2080-40A-R1.0,
> LS2081A-R1.1, LS2085-45A-R1.0, LS2088-48A-R1.0,
> LS2088-48A-R1.1, LX2160-2120-2080A-R1.0.
> 
> Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
> ---
> Change in v4:
>                  Remove the point in "yinbo.zhu"

> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 5141856..20c9af4 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1521,6 +1521,18 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
>   			xhci_ring_cmd_db(xhci);
>   		}
>   
> +		/*
> +		 *erratum A-009668: Stop Endpoint Command does not complete.
> +		 *Workaround: Instead of issuing a Stop Endpoint Command,
> +		 *issue a Disable Slot Command with the corresponding slot ID.
> +		 *Alternately, you can issue an Address Device Command with
> +		 *BSR=1
> +		 */
> +		if ((urb->dev->speed <= USB_SPEED_HIGH) &&
> +					(xhci->quirks & XHCI_STOP_EP_IN_U1)) {
> +			xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
> +				urb->dev->slot_id);
> +		}

Address device in xHCI is done with a address device command on the command ring,
if it times out then xhci_handle_command_timeout() will be called. No stop endpoint command
should be involved (if I remember correctly)

xhci_urb_dequeue() may be called in other cases for LS/FS URBs that timed out,
due to your split transaction hang errata, but a disable slot command can't just be bluntly
issued before first properly stopping all slot endpoints, and giving back all the other pending URBs

-Mathias

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [v4,3/3] usb: host: Implement workaround for Erratum A-009668
@ 2018-01-04 14:58     ` Mathias Nyman
  0 siblings, 0 replies; 12+ messages in thread
From: Mathias Nyman @ 2018-01-04 14:58 UTC (permalink / raw)
  To: yinbo.zhu, Felipe Balbi, Greg Kroah-Hartman, Mathias Nyman,
	open list:DESIGNWARE USB3 DRD IP DRIVER, open list
  Cc: xiaobo.xie, jerry.huang, ran.wang_1

On 19.12.2017 12:16, yinbo.zhu@nxp.com wrote:
> From: yinbo.zhu <yinbo.zhu@nxp.com>
> 
> This issue is observed in USB 2.0 mode when the USB 3.0 host
> controller is connected to a FS/LS device via a hub. The
> host controller issues start-split (SSPLIT) and (CSPLIT)
> tokens to accomplish a split-transaction. A
> split-transaction consists of a SSPLIT token, token/data
> consists of a SSPLIT token, token/data packets, CSPLIT token
> and token/data/handshake packets. A SSPLIT token is issued
> by the host controller to the hub followed by token/data/
> handshake packets. The hub then relays the token/data/
> handshake packets to the FS /LS device. Sometime later, the
> host controller issues a CSPLIT token followed by the same
> token/data/handshake packets to the hub to complete the
> split-transaction. As an example scenario, when the xHCI
> driver issues an Address device command with BSR=0, the
> host controller sends SETUP(SET_ADDRESS) tokens on the USB
> as part of splittransactions. If the host controller
> receives a NYET response from the hub for the CSPLIT SETUP,
> it means that the split-transaction has not yet been
> completed or the hub is not able to handle the split
> transaction. In such a case, the host controller keeps
> retrying the splittransactions until such time an ACK
> response is received from the hub for the CSPLIT SETUP token
> . If the split-transactions do not complete in a time bound
> manner, the xHCI driver may issue a Stop Endpoint Command.
> The host controller does not service the Stop Endpoint
> Command and eventually the xHCI driver times out waiting for
> the Stop Endpoint Command to complete.
> 
> Impact: Stop Endpoint Command does not complete.
> Workaround: Instead of issuing a Stop Endpoint Command,
> issue a Disable Slot Command with the corresponding slot
> ID. Alternately, you can issue an Address Device Command
> with BSR=1.
> 
> Configs Affected:
> LS1012A-R1.0, LS1012A-R2.0, LS1021-20-22A-R1.0,
> LS1021-20-22A-R2.0, LS1043-23A-R1.0, LS1043-23A-R1.1,
> LS1046-26A-R1.0, LS1088-48A-R1.0, LS2080-40A-R1.0,
> LS2081A-R1.1, LS2085-45A-R1.0, LS2088-48A-R1.0,
> LS2088-48A-R1.1, LX2160-2120-2080A-R1.0.
> 
> Signed-off-by: yinbo zhu <yinbo.zhu@nxp.com>
> ---
> Change in v4:
>                  Remove the point in "yinbo.zhu"

> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 5141856..20c9af4 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1521,6 +1521,18 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
>   			xhci_ring_cmd_db(xhci);
>   		}
>   
> +		/*
> +		 *erratum A-009668: Stop Endpoint Command does not complete.
> +		 *Workaround: Instead of issuing a Stop Endpoint Command,
> +		 *issue a Disable Slot Command with the corresponding slot ID.
> +		 *Alternately, you can issue an Address Device Command with
> +		 *BSR=1
> +		 */
> +		if ((urb->dev->speed <= USB_SPEED_HIGH) &&
> +					(xhci->quirks & XHCI_STOP_EP_IN_U1)) {
> +			xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
> +				urb->dev->slot_id);
> +		}

Address device in xHCI is done with a address device command on the command ring,
if it times out then xhci_handle_command_timeout() will be called. No stop endpoint command
should be involved (if I remember correctly)

xhci_urb_dequeue() may be called in other cases for LS/FS URBs that timed out,
due to your split transaction hang errata, but a disable slot command can't just be bluntly
issued before first properly stopping all slot endpoints, and giving back all the other pending URBs

-Mathias
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-01-04 14:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-19 10:16 [PATCH v4 1/3] usb: host: Implement workaround for Erratum A-007463 yinbo.zhu
2017-12-19 10:16 ` [v4,1/3] " yinbo.zhu
2017-12-19 10:16 ` [PATCH v4 2/3] usb: host: Implement workaround for Erratum A-009611 yinbo.zhu
2017-12-19 10:16   ` [v4,2/3] " yinbo.zhu
2018-01-04 14:32   ` [PATCH v4 2/3] " Mathias Nyman
2018-01-04 14:32     ` [v4,2/3] " Mathias Nyman
2017-12-19 10:16 ` [PATCH v4 3/3] usb: host: Implement workaround for Erratum A-009668 yinbo.zhu
2017-12-19 10:16   ` [v4,3/3] " yinbo.zhu
2018-01-04 14:58   ` [PATCH v4 3/3] " Mathias Nyman
2018-01-04 14:58     ` [v4,3/3] " Mathias Nyman
2018-01-04 13:38 ` [PATCH v4 1/3] usb: host: Implement workaround for Erratum A-007463 Mathias Nyman
2018-01-04 13:38   ` [v4,1/3] " Mathias Nyman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.