[PATCH] Add Etron XHCI quirk to avoid warning spam

[PATCH] Add Etron XHCI quirk to avoid warning spam

[Daniel J Blueman]

When various USB3 devices with Etron XHCI controllers, we see a bunch of
warnings:
xhci_hcd 0000:02:00.0: WARN Successful completion on short TX: needs
XHCI_TRUST_TX_LENGTH quirk?

Acknowledge the issue by adding the quirk.

Signed-off-by: Daniel J Blueman <daniel@quora.org>
---
 drivers/usb/host/xhci-pci.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 18b231b..715ad11 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -95,10 +95,13 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 		xhci->limit_active_eps = 64;
 		xhci->quirks |= XHCI_SW_BW_CHECKING;
 	}
-	if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
-			pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
-		xhci->quirks |= XHCI_RESET_ON_RESUME;
-		xhci_dbg(xhci, "QUIRK: Resetting on resume\n");
+	if (pdev->vendor == PCI_VENDOR_ID_ETRON) {
+		xhci->quirks |= XHCI_TRUST_TX_LENGTH;
+
+		if (pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
+			xhci->quirks |= XHCI_RESET_ON_RESUME;
+			xhci_dbg(xhci, "QUIRK: Resetting on resume\n");
+		}
 	}
 	if (pdev->vendor == PCI_VENDOR_ID_VIA)
 		xhci->quirks |= XHCI_RESET_ON_RESUME;
-- 
1.7.9.5

Re: [PATCH] Add Etron XHCI quirk to avoid warning spam

[Sarah Sharp]

Hi Daniel,

I already have a patch in my queue for this.  However, it keys off the
PCI_DEVICE_ID_ASROCK_P67 PCI device ID.  Do you have another Etron
device with a different device ID that needs this quirk?

Sarah Sharp

On Fri, Jul 27, 2012 at 12:03:44PM +0800, Daniel J Blueman wrote:
> When various USB3 devices with Etron XHCI controllers, we see a bunch of
> warnings:
> xhci_hcd 0000:02:00.0: WARN Successful completion on short TX: needs
> XHCI_TRUST_TX_LENGTH quirk?
> 
> Acknowledge the issue by adding the quirk.
> 
> Signed-off-by: Daniel J Blueman <daniel@quora.org>
> ---
>  drivers/usb/host/xhci-pci.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 18b231b..715ad11 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -95,10 +95,13 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>  		xhci->limit_active_eps = 64;
>  		xhci->quirks |= XHCI_SW_BW_CHECKING;
>  	}
> -	if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
> -			pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
> -		xhci->quirks |= XHCI_RESET_ON_RESUME;
> -		xhci_dbg(xhci, "QUIRK: Resetting on resume\n");
> +	if (pdev->vendor == PCI_VENDOR_ID_ETRON) {
> +		xhci->quirks |= XHCI_TRUST_TX_LENGTH;
> +
> +		if (pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
> +			xhci->quirks |= XHCI_RESET_ON_RESUME;
> +			xhci_dbg(xhci, "QUIRK: Resetting on resume\n");
> +		}
>  	}
>  	if (pdev->vendor == PCI_VENDOR_ID_VIA)
>  		xhci->quirks |= XHCI_RESET_ON_RESUME;
> -- 
> 1.7.9.5
> 

Re: [PATCH] Add Etron XHCI quirk to avoid warning spam

[Daniel J Blueman]

On 27 July 2012 14:14, Sarah Sharp <sarah.a.sharp@linux.intel.com> wrote:
> On Fri, Jul 27, 2012 at 12:03:44PM +0800, Daniel J Blueman wrote:
>> When various USB3 devices with Etron XHCI controllers, we see a bunch of
>> warnings:
>> xhci_hcd 0000:02:00.0: WARN Successful completion on short TX: needs
>> XHCI_TRUST_TX_LENGTH quirk?
>>
>> Acknowledge the issue by adding the quirk.
>>
>> Signed-off-by: Daniel J Blueman <daniel@quora.org>
[]
> I already have a patch in my queue for this.  However, it keys off the
> PCI_DEVICE_ID_ASROCK_P67 PCI device ID.  Do you have another Etron
> device with a different device ID that needs this quirk?

Yes, the subsystem ID is different [1] (but Zotac program it the same
as the vendor and device IDs here), however what you say suggests the
problem is general to this Etron XHCI controller (1b6f:7023), as we'd
suspect anyway.

Thus the more general patch I posted makes better sense perhaps?

Thanks,
  Daniel

--- [1]

# lspci -vs 02:00.0
02:00.0 USB controller: Etron Technology, Inc. EJ168 USB 3.0 Host
Controller (rev 01) (prog-if 30 [XHCI])
	Subsystem: Etron Technology, Inc. EJ168 USB 3.0 Host Controller
