[PATCH 1/2] usb: host: Implement workaround for Erratum A-009611

[PATCH 1/2] usb: host: Implement workaround for Erratum A-009611

[yinbo.zhu]

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

Description: 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 FIFO 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.

Signed-off-by: yinbo.zhu <yinbo.zhu@nxp.com>
---
 drivers/usb/host/xhci.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2d6a98c1dc12..35f7821bc8b2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1501,13 +1501,22 @@ 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 +
-			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);
+
+		/*
+		 *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) {
+			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);
+		}
 	}
 done:
 	spin_unlock_irqrestore(&xhci->lock, flags);
-- 
2.14.1

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

[Greg Kroah-Hartman]

On Mon, Sep 11, 2017 at 05:41:32PM +0800, yinbo.zhu@nxp.com wrote:
> From: "yinbo.zhu" <yinbo.zhu@nxp.com>

Please use your "real" name in the from portion, I don't think you have
a "." in your name, right?  :)

And what hardware is this erratum for?  Please put that in the subject
line as well, as well as xhci, otherwise it is very vague.

> 
> Description: This is a occasional problem where the software

No need for "Description:" in a changelog text, the whole thing is a
description.

> 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 FIFO 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.
> 
> Signed-off-by: yinbo.zhu <yinbo.zhu@nxp.com>
> ---
>  drivers/usb/host/xhci.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 2d6a98c1dc12..35f7821bc8b2 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1501,13 +1501,22 @@ 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 +
> -			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);
> +
> +		/*
> +		 *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.
> +		 */

Please properly format your comments.

And what is the A-009611: for?

And is this really for all xhci controllers?

> +		if (ep_ring->enqueue == ep_ring->dequeue) {
> +			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);
> +		}
>  	}


thanks,

greg k-h

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

[Mathias Nyman]

On 11.09.2017 12:41, yinbo.zhu@nxp.com wrote:
> From: "yinbo.zhu" <yinbo.zhu@nxp.com>
>
> Description: 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 FIFO 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.

This doesn't seem like the correct way to solve the issue.
This workaround will prevent canceling URBs.

we need to stop the endpoint ring before we can touch the TRBs on the ring.
If we want to cancel a URB we need to either hop over its TRBs on the ring,
or turn them to no-ops. The canceled URB is only given back after this.

Imagine a USB camera queuing ~100 TRBs in advance, then user wants to change video
mode and cancel the pending URBs.
We would have to wait for the camera to finish these 100TRBs before doing anything.

Queuing a stop endpoint command is essential to canceling a URB, and we can't
wait for the ring to be empty before issuing it.

-Mathias