Am Samstag, 9. August 2003 02:04 schrieb David Brownell:
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).
If you don't want wait_event_timeout, it seems to me that only struct completion should be used like this:
How about making it even simpler, and replacing that useless diagnostic with one that might help troubleshooting? :)
This is an even simpler rewrite, which doesn't lay any kind of groundwork for interruptible synchronous calls. (I still think we should have that, but this patch seems ready to go the next time Greg merges patches...)
- Dave
--- 1.34/drivers/usb/core/message.c Mon Aug 11 07:56:25 2003
+++ edited/drivers/usb/core/message.c Thu Aug 14 10:55:50 2003
@@ -16,77 +16,66 @@
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/mm.h>
+#include <linux/timer.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;
+ complete((struct completion *)urb->context);
+}
- awd->done = 1;
- wmb();
- wake_up(&awd->wqh);
+
+static void timeout_kill(unsigned long data)
+{
+ struct urb *urb = (struct urb *) data;
+
+ dev_warn(&urb->dev->dev, "%s timeout on ep%d%s\n",
+ usb_pipecontrol(urb->pipe) ? "control" : "bulk",
+ usb_pipeendpoint(urb->pipe),
+ usb_pipein(urb->pipe) ? "in" : "out");
+ usb_unlink_urb(urb);
}
// Starts urb and waits for completion or timeout
+// note that this call is NOT interruptible, while
+// many device driver i/o requests should be interruptible
static int usb_start_wait_urb(struct urb *urb, int timeout, int* actual_length)
{
- DECLARE_WAITQUEUE(wait, current);
- struct usb_api_data awd;
- int status;
-
- init_waitqueue_head(&awd.wqh);
- awd.done = 0;
-
- set_current_state(TASK_UNINTERRUPTIBLE);
- add_wait_queue(&awd.wqh, &wait);
-
- urb->context = &awd;
- 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();
+ struct completion done;
+ struct timer_list timer;
+ int status;
+
+ might_sleep();
+ init_completion(&done);
+ urb->context = &done;
+ urb->transfer_flags |= URB_ASYNC_UNLINK;
+ urb->actual_length = 0;
+
+ if (timeout > 0) {
+ init_timer(&timer);
+ timer.expires = jiffies + timeout;
+ timer.data = (unsigned long)urb;
+ timer.function = timeout_kill;
+ add_timer(&timer);
}
-
- 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;
- }
- } else
+ status = usb_submit_urb(urb, GFP_NOIO);
+ if (status == 0) {
+ wait_for_completion(&done);
status = urb->status;
+ /* note: HCDs return ETIMEDOUT for other reasons too */
+ if (status == -ECONNRESET)
+ status = -ETIMEDOUT;
+ }
+ if (timeout > 0)
+ del_timer_sync(&timer);
if (actual_length)
*actual_length = urb->actual_length;
-
usb_free_urb(urb);
- return status;
+ return status;
}
/*-------------------------------------------------------------------*/