[]
# lspci -vns 02:00.0
02:00.0 0c03: 1b6f:7023 (rev 01) (prog-if 30 [XHCI])
	Subsystem: 1b6f:7023
-- 
Daniel J Blueman

Re: [PATCH] Add Etron XHCI quirk to avoid warning spam

[Sarah Sharp]

On Fri, Jul 27, 2012 at 02:40:56PM +0800, Daniel J Blueman wrote:
> On 27 July 2012 14:14, Sarah Sharp <sarah.a.sharp@linux.intel.com> wrote:
> > On Fri, Jul 27, 2012 at 12:03:44PM +0800, Daniel J Blueman wrote:
> >> When various USB3 devices with Etron XHCI controllers, we see a bunch of
> >> warnings:
> >> xhci_hcd 0000:02:00.0: WARN Successful completion on short TX: needs
> >> XHCI_TRUST_TX_LENGTH quirk?
> >>
> >> Acknowledge the issue by adding the quirk.
> >>
> >> Signed-off-by: Daniel J Blueman <daniel@quora.org>
> []
> > I already have a patch in my queue for this.  However, it keys off the
> > PCI_DEVICE_ID_ASROCK_P67 PCI device ID.  Do you have another Etron
> > device with a different device ID that needs this quirk?
> 
> Yes, the subsystem ID is different [1] (but Zotac program it the same
> as the vendor and device IDs here), however what you say suggests the
> problem is general to this Etron XHCI controller (1b6f:7023), as we'd
> suspect anyway.
> 
> Thus the more general patch I posted makes better sense perhaps?

I'd really like to keep this quirk specific to the particular PCI vendor
and device ID.  It's possible that their next chip version will have the
opposite issue (short TX completion code and bad untransferred length).

Your patch turned it on for all Etron hosts, so I would rather keep my
version:

http://git.kernel.org/?p=linux/kernel/git/sarah/xhci.git;a=commit;h=12751f75720391bb2b607acdb2537f02e313251e

I also added a second patch to rate limit the messages for other hosts
that have this issue but don't have the quirk turned on:

http://git.kernel.org/?p=linux/kernel/git/sarah/xhci.git;a=commit;h=5de3270101d30a4a8023815f8010eef2d2d40613

Sarah Sharp

Re: [PATCH] Add Etron XHCI quirk to avoid warning spam

[Daniel J Blueman]

On 28 July 2012 01:10, Sarah Sharp <sarah.a.sharp@linux.intel.com> wrote:
> On Fri, Jul 27, 2012 at 02:40:56PM +0800, Daniel J Blueman wrote:
>> On 27 July 2012 14:14, Sarah Sharp <sarah.a.sharp@linux.intel.com> wrote:
>> > On Fri, Jul 27, 2012 at 12:03:44PM +0800, Daniel J Blueman wrote:
>> >> When various USB3 devices with Etron XHCI controllers, we see a bunch of
>> >> warnings:
>> >> xhci_hcd 0000:02:00.0: WARN Successful completion on short TX: needs
>> >> XHCI_TRUST_TX_LENGTH quirk?
>> >>
>> >> Acknowledge the issue by adding the quirk.
>> >>
>> >> Signed-off-by: Daniel J Blueman <daniel@quora.org>
>> []
>> > I already have a patch in my queue for this.  However, it keys off the
>> > PCI_DEVICE_ID_ASROCK_P67 PCI device ID.  Do you have another Etron
>> > device with a different device ID that needs this quirk?
>>
>> Yes, the subsystem ID is different [1] (but Zotac program it the same
>> as the vendor and device IDs here), however what you say suggests the
>> problem is general to this Etron XHCI controller (1b6f:7023), as we'd
>> suspect anyway.
>>
>> Thus the more general patch I posted makes better sense perhaps?
>
> I'd really like to keep this quirk specific to the particular PCI vendor
> and device ID.  It's possible that their next chip version will have the
> opposite issue (short TX completion code and bad untransferred length).
>
> Your patch turned it on for all Etron hosts, so I would rather keep my
> version:
>
> http://git.kernel.org/?p=linux/kernel/git/sarah/xhci.git;a=commit;h=12751f75720391bb2b607acdb2537f02e313251e
[]

Ok, the patch is correct since PCI_DEVICE_ID_ASROCK_P67 evaluates to
0x7023, which is the Etron EJ168 device ID.

Board-specific IDs (as the definition name suggests) are used for the
subsystem IDs, so this name is misleading (and misled me at least).

Can you fix this up with a patch to change PCI_DEVICE_ID_ASROCK_P67 to
PCI_DEVICE_ID_ETRON_EJ168, else I can cook and test a patch?

Thanks,
  Daniel
-- 
Daniel J Blueman