Re: [PATCH] usb, xhci, rt2800usb: do not perform Soft Retry

From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Stanislaw Gruszka <stf_xl@wp.pl>, Greg KH <greg@kroah.com>
Cc: linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org,
	Mathias Nyman <mathias.nyman@intel.com>,
	Bernhard <bernhard.gebetsberger@gmx.at>
Subject: Re: [PATCH] usb, xhci, rt2800usb: do not perform Soft Retry
Date: Fri, 22 Jan 2021 17:00:21 +0200	[thread overview]
Message-ID: <eb37b28d-5046-f0cd-92ee-55af0e350802@linux.intel.com> (raw)
In-Reply-To: <20210122132650.GA13029@wp.pl>

On 22.1.2021 15.26, Stanislaw Gruszka wrote:
> On Fri, Jan 22, 2021 at 12:56:30PM +0100, Greg KH wrote:
>> On Fri, Jan 22, 2021 at 11:43:42AM +0100, stf_xl@wp.pl wrote:
>>> From: Stanislaw Gruszka <stf_xl@wp.pl>
>>>
>>> Since f8f80be501aa ("xhci: Use soft retry to recover faster from transaction
>>> errors") on some systems rt2800usb devices are unable to operate. Looks
>>> that due to firmware or hardware limitations of those devices, they
>>> require full recovery from USB Transaction Errors.
>>>
>>> To avoid the problem add URB transfer flag, that restore pre f8f80be501aa
>>> xhci behaviour when the flag is set. For now only add it only to rt2800usb
>>> driver.
>>
>> This feels like a really heavy hammer, to add a xhci flag for a single
>> broken device.

I agree, rootcause is still unknown.
This bug hasn't gotten enough attention.

I'm tidying up a rewrite of areas that touches this, but it didn't seem to help.
I'd anyway like to get the rewrite done first, then get a new set of logs and traces,
and take a fresh look at this

Meanwhile it could be checked if this issue is seen only on some xHCI controllers.
Maybe some vendors don't support soft retry properly.
we could easily prevent soft retry usage on those xHC hosts. 

>>
>> Are you sure this is really needed?
> 
> I'm not sure if this is needed, however this particular bug was reported
> as regression caused by f8f80be501aa commit on 4.19 -> 4.20 cycle. It
> was reported to Mathias Nyman - xhci maintainer and f8f80be501aa commit
> author. But since then, regardless of some Mathias work on this on xhci
> side, we did not get solution that fixed the problem.
> 
> From other side, I can ask why change from f8f80be501aa is need? Taking
> it's commit message, the benefit of the change is not obvious. What
> I can notice, is that it only broke support for hardware that worked
> previously. However I assume that the change is useful and needed,
> so I come up with patch that just revert this change only for rt2800usb.

Significantly faster recovery from transaction errors. Many errors are temporary
due to electrical interference, and a simple retry will solve the case.
see xhci spec section 4.6.8.1 "Soft Retry" for details.

> 
>>  What does this device do on other
>> operating systems, do they have such a quirk for their host controller
>> driver?
> 
> I don't know what other OSes do.
> 
>> Or is this due to the specific host controller device hardware?  Should
>> this be a xhci quirk for a specific pci device instead?

Exactly, this should be checked.
Stanislaw, weren't there a few users already that saw this issue?
Do we know what xHCI controllers they were using?

-Mathias