On Fri, 29 Jun 2012, Hans de Goede wrote: > Too soon. As said I still needed to test the patch with isoc > devices, and it failed. It turns out most controllers don't > handle urb->sg for isoc transfers! I've checked the ehci code, > and it certainly does not. Ah, yes. We never envisioned terribly large isochronous transfers, since they naturally need to be broken up by time period anyway. Only bulk -- although it was just as easy to add SG support for interrupt at the same time. > If you look at ehci-sched.c: itd_sched_init() it > unconditionally uses urb->transfer_dma. I've not checked the > xhci code, but I got similar (non working) results during > testing there. > > This may very well be by design, but when writing the devio.c > patch I thought it would be best to split all large-ish > transfers, abnd that turns out to not be a good idea :) Yeah, isochronous transfers shouldn't be broken up this way. > This is not really a problem since large isoc urbs can easily > be split by userspace by simply using less iso-packets / urb, > without any of the issues we've seen with bulk transfer > splitting. > > And likewise there are no kernel drivers needing sg support > for isoc transfers. We may want to add a BUG_ON urb->sg > being set for isoc transfers somewhere though, in case a > driver author ever does decide to try and use sg there, > like I did :) Not a bad idea. > So I'm sending a new version of the patchset, where the > use of scatterlists is limited to bulk transfers (just a > small change really). > > This new version also addresses / changes: > -making the caps ioctl parameter u32 instead of u64 > -a missing "totlen -= u;" statement in the loop allocating > the per sg entry buffers, causing the last buffer to always > be USB_SG_SIZE, even if a smaller buffer would suffice Okay. Alan Stern -- 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
