Re: [PATCH v3] usb: xhci: Issue stop EP command only when the EP state is running

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

 



On 12.07.2017 08:30, Shyam Sundar S K wrote:


On 6/22/2017 4:34 PM, Mathias Nyman wrote:
On 22.06.2017 13:44, Shyam Sundar S K wrote:

On 6/19/2017 9:42 AM, Shyam Sundar S K wrote:
on AMD platforms with SNPS 3.1 USB controller if stop endpoint command is
issued the controller does not respond, when the EP is not in running
state. HW completes the command execution and reports
"Context State Error" completion code. This is as per the spec. However
HW on receiving the second command additionally marks EP to Flow control
state in HW which is RTL bug. This bug causes the HW not to respond
to any further doorbells that are rung by the driver. This makes the EP
to not functional anymore and causes gross functional failures.

As a workaround, not to hit this problem, its better we check the EP state
and issue the stop EP command only when the EP is in running state.

Also, with the inclusion of this patch, there maybe a possible race
condition that may get triggered as mentioned in the xHCI spec 4.6.9

Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
Signed-off-by: Nehal Shah <Nehal-bakulchandra.Shah@xxxxxxx>
---
   drivers/usb/host/xhci-hub.c | 30 +++++++++++++++++++++---------
   1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0dde49c..d39b1e0 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -398,17 +398,29 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
       spin_lock_irqsave(&xhci->lock, flags);
       for (i = LAST_EP_INDEX; i > 0; i--) {
           if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
-            struct xhci_command *command;
-            command = xhci_alloc_command(xhci, false, false,
-                             GFP_NOWAIT);
-            if (!command) {
-                spin_unlock_irqrestore(&xhci->lock, flags);
-                xhci_free_command(xhci, cmd);
-                return -ENOMEM;

+            /* Check endpoint is running before stopping it,
+             * AMD SNPS 3.1 xHC may misbehave otherwise
+             */
+
+            struct xhci_ep_ctx *ep_ctx;
+
+            ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, i);
+
+            if (GET_EP_CTX_STATE(ep_ctx) ==  EP_STATE_RUNNING) {
+                struct xhci_command *command;
+
+                command = xhci_alloc_command(xhci, false, false,
+                                 GFP_NOWAIT);
+                if (!command) {
+                    spin_unlock_irqrestore(&xhci->lock,
+                                   flags);
+                    xhci_free_command(xhci, cmd);
+                    return -ENOMEM;
+                }
+                xhci_queue_stop_endpoint(xhci, command,
+                             slot_id, i, suspend);
               }
-            xhci_queue_stop_endpoint(xhci, command, slot_id, i,
-                         suspend);
           }
       }
       xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
Hi Mathias,

I think, all your recommendations were taken care in the above patch. Any feedback on this ?


Looks good so far,
I'll start looking/applying/testing it more after the midsummer festivities.
I'll let you know if any new issues come up, otherwise I'll just apply it and send
it forward

Hi Mathias,

Any updates on this ?


Added a simplified version of this to my for-usb-linus branch:

https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-linus&id=af58c6348ce66860b45ae14391df8511bea5904d

I'll run some quick smoketests on that branch, then I'll send it forwared to 4.13-rcx

-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