> It's never a NOP unless the unlink somehow completed already.
> As I said:  using async unlinks because I want to see all the
> synchronization right here, clearly visible, no hidden magic.

OK, maybe I am dense, but this doesn't make sense.
Let's have another closer look:

--- 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);

Here, you get woken up.

 }
 
 // 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();

unnecessary, wait_for_completion() has the test

+       prepare_to_wait(&done.wait, &wait, TASK_UNINTERRUPTIBLE);

You prepare to sleep using a conventional wait queue

        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)

And here you sleep, doing the wait on the waitqueue
of the struct completion

+                               ? 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);

What for? You already waited with schedule_timeout.

+               /* 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;
 }


-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to