Subject: Re: [PATCH v4 3/3] usb: host: Implement workaround for Erratum
 A-009668
To: yinbo.zhu@nxp.com, Felipe Balbi <balbi@ti.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Mathias Nyman <mathias.nyman@intel.com>,
        "open list:DESIGNWARE USB3 DRD IP DRIVER" <linux-usb@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Cc: xiaobo.xie@nxp.com, jerry.huang@nxp.com, ran.wang_1@nxp.com
References: <20171219101618.45650-1-yinbo.zhu@nxp.com>
 <20171219101618.45650-3-yinbo.zhu@nxp.com>
From: Mathias Nyman <mathias.nyman@linux.intel.com>
Message-ID: <3b552726-6be3-cbb1-4af0-b50ceb3297a9@linux.intel.com>
Date: Thu, 4 Jan 2018 16:58:10 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: <20171219101618.45650-3-yinbo.zhu@nxp.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org

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