Re: [PATCH v4 2/3] usb: host: Implement workaround for Erratum A-009611

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19.12.2017 12:16, yinbo.zhu@xxxxxxx wrote:
From: yinbo.zhu <yinbo.zhu@xxxxxxx>

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@xxxxxxx>
---
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux