On Thu, Aug 11, 2011 at 07:27:15PM +0200, Daniel Mack wrote: > On Thu, Aug 11, 2011 at 6:45 PM, Sarah Sharp > <sarah.a.sharp@xxxxxxxxxxxxxxx> wrote: > > On Thu, Aug 11, 2011 at 02:57:41AM +0200, Daniel Mack wrote: > >> On Thu, Aug 11, 2011 at 1:15 AM, Sarah Sharp > >> <sarah.a.sharp@xxxxxxxxxxxxxxx> wrote: > >> > On Wed, Aug 10, 2011 at 05:33:02PM +0200, Daniel Mack wrote: > >> >> On 08/10/2011 04:32 PM, Alan Stern wrote: > >> >> >Looking at the driver's current code, it appears that your patch > >> >> >does not fix the bug properly. Using discontiguous regions in the > >> >> >transfer buffer is perfectly okay. The real problem is later on, > >> >> >where you do: > >> >> > > >> >> >if (send_it) { out->number_of_packets = FRAMES_PER_URB; > >> >> > > >> >> >This should be > >> >> > > >> >> >out->number_of_packets = outframe; > >> >> > > >> >> >The way it is now, the USB stack will try to use data from all the > >> >> >frame descriptors, and the last few will be stale because the loop > >> >> >doesn't set them. > >> >> > >> >> That's actually true, even though it doesn't seem to cause any trouble. > >> >> I tested everything here of course, and the output URBs return back from > >> >> the USB stack with their length fields zeroed out, which then > >> >> causes the stack to send packets with zero-length fields at the end. > >> > > >> > Actually, it causes system hangs when the driver is loaded on a device > >> > attached to a USB 3.0 port, as Alan Stern pointed out: > >> > > >> > https://bugzilla.kernel.org/show_bug.cgi?id=40702 > >> > >> Yes, I've noticed this. > >> > >> > Please don't submit zero-length transfers. The xHCI driver just isn't > >> > able to handle it. Arguably, it probably should have just rejected your > >> > URB when it found a zero length buffer, so I'll probably be submitting a > >> > patch to fix that. > >> > >> According to the spec, sending zero-length frames should be fine, no? > >> Is there any particular reason why XCHI can't handle this while EHCI > >> can? And does my patch fix the driver for XHCI? > > > > Ok, yes, you're correct that the xHCI spec allows the transfer length to > > be set to zero. In the case where the frame buffer is zero-length, is > > the buffer pointer still valid? It's not clear from the spec whether it > > needs to be. > > Well, the buffer pointer is set in the URB, not in its individual iso > subframes which just denotes them via the offset field. So yes, it is > valid in my case. But it doesn't matter anymore, as the code which > does that is now gone :) Do you mean it was removed with this patch: http://git.kernel.org/?p=linux/kernel/git/tiwai/sound-2.6.git;a=commitdiff;h=15439b Because according to Matej, he applied that patch, plus my patch to reject zero-length buffers[1], and he saw debugging that indicated he *did* see zero-length buffers. Is there any chance your driver might submit a zero-length buffer in the middle of the isochronous URB transfer array? Sarah Sharp [1] http://git.kernel.org/?p=linux/kernel/git/sarah/xhci.git;a=commitdiff;h=e70d79bda63050729fcef654167d22eb59380aa6 -- 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