On 16.08.2017 05:15, Lu Baolu wrote:
Hi,
On 08/15/2017 07:30 PM, Mathias Nyman wrote:
On 11.08.2017 05:41, Lu Baolu wrote:
Xhci driver handles USB transaction errors on transfer events,
but transaction errors are possible on address device command
completion events as well.
The xHCI specification (section 4.6.5) says: A USB Transaction
Error Completion Code for an Address Device Command may be due
to a Stall response from a device. Software should issue a Disable
Slot Command for the Device Slot then an Enable Slot Command to
recover from this error.
This patch handles USB transaction errors on address command
completion events. The related discussion threads can be found
through below links.
http://marc.info/?l=linux-usb&m=149362010728921&w=2
http://marc.info/?l=linux-usb&m=149252752825755&w=2
Suggested-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
---
drivers/usb/host/xhci.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d6b728d..95780f8 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3822,6 +3822,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
break;
case COMP_USB_TRANSACTION_ERROR:
dev_warn(&udev->dev, "Device not responding to setup %s.\n", act);
+
+ ret = xhci_disable_slot(xhci, udev->slot_id);
+ if (!ret)
+ xhci_alloc_dev(hcd, udev);
Might be a xhci->mutex locking issue here,
both xhci_setup_device() and xhci_alloc_dev() take xhci->mutex
I will apply xhci->mutex in this patch for code consistency, but I think
we can drop xhci->mutex (in a separated patch) anyway.
xhci->mutex was introduced to protect two race sources of xhci->slot_id
and xhci->addr_dev by below commit:
commit a00918d0521df1c7a2ec9143142a3ea998c8526d
usb: host: xhci: add mutex for non-thread-safe data
c2d3d49 usb: xhci: move slot_id from xhci_hcd to xhci_command structure
87e44f2 usb: xhci: remove the use of xhci->addr_dev
So we don't need xhci->mutex any more. I will try to do this in a separated
patch with more tests. For now, I will add xhci->mutex for code consistency.
Fixing xhci->mutex sounds like a good idea, and you are right, it's no longer
needed for slot_id or addr_dev.
But it's use was extended to protect xhci_stop() and xhci_setup_device()
from racing at fast xhci host hotplug/removes in this patch.
commit 85ac90f8953a58f6a057b727bc9db97721e3fb8e
Author: Roger Quadros <rogerq@xxxxxx>
Date: Mon Sep 21 17:46:12 2015 +0300
usb: xhci: lock mutex on xhci_stop
Else it races with xhci_setup_device
But I think it's safe to remove xhci->mutex from xhci_alloc_dev()
There's no huurry with the patch 5/5 so would be nice to get that mutex
cleaned up before.
Thanks
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
