[PATCH v3 1/3] usb: host: Implement workaround for Erratum A-007463

From: yinbo.zhu
Date: Tue Dec 19 2017 - 05:12:42 EST

From: yinbo.zhu <yinbo.zhu@xxxxxxx>

When a transaction error (defined in Section, "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
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
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.
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@xxxxxxx>
Change in v3:
Adjust the commit information.
Replace "(1<<32)" with BIT(32).
Update the DT properity description.
Update erratum description in code comment.
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,
+ dwc->quirk_reverse_in_out = device_property_read_bool(dev,
+ "snps,quirk_reverse_in_out");
device_property_read_u8(dev, "snps,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,
+ }
} 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)
/* For controller with a broken Port Disable implementation */
#define XHCI_BROKEN_PORT_PED (1 << 25)