[PATCH 1/1] usb: xhci: Handle USB transaction error on address command

[PATCH 1/1] usb: xhci: Handle USB transaction error on address command

[Lu Baolu]

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@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/usb/host/xhci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b2ff1ff..9cc56cd 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3836,6 +3836,12 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 		ret = -EINVAL;
 		break;
 	case COMP_USB_TRANSACTION_ERROR:
+		xhci_free_virt_device(xhci, udev->slot_id);
+		ret = xhci_disable_slot(xhci, command, udev->slot_id);
+		udev->slot_id = 0;
+		if (!ret)
+			xhci_alloc_dev(hcd, udev);
+
 		dev_warn(&udev->dev, "Device not responding to setup %s.\n", act);
 		ret = -EPROTO;
 		break;
-- 
2.7.4

Re: [PATCH 1/1] usb: xhci: Handle USB transaction error on address command

[Lu Baolu]

Hi,

On 07/26/2017 01:11 PM, Xing, Zhengjun wrote:
>
>
> On 7/25/2017 1:09 PM, 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@linux.intel.com>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   drivers/usb/host/xhci.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index b2ff1ff..9cc56cd 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -3836,6 +3836,12 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
>>           ret = -EINVAL;
>>           break;
>>       case COMP_USB_TRANSACTION_ERROR:
>> +        xhci_free_virt_device(xhci, udev->slot_id);
>
>  In xhci_free_virt_device  xhci->devs[slot_id]will be set to NULL.
>
>> +        ret = xhci_disable_slot(xhci, command, udev->slot_id);
>
> When xhci_disable_slot check xhci->devs[slot_id] is NULL, just return -EINVAL, the slot will not be disabled.

Yes, really.

I don't think xhci_disable_slot() should return directly when
the virtual device structure is not allocated. This function
is also used to issue a disable slot command when the
virtual device data allocation fails in xhci_alloc_dev().

I will develop v2 patch with a fix for xhci_disable_slot().

Best regards,
Lu Baolu