Hi Noralf, Noralf Trønnes wrote: > >> +static int gud_usb_bulk(struct gud_device *gdrm, size_t len) .. > >> + timer_setup_on_stack(&ctx.timer, gud_usb_bulk_timeout, 0); > >> + mod_timer(&ctx.timer, jiffies + msecs_to_jiffies(3000)); > >> + > >> + usb_sg_wait(&ctx.sgr); > >> + > >> + if (!del_timer_sync(&ctx.timer)) > >> + ret = -ETIMEDOUT; .. > > Mention in the commit message that sending USB bulk transfers with > > an sglist could be unstable Can you explain a bit about /how/ it is unstable? As you write, usb_bulk_msg() (as used before) has a timeout which is passed to the host controller hardware and implemented there. I haven't used SG with kernel USB but I would expect such a timeout to still be available with SG? > usb_bulk_msg() has builtin timeout handling and during development of > a microcontroller gadget implementation I've triggered this timeout > several times when the uC usb interrupts stopped firing. The device not responding to bulk packets scheduled and sent by the host is a real error /in the device/ and thus not neccessarily something the kernel must handle gracefully.. I think it's quite nice to do so, but one can argue that it's not strictly required. But more importantly: Remember that bulk transfer has no delivery time guarantee. It can take indefinitely long until a bulk transfer is scheduled by the host on a busy bus which is starved with more important things (control, interrupt, iso transfers) - that's not an error at all, and may be indistinguishable from the device not responding to packets actually sent by the host. Having a timeout is important, I just expect the USB SG interface to support it since it is the hardware that times out in the non-SG case. And since this is essentially real time data maybe a shorter timeout is better? 3 seconds seems really long. The timeout must include all latency for a frame, so e.g. 16ms (60 Hz) is too short for sure. But maybe something like 500ms? //Peter
