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
