[RFT PATCH v1 1/5] Revert "usb: ehci-hcd: Keep async schedule running"

[Marek Vasut]

On 3/24/20 7:11 PM, Lukasz Majewski wrote:
> Hi Marek,

Hi,

>> On 3/24/20 8:06 AM, Lukasz Majewski wrote:
>>> Hi Marek,  
>>
>> Hi,
>>
>> [...]
>>
>>>>>> You should probably figure out why this doesn't work first and
>>>>>> then add fixes on top.    
>>>>>
>>>>> Haven't you seen such problem during code development on your
>>>>> setup when developing this patch?     
>>>>
>>>> During the development of the patch, I don't remember, sorry. I
>>>> most certainly saw various failure modes, however those should not
>>>> be present mainline.  
>>>
>>> The issue is that the qhtoken is not updated at all. 
>>>
>>> Maybe you remember - is Linux using async setup by default (as
>>> introduced in SHA1: 02b0e1a36c5bc20174299312556ec4e266872bd6) ?  
>>
>> If I recall correctly, it is using async schedule for bulk transfers.
>> But the code is available, so you can double-check that.
>>
>>>> I tested this patch with the problematic USB sticks on R-Car Gen3
>>>> and with SMSC95xx USB ethernet adapter last weekend and I didn't
>>>> see a problem.  
>>>
>>> Ok.
>>>
>>> For i.MX6Q:
>>> The SHA1: 02b0e1a36c5bc20174299312556ec4e266872bd6 patch causes the
>>> iMX6Q to fail after a few minutes of testing. General in i.MX6Q the
>>> usb is NOT robust at all.
>>>
>>> For i.MX53:
>>> With patch 02b0e1a36c5bc20174299312556ec4e266872bd6 applied it also
>>> breaks after a few minutes.  
>>
>> So on CI HDRC , there is some difference in behavior. That is what you
>> need to find and fix then.
> 
> The conclusion is that some boards/implementations are broken.

At least systems with CI HDRC.

>>> With this patch series applied it works for 2 days now without any
>>> issue.  
>>
>> Except performance is totally degraded
> 
> So we do have _very_ fast USB which breaks after a few minutes of
> constant testing (with procedure stated earlier).

No, we have a controller which manifests some problem and that problem
needs to be identified and fixed. Whether it's in the stack or in the
controller driver is to be seen.

>> and there is still no clear
>> explanation _why_ any of these patches are needed
> 
> Haven't I explicitly explained in previous mails why XACTARR error shall
> be handled? Nor the original thread did it? Wasn't the cover-letter
> verbose enough?

Regarding XACTERR, I agree it should be handled somehow.

However, I don't think handling XACTERR is what fixes your problems with
the USB sticks, is it ?

Also, it is still unclear whether handling XACTERR exactly the same as
STALL is the right thing to do. Is it ? I think it's not.

>> and/or whether doing
>> write to a block device with these patches may cause data corruption.
> 
> So I will ask differently - what _may_ happen when the "TD -
> token=XXXX" error shows up and the board hangs? Wouldn't we risk some
> unwanted storage corruption?

The behavior is undefined.

-- 
Best regards,
Marek Vasut