The code that manges the synchronous control/bulk calls has been a mess for ages. This patch rewrites it using:
- "struct completion" instead of a usb-internal clone therof, - prepare_to_wait()/finish_wait() instead of the tangled mess it now uses (or a new wait_event_timeout call, as in previous versions of this patch).
It's a net code shrink and simplification.
Please review and merge.
- Dave
--- 1.33/drivers/usb/core/message.c Fri Aug 1 05:03:12 2003 +++ edited/drivers/usb/core/message.c Fri Aug 8 16:39:14 2003 @@ -16,75 +16,60 @@ #include <linux/slab.h> #include <linux/init.h> #include <linux/mm.h> +#include <linux/completion.h> #include <asm/byteorder.h> #include "hcd.h" /* for usbcore internals */ #include "usb.h" -struct usb_api_data { - wait_queue_head_t wqh; - int done; -}; - static void usb_api_blocking_completion(struct urb *urb, struct pt_regs *regs) { - struct usb_api_data *awd = (struct usb_api_data *)urb->context; - - awd->done = 1; - wmb(); - wake_up(&awd->wqh); + complete ((struct completion *)urb->context); } // Starts urb and waits for completion or timeout static int usb_start_wait_urb(struct urb *urb, int timeout, int* actual_length) { - DECLARE_WAITQUEUE(wait, current); - struct usb_api_data awd; + DEFINE_WAIT(wait); + DECLARE_COMPLETION(done); int status; - init_waitqueue_head(&awd.wqh); - awd.done = 0; - - set_current_state(TASK_UNINTERRUPTIBLE); - add_wait_queue(&awd.wqh, &wait); + urb->transfer_flags |= URB_ASYNC_UNLINK; + urb->context = &done; - urb->context = &awd; + might_sleep(); + prepare_to_wait(&done.wait, &wait, TASK_UNINTERRUPTIBLE); status = usb_submit_urb(urb, GFP_ATOMIC); - if (status) { - // something went wrong - usb_free_urb(urb); - set_current_state(TASK_RUNNING); - remove_wait_queue(&awd.wqh, &wait); - return status; - } - - while (timeout && !awd.done) - { - timeout = schedule_timeout(timeout); - set_current_state(TASK_UNINTERRUPTIBLE); - rmb(); - } + if (status != 0) { + urb->actual_length = 0; - set_current_state(TASK_RUNNING); - remove_wait_queue(&awd.wqh, &wait); + /* normal case: we woke without a timeout */ + } else if (schedule_timeout((timeout == 0) + ? MAX_SCHEDULE_TIMEOUT + : timeout) != 0) { + status = urb->status; - if (!timeout && !awd.done) { - if (urb->status != -EINPROGRESS) { /* No callback?!! */ - printk(KERN_ERR "usb: raced timeout, " - "pipe 0x%x status %d time left %d\n", - urb->pipe, urb->status, timeout); - status = urb->status; - } else { - warn("usb_control/bulk_msg: timeout"); - usb_unlink_urb(urb); // remove urb safely - status = -ETIMEDOUT; + /* abnormal: timed out, so force completion via unlink */ + } else { + status = usb_unlink_urb(urb); + switch (status) { + case -EINPROGRESS: /* normal */ + case -EBUSY: /* already completing */ + dev_err(&urb->dev->dev, "control/bulk timeout\n"); + break; + default: + /* shouldn't happen */ + dev_err(&urb->dev->dev, "c/b unlink err %d\n", status); + break; } - } else - status = urb->status; + wait_for_completion(&done); + /* NOTE: HCDs return this status too */ + status = -ETIMEDOUT; + } + finish_wait(&done.wait, &wait); if (actual_length) *actual_length = urb->actual_length; - usb_free_urb(urb); return status; }