Re: URB completion order, normal behavior or bug?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/23/2013 09:08 PM, Alan Stern wrote:
On Sun, 23 Jun 2013, Daniel Santos wrote:

So I'm working on this MCP2210 driver which talks in 64-byte
request/response pairs using interrupt URBs (it can be used with
hid-generic, but I don't want to do that).  I'm testing on a Raspberry
Pi Model B using the Raspbian 3.9.7 kernel. My usual order of things is:
Maybe you don't care about this any more, but in case you still do,
here's my perspective.  This applies to the drivers in the standard
kernel; I don't know anything in particular about the driver used in
the RPi.

I submit an out URB (to rx my response)
No, an OUT URB transfers data _out_ from the computer to the device.
The response goes from the device _in_ to the computer.

I sure do appreciate this because I was initially confused about this and am now re-confused. I presumed that "out" meant "out of the computer and into the USB device". However, I indeed have the variable labeled "out" that I am using to receive responses and visa versa. I have this little struct I use for both the IN and OUT URBs in my device struct:

struct mcp2210_endpoint_proxy {
    const struct usb_host_endpoint *ep;
    struct urb *urb;
    struct mcp2210_msg *buffer;
    /* FIXME: inefficient */
    u8 submitted:1;
    u8 completed:1;
    u8 kill:1;
    u8 retry_count:4;
    u8 is_dir_in:1;
};

/* then in my device struct: */
struct mcp2210_device {
    struct usb_device *udev;
    struct usb_interface *intf;
    struct spi_master *spi_master;
    struct gpio_chip *gpio_chip;

...
    struct mcp2210_endpoint_proxy in;
    struct mcp2210_endpoint_proxy out;
...
};

I initialize them both with a single function and it works! This is what caused me to believe that the URB directions were described from the USB device standpoint. (please pardon my mailer's mangling of whitespace)


static __always_inline int init_endpoint(struct mcp2210_device *dev,
                     const struct usb_host_endpoint *ep)
{
    int is_dir_in = !!usb_endpoint_dir_in(&ep->desc);
    struct mcp2210_endpoint_proxy *dest;
    unsigned int pipe;
    usb_complete_t complete_fn;
    struct urb *urb;

    if (is_dir_in) {
        dest = &dev->in;
        pipe = usb_sndintpipe(dev->udev, ep->desc.bEndpointAddress);
        complete_fn = urb_complete_in;
    } else {
        dest = &dev->out;
        pipe = usb_rcvintpipe(dev->udev, ep->desc.bEndpointAddress);
        complete_fn = urb_complete_out;
    }
    dest->is_dir_in = is_dir_in;

    if (unlikely(dest->ep)) {
        mcp2210_warn("Unexpected: more than one interrupt %s endpoint "
                 "discovered, ingoring them\n",
                 urb_dir_str[is_dir_in]);
        return 0;
    }

    urb = usb_alloc_urb(0, GFP_KERNEL);
    if (unlikely(!urb)) {
        mcp2210_err("Failed to alloc %s URB\n", urb_dir_str[is_dir_in]);
        return -ENOMEM;
    }

    dest->buffer = usb_alloc_coherent(dev->udev, 64, GFP_KERNEL,
                      &urb->transfer_dma);
    if (unlikely(!dest->buffer)) {
        usb_free_urb(urb);
        mcp2210_err("Failed to alloc %s URB DMA buffer\n",
                urb_dir_str[is_dir_in]);
        return -ENOMEM;
    }

    dest->ep = ep;
    dest->urb = urb;

    usb_fill_int_urb(urb,
            dev->udev,
            pipe,
            dest->buffer,
            64,
            complete_fn,
            dev,
            ep->desc.bInterval);
    urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;

    return 0;
}

This is how I use it in my probe function:

int mcp2210_probe(struct usb_interface *intf, const struct usb_device_id *id)
{
    struct usb_device *udev = interface_to_usbdev(intf);
    struct usb_host_interface *intf_desc = intf->cur_altsetting;
    struct mcp2210_device *dev;
    struct usb_host_endpoint *ep, *ep_end;
    int ret = -ENODEV;
    int i;

    printk("mcp2210_probe\n");

...

    /* Set up interrupt endpoint information. */
    ep_end = &intf_desc->endpoint[intf_desc->desc.bNumEndpoints];
    for (ep = intf_desc->endpoint; ep != ep_end; ++ep) {

        if (!usb_endpoint_xfer_int(&ep->desc))
            continue;

        if (init_endpoint(dev, ep))
            goto error0;
    }

    if (unlikely(!dev->in.ep || !dev->out.ep)) {
        mcp2210_err("could not find in and/or out interrupt endpoints");
        goto error0;
    }

...
}

I don't understand why this works and the previous didn't in light of your comment (as well as my original assumptions). Can you see any flaws in this? I've used !!usb_endpoint_dir_in(&ep->desc) to get a one or zero for rather or not I have the IN endpoint since I store this in a u8:1 bitfield.

I submit an in URB (with my request)
Likewise, your request goes _out_ from the computer to the device.

My in URB completion handler is called
My out URB completion handler is called.
Because of the confusion in the terminology, it's hard to tell what
this means.  I take it that normally, the request completes before the
response does (which is what you would expect).

But occasionally (more so when my system is heavily loaded, spamming
printks & such), the out URB completion handler is called first.  I
initially thought this was due to the device chattering or some
electrical noise as I have all of this open with wires in bread-boards &
such, but then I dumped the contents of the out URB and discovered that
it was a valid response for the given request. So the transaction must
have completed normally & correctly, but my completion handlers weren't
called in the same order as the actual communications occurred.
That can happen.  URBs for any single endpoint always complete in order
of submission (unless one of them terminates prematurely because it was
cancelled), but this is not guaranteed for URBs to different endpoints.
If both URBs happen to complete in the same frame or microframe, the
order in which the handlers get called is entirely up to the host
controller driver.

Of course, if you didn't submit the response URB until after the
request URB completed, then everything would always go in the expected
order.  (Just stating the obvious...)

Gotcha here! My thought on submitting them both at the same time is to (hopefully, theoretically, etc.) speed up the transaction, but also to give me more opportunities to allocate memory from the GFP_KERNEL pool (I'm not really sure how much usb_submit_urb needs to allocate).

I'm (obviously) new to device drivers, but I wasn't able to find any
information saying that this is normal.  Is it?
It is a little unusual, but it can happen.

And now I know.  Learning is good. :)

Also, again usually when I have debug enabled, it is common for me to
have a stalled out URB (that I detect with a manager thread running in a
delayed work queue) that I have to kill and re-submit. This almost
always stalls as well, so I have to re-submit the in URB (request) as
well.  I'm not sure of the cause of that either, but it causes me to
think that some of my responses are being dropped on the floor, which
wont allow me to build a reliable device since double-sending some
requests will cause undesired results.

Anyway, I guess I'll test on my workstation for now, but it would be
nice to understand what's happening.
Things will probably make more sense on a workstation.

Hope this helps,

It does indeed, thank you. I'm going to start testing on a laptop since it's still crashing and I just re-did my locking (so I'm expecting plenty more of it).

Daniel
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux