Greg,
Anything wrong with this patch ?
thanks
Franck
Franck Bui-Huu wrote:
> This patch uses completion timeout instead of a timer to implement
> a timeout when submitting an URB in usb_start_wait_urb().
>
> It also fixes a small issue. With the previous code, if no timeout
> happened and the URB's status was set to ECONNRESET value, the code
> assumed wrongly that a timeout had occured.
>
> Signed-off-by: Franck Bui-Huu <[EMAIL PROTECTED]>
> ---
> drivers/usb/core/message.c | 73
> +++++++++++++++++---------------------------
> 1 files changed, 29 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 08fb20f..6213c6d 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -24,59 +24,44 @@ static void usb_api_blocking_completion(
> }
>
>
> -static void timeout_kill(unsigned long data)
> -{
> - struct urb *urb = (struct urb *) data;
> -
> - 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)
> +/*
> + * Starts urb and waits for completion or timeout. Note that this call
> + * is NOT interruptible. Many device driver i/o requests should be
> + * interruptible and therefore these drivers should implement their
> + * own interruptible routines.
> + */
> +static int usb_start_wait_urb(struct urb *urb, int timeout, int
> *actual_length)
> {
> - struct completion done;
> - struct timer_list timer;
> - int status;
> + struct completion done;
> + unsigned long expire;
> + int status;
>
> init_completion(&done);
> urb->context = &done;
> urb->actual_length = 0;
> status = usb_submit_urb(urb, GFP_NOIO);
> -
> - if (status == 0) {
> - if (timeout > 0) {
> - init_timer(&timer);
> - timer.expires = jiffies + msecs_to_jiffies(timeout);
> - timer.data = (unsigned long)urb;
> - timer.function = timeout_kill;
> - /* grr. timeout _should_ include submit delays. */
> - add_timer(&timer);
> - }
> - wait_for_completion(&done);
> + if (unlikely(status))
> + goto out;
> +
> + expire = timeout ? msecs_to_jiffies(timeout) : MAX_SCHEDULE_TIMEOUT;
> + if (!wait_for_completion_timeout(&done, expire)) {
> +
> + dev_dbg(&urb->dev->dev,
> + "%s timed out on ep%d%s len=%d/%d\n",
> + current->comm,
> + usb_pipeendpoint(urb->pipe),
> + usb_pipein(urb->pipe) ? "in" : "out",
> + urb->actual_length,
> + urb->transfer_buffer_length);
> +
> + usb_kill_urb(urb);
> + status = urb->status == -ENOENT ? -ETIMEDOUT : urb->status;
> + } else
> status = urb->status;
> - /* note: HCDs return ETIMEDOUT for other reasons too */
> - if (status == -ECONNRESET) {
> - dev_dbg(&urb->dev->dev,
> - "%s timed out on ep%d%s len=%d/%d\n",
> - current->comm,
> - usb_pipeendpoint(urb->pipe),
> - usb_pipein(urb->pipe) ? "in" : "out",
> - urb->actual_length,
> - urb->transfer_buffer_length
> - );
> - if (urb->actual_length > 0)
> - status = 0;
> - else
> - status = -ETIMEDOUT;
> - }
> - if (timeout > 0)
> - del_timer_sync(&timer);
> - }
> -
> +out:
> if (actual_length)
> *actual_length = urb->actual_length;
> +
> usb_free_urb(urb);
> return status;
> }
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
