public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Romain Lievin <lkml@lievin.net>
To: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Cc: Greg Kroah <greg@kroah.com>
Subject: [PATCH] tiglusb: wrong timeout value
Date: Mon, 19 Apr 2004 10:55:59 +0200	[thread overview]
Message-ID: <20040419085559.GA13904@lievin.net> (raw)

Hi,

this patch (cumulative; 2.4 & 2.6) fixes another bug in the tiglusb driver. The formula used to calculate jiffies from timeout is wrong.
The new formula is ok and takes care of integer computation/rounding.
This is the same kind of bug than in the tipar char driver.

Please apply, Romain.
======================[ cut here ]===========================
diff -Naur linux-2.6.5.orig/drivers/usb/misc/tiglusb.c linux-2.6.5/drivers/usb/misc/tiglusb.c
--- linux-2.6.5.orig/drivers/usb/misc/tiglusb.c	2004-04-04 05:36:14.000000000 +0200
+++ linux-2.6.5/drivers/usb/misc/tiglusb.c	2004-04-19 10:47:59.000000000 +0200
@@ -3,7 +3,7 @@
  * tiglusb -- Texas Instruments' USB GraphLink (aka SilverLink) driver.
  * Target: Texas Instruments graphing calculators (http://lpg.ticalc.org).
  *
- * Copyright (C) 2001-2002:
+ * Copyright (C) 2001-2004:
  *   Romain Lievin <roms@lpg.ticalc.org>
  *   Julien BLACHE <jb@technologeek.org>
  * under the terms of the GNU General Public License.
@@ -20,6 +20,8 @@
  *   1.04, Julien: clean-up & fixes; Romain: 2.4 backport.
  *   1.05, Randy Dunlap: bug fix with the timeout parameter (divide-by-zero).
  *   1.06, Romain: synched with 2.5, version/firmware changed (confusing).
+ *   1.07, Romain: fixed bad use of usb_clear_halt (invalid argument);
+ *          timeout argument checked in ioctl + clean-up.
  */
 
 #include <linux/module.h>
@@ -38,8 +40,8 @@
 /*
  * Version Information
  */
-#define DRIVER_VERSION "1.06"
-#define DRIVER_AUTHOR  "Romain Lievin <roms@lpg.ticalc.org> & Julien Blache <jb@jblache.org>"
+#define DRIVER_VERSION "1.07"
+#define DRIVER_AUTHOR  "Romain Lievin <roms@tilp.info> & Julien Blache <jb@jblache.org>"
 #define DRIVER_DESC    "TI-GRAPH LINK USB (aka SilverLink) driver"
 #define DRIVER_LICENSE "GPL"
 
@@ -72,15 +74,15 @@
 {
 	unsigned int pipe;
 
-	pipe = usb_sndbulkpipe (dev, 1);
-	if (usb_clear_halt (dev, usb_pipeendpoint (pipe))) {
-		err ("clear_pipe (r), request failed");
+	pipe = usb_sndbulkpipe (dev, 2);
+	if (usb_clear_halt (dev, pipe)) {
+		err ("clear_pipe (w), request failed");
 		return -1;
 	}
 
-	pipe = usb_sndbulkpipe (dev, 2);
-	if (usb_clear_halt (dev, usb_pipeendpoint (pipe))) {
-		err ("clear_pipe (w), request failed");
+	pipe = usb_rcvbulkpipe (dev, 1);
+	if (usb_clear_halt (dev, pipe)) {
+		err ("clear_pipe (r), request failed");
 		return -1;
 	}
 
@@ -181,17 +183,16 @@
 
 	pipe = usb_rcvbulkpipe (s->dev, 1);
 	result = usb_bulk_msg (s->dev, pipe, buffer, bytes_to_read,
-			       &bytes_read, HZ * 10 / timeout);
+			       &bytes_read, (HZ * timeout) / 10);
 	if (result == -ETIMEDOUT) {	/* NAK */
-		ret = result;
-		if (!bytes_read) {
+		if (!bytes_read)
 			dbg ("quirk !");
-		}
 		warn ("tiglusb_read, NAK received.");
+		ret = result;
 		goto out;
 	} else if (result == -EPIPE) {	/* STALL -- shouldn't happen */
 		warn ("clear_halt request to remove STALL condition.");
-		if (usb_clear_halt (s->dev, usb_pipeendpoint (pipe)))
+		if (usb_clear_halt (s->dev, pipe))
 			err ("clear_halt, request failed");
 		clear_device (s->dev);
 		ret = result;
@@ -243,7 +244,7 @@
 
 	pipe = usb_sndbulkpipe (s->dev, 2);
 	result = usb_bulk_msg (s->dev, pipe, buffer, bytes_to_write,
-			       &bytes_written, HZ * 10 / timeout);
+			       &bytes_written, (HZ * timeout) / 10);
 
 	if (result == -ETIMEDOUT) {	/* NAK */
 		warn ("tiglusb_write, NAK received.");
@@ -251,7 +252,7 @@
 		goto out;
 	} else if (result == -EPIPE) {	/* STALL -- shouldn't happen */
 		warn ("clear_halt request to remove STALL condition.");
-		if (usb_clear_halt (s->dev, usb_pipeendpoint (pipe)))
+		if (usb_clear_halt (s->dev, pipe))
 			err ("clear_halt, request failed");
 		clear_device (s->dev);
 		ret = result;
@@ -292,15 +293,16 @@
 
 	switch (cmd) {
 	case IOCTL_TIUSB_TIMEOUT:
-		timeout = arg;	// timeout value in tenth of seconds
+		if (arg > 0)
+			timeout = arg;
+		else
+			ret = -EINVAL;
 		break;
 	case IOCTL_TIUSB_RESET_DEVICE:
-		dbg ("IOCTL_TIGLUSB_RESET_DEVICE");
 		if (clear_device (s->dev))
 			ret = -EIO;
 		break;
 	case IOCTL_TIUSB_RESET_PIPES:
-		dbg ("IOCTL_TIGLUSB_RESET_PIPES");
 		if (clear_pipes (s->dev))
 			ret = -EIO;
 		break;
@@ -447,7 +449,7 @@
 
 #ifndef MODULE
 /*
- * You can use 'tiusb=timeout'
+ * You can use 'tiusb=timeout' to set timeout.
  */
 static int __init
 tiglusb_setup (char *str)
@@ -457,10 +459,11 @@
 	str = get_options (str, ARRAY_SIZE (ints), ints);
 
 	if (ints[0] > 0) {
-		timeout = ints[1];
+		if (ints[1] > 0)
+			timeout = ints[1];
+		else
+			info ("tiglusb: wrong timeout value (0), using default value.");
 	}
-	if (timeout <= 0)
-		timeout = TIMAXTIME;
 
 	return 1;
 }
@@ -502,9 +505,6 @@
 
 	info (DRIVER_DESC ", version " DRIVER_VERSION);
 
-	if (timeout <= 0)
-		timeout = TIMAXTIME;
-
 	return 0;
 }
 

======================[ cut here ]===========================
diff -Naur linux-2.4.26.orig/drivers/usb/tiglusb.c linux-2.4.26/drivers/usb/tiglusb.c
--- linux-2.4.26.orig/drivers/usb/tiglusb.c	2003-06-13 16:51:37.000000000 +0200
+++ linux-2.4.26/drivers/usb/tiglusb.c	2004-04-19 10:42:14.000000000 +0200
@@ -3,7 +3,7 @@
  * tiglusb -- Texas Instruments' USB GraphLink (aka SilverLink) driver.
  * Target: Texas Instruments graphing calculators (http://lpg.ticalc.org).
  *
- * Copyright (C) 2001-2002:
+ * Copyright (C) 2001-2004:
  *   Romain Lievin <roms@lpg.ticalc.org>
  *   Julien BLACHE <jb@technologeek.org>
  * under the terms of the GNU General Public License.
@@ -14,11 +14,14 @@
  * and the website at:  http://lpg.ticalc.org/prj_usb/
  * for more info.
  *
+ * History:
  *   1.0x, Romain & Julien: initial submit.
  *   1.03, Greg Kroah: modifications.
  *   1.04, Julien: clean-up & fixes; Romain: 2.4 backport.
  *   1.05, Randy Dunlap: bug fix with the timeout parameter (divide-by-zero).
  *   1.06, Romain: synched with 2.5, version/firmware changed (confusing).
+ *   1.07, Romain: fixed bad use of usb_clear_halt (invalid argument),
+ *         checking for a valid timeout, fixed wrong timeout formula, clean-up.
  */
 
 #include <linux/module.h>
@@ -38,8 +41,8 @@
 /*
  * Version Information
  */
-#define DRIVER_VERSION "1.06"
-#define DRIVER_AUTHOR  "Romain Lievin <roms@lpg.ticalc.org> & Julien Blache <jb@jblache.org>"
+#define DRIVER_VERSION "1.07"
+#define DRIVER_AUTHOR  "Romain Lievin <roms@tilp.info> & Julien Blache <jb@jblache.org>"
 #define DRIVER_DESC    "TI-GRAPH LINK USB (aka SilverLink) driver"
 #define DRIVER_LICENSE "GPL"
 
@@ -74,15 +77,15 @@
 {
 	unsigned int pipe;
 
-	pipe = usb_sndbulkpipe (dev, 1);
-	if (usb_clear_halt (dev, usb_pipeendpoint (pipe))) {
-		err ("clear_pipe (r), request failed");
+	pipe = usb_sndbulkpipe (dev, 2);
+	if (usb_clear_halt (dev, pipe)) {
+		err ("clear_pipe (w), request failed");
 		return -1;
 	}
 
-	pipe = usb_sndbulkpipe (dev, 2);
-	if (usb_clear_halt (dev, usb_pipeendpoint (pipe))) {
-		err ("clear_pipe (w), request failed");
+	pipe = usb_rcvbulkpipe (dev, 1);
+	if (usb_clear_halt (dev, pipe)) {
+		err ("clear_pipe (r), request failed");
 		return -1;
 	}
 
@@ -179,17 +182,16 @@
 
 	pipe = usb_rcvbulkpipe (s->dev, 1);
 	result = usb_bulk_msg (s->dev, pipe, buffer, bytes_to_read,
-			       &bytes_read, HZ * 10 / timeout);
+			       &bytes_read, (HZ * timeout) / 10);
 	if (result == -ETIMEDOUT) {	/* NAK */
-		ret = result;
-		if (!bytes_read) {
+		if (!bytes_read)
 			dbg ("quirk !");
-		}
 		warn ("tiglusb_read, NAK received.");
+		ret = result;
 		goto out;
 	} else if (result == -EPIPE) {	/* STALL -- shouldn't happen */
 		warn ("clear_halt request to remove STALL condition.");
-		if (usb_clear_halt (s->dev, usb_pipeendpoint (pipe)))
+		if (usb_clear_halt (s->dev, pipe))
 			err ("clear_halt, request failed");
 		clear_device (s->dev);
 		ret = result;
@@ -236,7 +238,7 @@
 
 	pipe = usb_sndbulkpipe (s->dev, 2);
 	result = usb_bulk_msg (s->dev, pipe, buffer, bytes_to_write,
-			       &bytes_written, HZ * 10 / timeout);
+			       &bytes_written, (HZ * timeout) / 10);
 
 	if (result == -ETIMEDOUT) {	/* NAK */
 		warn ("tiglusb_write, NAK received.");
@@ -244,7 +246,7 @@
 		goto out;
 	} else if (result == -EPIPE) {	/* STALL -- shouldn't happen */
 		warn ("clear_halt request to remove STALL condition.");
-		if (usb_clear_halt (s->dev, usb_pipeendpoint (pipe)))
+		if (usb_clear_halt (s->dev, pipe))
 			err ("clear_halt, request failed");
 		clear_device (s->dev);
 		ret = result;
@@ -284,15 +286,16 @@
 
 	switch (cmd) {
 	case IOCTL_TIUSB_TIMEOUT:
-		timeout = arg;	// timeout value in tenth of seconds
+		if (arg > 0)
+			timeout = (int)arg;
+		else
+			ret = -EINVAL;
 		break;
 	case IOCTL_TIUSB_RESET_DEVICE:
-		dbg ("IOCTL_TIGLUSB_RESET_DEVICE");
 		if (clear_device (s->dev))
 			ret = -EIO;
 		break;
 	case IOCTL_TIUSB_RESET_PIPES:
-		dbg ("IOCTL_TIGLUSB_RESET_PIPES");
 		if (clear_pipes (s->dev))
 			ret = -EIO;
 		break;
@@ -430,7 +433,7 @@
 
 #ifndef MODULE
 /*
- * You can use 'tiusb=timeout'
+ * You can use 'tiusb=timeout' to set timeout.
  */
 static int __init
 tiglusb_setup (char *str)
@@ -440,10 +443,11 @@
 	str = get_options (str, ARRAY_SIZE (ints), ints);
 
 	if (ints[0] > 0) {
-		timeout = ints[1];
+		if (ints[1] > 0)
+			timeout = ints[1];
+		else
+			info ("tiglusb: wrong timeout value (0), using default value.");
 	}
-	if (!timeout)
-		timeout = TIMAXTIME;
 
 	return 1;
 }
@@ -466,8 +470,6 @@
 		init_waitqueue_head (&s->wait);
 		init_waitqueue_head (&s->remove_ok);
 	}
-	if (timeout <= 0)
-               timeout = TIMAXTIME;
 
 	/* register device */
 	if (register_chrdev (TIUSB_MAJOR, "tiglusb", &tiglusb_fops)) {
@@ -487,12 +489,6 @@
 
 	info (DRIVER_DESC ", version " DRIVER_VERSION);
 
-       if (timeout <= 0)
-               timeout = TIMAXTIME;
-
-	if (!timeout)
-		timeout = TIMAXTIME;
-
 	return 0;
 }
 

======================[ cut here ]===========================
-- 
Romain Liévin (roms):         <lkml@lievin.net>
Web site:                     http://www.lievin.net
"Linux, y'a moins bien mais c'est plus cher !"







             reply	other threads:[~2004-04-19  8:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-04-19  8:55 Romain Lievin [this message]
2004-04-22 21:31 ` [PATCH] tiglusb: wrong timeout value Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20040419085559.GA13904@lievin.net \
    --to=lkml@lievin.net \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox