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