All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.