Hi Arvid, On Mon, Apr 16, 2012 at 05:11:25PM +0000, Arvid Brodin wrote: > On 2012-04-13 16:39, Michael Grzeschik wrote: > > Hi Arvid, > > > > On Fri, Apr 13, 2012 at 01:58:25PM +0000, Arvid Brodin wrote: > >> On 2012-04-13 12:59, Michael Grzeschik wrote: > >>> The errata2 workaround is described in the Datasheet as follows: > >>> > >>> An SOF INT can be used instead of an ATL INT with polling on Done > >>> bits. A time-out can be implemented and if a certain Done bit is never > >>> set, verification of the PTD completion can be done by reading PTD > >>> contents (valid bit). This is a proven workaround implemented in > >>> software. > >>> > >>> Therefore, we should do so, and ignore the check > >>> for the ACTIVE_BIT in DW3. Because it is possible > >>> that this bit is still set when the issue accures. > >> > >> I believe you trust the datasheet too much! :) I've tried this before (in fact, most of > >> your code seems to be from an earlier patch of mine). It doesn't work, as explained in the > >> comment to errata2_function() (and as tested with the usbtest/testusb suite). > > > > Yes, i know the older patches struggling with that issue. But in both > > cases there were some details which does not fit to the exact errata2 > > fix description. > > > > In the first patch (USB: isp1760: Implement solution for erratum 2) > > the content checking (Valid Bit: 0) was totaly left out of sight. > > > > The second patch (usb/isp1760: Use polling instead of SOF interrupts to > > fix Errata 2) was checking too much. As described above, the check for > > the ACTIVE_BIT must not be done. > > > > In the current implementation we still run into hanging unhandled urbs > > because they sometimes doesn't get both bits toggled. (V==0 && A==0) > > > > After solving this, it is easy to change the polling function back to > > always enabled interrupts. In fact the enabling of SOF Interrupts > > doesn't have anything else in mind than to guarantee that we never run > > out of interrupts. That actually is nothing else then polling in > > hardware. Instead of doing it with a timer function. > > Aha, I missed the removal of the ACTIVE_BIT==0 check in the patch. Nice find! > > So your patch is really doing two things: > > 1) Remove the ACTIVE_BIT==0 check - this fixes the bug you are hitting. Great! > 2) Switch from software timer checking at ~5 Hz to SOF interrupt checking at, was it 1 > kHz? Is this really neccessary/useful? Yes, because with some high rated transfers going via the host controller, especially in the case of having many slots queued at once, the issue can trigger actually a lot. Its better not to wait that long time every time that issue happens. This can run that host controller actually worthless and not worth calling "high speed" capable. ;-) Regards, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- 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
