[PATCH 2/2] drm/gud: Use scatter-gather USB bulk transfer

[Peter Stuge]

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