[PATCH] xhci: Add new short TX quirk for Fresco Logic host.

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

 



Sergio reported that when he recorded audio from a USB headset mic
plugged into the USB 3.0 port on his ASUS N53SV-DH72, the audio sounded
"robotic".  When plugged into the USB 2.0 port under EHCI on the same
laptop, the audio sounded fine.  The device is:

Bus 002 Device 004: ID 046d:0a0c Logitech, Inc. Clear Chat Comfort USB Headset

The problem was tracked down to the Fresco Logic xHCI host controller
not correctly reporting short transfers on isochronous IN endpoints.
The driver would submit a 96 byte transfer, the device would only send
88 or 90 bytes, and the xHCI host would report the transfer had a
"successful" completion code, with an untransferred buffer length of 8
or 6 bytes.

The successful completion code and non-zero untransferred length is a
contradiction.  The xHCI host is supposed to only mark a transfer as
successful if all the bytes are transferred.  Otherwise, the transfer
should be marked with a short packet completion code.  Without the EHCI
bus trace, we wouldn't know whether the xHCI driver should trust the
completion code or the untransferred length.  With it, we know to trust
the untransferred length.

Add a new xHCI quirk for the Fresco Logic host controller.  If a
transfer is reported as successful, but the untransferred length is
non-zero, print a warning.  For the Fresco Logic host, change the
completion code to COMP_SHORT_TX and process the transfer like a short
transfer.

This should be backported to stable kernels that contain the commit
f5182b4155b9d686c5540a6822486400e34ddd98 "xhci: Disable MSI for some
Fresco Logic hosts."  That commit was marked for stable kernels as old
as 2.6.36.

Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
Reported-by: Sergio Correia <lists@xxxxxxxx>
Tested-by: Sergio Correia <lists@xxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---

Hi Sergio,

Can you double check that this patch works for you?  If so, I'll send it
off to Greg KH for the 3.5 merge window.

Also, I figured out the buggy Fresco Logic chipset I was thinking of was
rev 0x0, not rev 0x4, so I don't think you have a pre-release xHCI
chipset.

Let me know if you find any other issues under this host controller.
This chipset is known to have several different quirks, and I'm not sure
we handle them all in Linux.

Sarah Sharp

 drivers/usb/host/xhci-pci.c  |    1 +
 drivers/usb/host/xhci-ring.c |   20 +++++++++++++++++---
 drivers/usb/host/xhci.h      |    1 +
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 7a856a7..19e8921 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -72,6 +72,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 		xhci_dbg(xhci, "QUIRK: Fresco Logic revision %u "
 				"has broken MSI implementation\n",
 				pdev->revision);
+		xhci->quirks |= XHCI_TRUST_TX_LENGTH;
 	}
 
 	if (pdev->vendor == PCI_VENDOR_ID_NEC)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 3d9422f..951a9c3 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1786,8 +1786,12 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	/* handle completion code */
 	switch (trb_comp_code) {
 	case COMP_SUCCESS:
-		frame->status = 0;
-		break;
+		if (TRB_LEN(le32_to_cpu(event->transfer_len)) == 0) {
+			frame->status = 0;
+			break;
+		}
+		if ((xhci->quirks & XHCI_TRUST_TX_LENGTH))
+			trb_comp_code = COMP_SHORT_TX;
 	case COMP_SHORT_TX:
 		frame->status = td->urb->transfer_flags & URB_SHORT_NOT_OK ?
 				-EREMOTEIO : 0;
@@ -1883,13 +1887,16 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	switch (trb_comp_code) {
 	case COMP_SUCCESS:
 		/* Double check that the HW transferred everything. */
-		if (event_trb != td->last_trb) {
+		if (event_trb != td->last_trb ||
+				TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
 			xhci_warn(xhci, "WARN Successful completion "
 					"on short TX\n");
 			if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
 				*status = -EREMOTEIO;
 			else
 				*status = 0;
+			if ((xhci->quirks & XHCI_TRUST_TX_LENGTH))
+				trb_comp_code = COMP_SHORT_TX;
 		} else {
 			*status = 0;
 		}
@@ -2048,6 +2055,13 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 	 * transfer type
 	 */
 	case COMP_SUCCESS:
+		if (TRB_LEN(le32_to_cpu(event->transfer_len)) == 0)
+			break;
+		if (xhci->quirks & XHCI_TRUST_TX_LENGTH)
+			trb_comp_code = COMP_SHORT_TX;
+		else
+			xhci_warn(xhci, "WARN Successful completion on short TX: "
+					"needs XHCI_TRUST_TX_LENGTH quirk?\n");
 	case COMP_SHORT_TX:
 		break;
 	case COMP_STOP:
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 3d69c4b..abfb321 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1479,6 +1479,7 @@ struct xhci_hcd {
 #define XHCI_RESET_ON_RESUME	(1 << 7)
 #define	XHCI_SW_BW_CHECKING	(1 << 8)
 #define XHCI_AMD_0x96_HOST	(1 << 9)
+#define XHCI_TRUST_TX_LENGTH	(1 << 10)
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
 	/* There are two roothubs to keep track of bus suspend info for */
-- 
1.7.9

--
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