Here's an updated version of Oliver's patch, changing:

- uses "struct completion" for simplicity

 - the wait_event_timeout() logic is now like the other macros
   which can return before the event happens:  returns a value

 - clarity for MAX_SCHEDULE_TIMEOUT scenarios, by using the
   standard "timeouts are long" calling convention internally

- uses only the async primitives

- restores a warning when the request times out.

- can use dev_dbg() calls in drivers/usb/core/message.c

Still needs more testing (notably the timeout paths!) and cleanup,
but I thought it'd be good to send this one around.

- Dave

--- 1.22/drivers/usb/core/message.c     Wed Mar  5 07:24:34 2003
+++ edited/drivers/usb/core/message.c   Wed Mar 19 23:18:46 2003
@@ -2,6 +2,14 @@
  * message.c - synchronous message handling
  */
 
+#include <linux/config.h>
+
+#ifdef CONFIG_USB_DEBUG
+       #define DEBUG
+#else
+       #undef DEBUG
+#endif
+
 #include <linux/pci.h> /* for scatterlist macros */
 #include <linux/usb.h>
 #include <linux/module.h>
@@ -11,78 +19,57 @@
 
 #include "hcd.h"       /* for usbcore internals */
 
-struct usb_api_data {
-       wait_queue_head_t wqh;
-       int done;
-};
-
-static void usb_api_blocking_completion(struct urb *urb, struct pt_regs *regs)
+static void done (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);
+       dev_dbg (&urb->dev->dev, "urb %p done\n", urb);
+       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)
+static int usb_start_wait_urb(struct urb *urb, long timeout, int *actual_length)
 { 
-       DECLARE_WAITQUEUE(wait, current);
-       struct usb_api_data awd;
+       DECLARE_COMPLETION (wait);
        int status;
 
-       init_waitqueue_head(&awd.wqh);  
-       awd.done = 0;
+       might_sleep();
 
-       set_current_state(TASK_UNINTERRUPTIBLE);
-       add_wait_queue(&awd.wqh, &wait);
-
-       urb->context = &awd;
-       status = usb_submit_urb(urb, GFP_ATOMIC);
+       urb->context = &wait;
+       urb->complete = done;
+       urb->transfer_flags |= URB_ASYNC_UNLINK;
+       status = usb_submit_urb (urb, GFP_NOIO);
        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();
-       }
-
-       set_current_state(TASK_RUNNING);
-       remove_wait_queue(&awd.wqh, &wait);
-
-       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;
+       /* cancel request if we time out */
+       if (wait_event_timeout (wait.wait, wait.done, timeout) == 0) {
+               if ((status = usb_unlink_urb (urb)) == -EINPROGRESS)
+                       wait_event (wait.wait, wait.done);
+               /* patch status unless we raced a "real" completion */
+               if (urb->status == -ECONNRESET) {
+                       dev_warn (&urb->dev->dev,
+                               "timeout urb for ep%d%s (len %d/%d)\n",
+                               usb_pipeendpoint (urb->pipe),
+                               usb_pipein (urb->pipe) ? "in" : "out",
+                               urb->actual_length,
+                               urb->transfer_buffer_length);
+                       urb->status = -ETIME;
                }
-       } else
-               status = urb->status;
+       }
+       status = urb->status;
 
        if (actual_length)
                *actual_length = urb->actual_length;
-
-       usb_free_urb(urb);
-       return status;
+       
+       return status;
 }
 
 /*-------------------------------------------------------------------*/
 // returns status (negative) or length (positive)
 int usb_internal_control_msg(struct usb_device *usb_dev, unsigned int pipe, 
-                           struct usb_ctrlrequest *cmd,  void *data, int len, int 
timeout)
+                           struct usb_ctrlrequest *cmd,  void *data, int len,
+                           int timeout /* 0 means MAX_SCHEDULE_TIMEOUT */ )
 {
        struct urb *urb;
        int retv;
@@ -93,9 +80,12 @@
                return -ENOMEM;
   
        usb_fill_control_urb(urb, usb_dev, pipe, (unsigned char*)cmd, data, len,
-                  usb_api_blocking_completion, 0);
+                  0, 0);
 
-       retv = usb_start_wait_urb(urb, timeout, &length);
+       retv = usb_start_wait_urb(urb,
+                       timeout ? (long)timeout : MAX_SCHEDULE_TIMEOUT,
+                       &length);
+       usb_free_urb(urb);
        if (retv < 0)
                return retv;
        else
@@ -182,6 +172,7 @@
                        void *data, int len, int *actual_length, int timeout)
 {
        struct urb *urb;
+       int r;
 
        if (len < 0)
                return -EINVAL;
@@ -190,10 +181,13 @@
        if (!urb)
                return -ENOMEM;
 
-       usb_fill_bulk_urb(urb, usb_dev, pipe, data, len,
-                   usb_api_blocking_completion, 0);
+       usb_fill_bulk_urb(urb, usb_dev, pipe, data, len, 0, 0);
 
-       return usb_start_wait_urb(urb,timeout,actual_length);
+       r = usb_start_wait_urb(urb,
+                       timeout ? (long)timeout : MAX_SCHEDULE_TIMEOUT,
+                       actual_length);
+       usb_free_urb(urb);
+       return r;
 }
 
 /*-------------------------------------------------------------------*/
--- 1.12/include/linux/wait.h   Fri Nov 15 07:36:32 2002
+++ edited/include/linux/wait.h Wed Mar 19 21:25:21 2003
@@ -173,6 +173,34 @@
        __ret;                                                          \
 })
 
+#define __wait_event_timeout(wq, condition, ret)                       \
+do {                                                                   \
+       wait_queue_t __wait;                                            \
+       init_waitqueue_entry(&__wait, current);                         \
+                                                                       \
+       add_wait_queue(&wq, &__wait);                                   \
+       for (;;) {                                                      \
+               set_current_state(TASK_UNINTERRUPTIBLE);                \
+               if (condition)                                          \
+                       break;                                          \
+               ret = schedule_timeout(ret);                            \
+               if (!ret)                                               \
+                       break;                                          \
+       }                                                               \
+       current->state = TASK_RUNNING;                                  \
+       remove_wait_queue(&wq, &__wait);                                \
+} while (0)
+
+#define wait_event_timeout(wq, condition, timeout)     \
+({                                                                     \
+       long __ret = timeout;                                           \
+       if (!(condition))                                               \
+               __wait_event_timeout(wq, condition, __ret); \
+       __ret;                                                          \
+})
+       
+
+
 #define __wait_event_interruptible_timeout(wq, condition, ret)         \
 do {                                                                   \
        wait_queue_t __wait;                                            \

Reply via email to