Hello Sarah,
On Wed, May 9, 2012 at 11:54 AM, Sarah Sharp
<sarah.a.sharp@xxxxxxxxxxxxxxx> wrote:
> 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.
>
Double-tested, with this patch applied it works as it should. Thanks
again for your help!
> 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.
>
Glad to hear :)
> 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.
>
I sure will, thanks again.
Sergio
> 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
--
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
