On 2012-04-17 10:56, Michael Grzeschik wrote: > 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. ;-) > Ok, I hear you. When you are done with the reworked "check for every urb in queue" patch I'd like to do some testing of your patches before acking them. :) (I'll be on a conference trip thursday-sunday so I won't be able to test during that time though.) -- Arvid Brodin Enea Services Stockholm AB - since February 16 a part of Xdin in the Alten Group. Soon we will be working under the common brand name Xdin. Read more at www.xdin.com.-- 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
