USB: visor: reimplement using generic framework

Kill custom read and write implementations (dynamically allocated write
urbs).

Note that I chose to remove the stat module parameter which was supposed
to keep count of the amount of data sent and received, but which has
been broken for three years (since b308e74d9c708ee2a9af14fbe235e0a41216f4ed
"USB: visor driver adapted to new tty buffering" -- bytes_in was
incorrectly updated and was thus always reported as 0).

Compile-only tested.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers/usb/serial/visor.c

@@ -38,17 +38,9 @@
 /* function prototypes for a handspring visor */
 static int  visor_open(struct tty_struct *tty, struct usb_serial_port *port);
 static void visor_close(struct usb_serial_port *port);
-static int  visor_write(struct tty_struct *tty, struct usb_serial_port *port,
-					const unsigned char *buf, int count);
-static int  visor_write_room(struct tty_struct *tty);
-static void visor_throttle(struct tty_struct *tty);
-static void visor_unthrottle(struct tty_struct *tty);
 static int  visor_probe(struct usb_serial *serial,
 					const struct usb_device_id *id);
 static int  visor_calc_num_ports(struct usb_serial *serial);
-static void visor_release(struct usb_serial *serial);
-static void visor_write_bulk_callback(struct urb *urb);
-static void visor_read_bulk_callback(struct urb *urb);
 static void visor_read_int_callback(struct urb *urb);
 static int  clie_3_5_startup(struct usb_serial *serial);
 static int  treo_attach(struct usb_serial *serial);
@@ -196,16 +188,11 @@ static struct usb_serial_driver handspring_device = {
 	.num_ports =		2,
 	.open =			visor_open,
 	.close =		visor_close,
-	.throttle =		visor_throttle,
-	.unthrottle =		visor_unthrottle,
+	.throttle =		usb_serial_generic_throttle,
+	.unthrottle =		usb_serial_generic_unthrottle,
 	.attach =		treo_attach,
 	.probe =		visor_probe,
 	.calc_num_ports =	visor_calc_num_ports,
-	.release =		visor_release,
-	.write =		visor_write,
-	.write_room =		visor_write_room,
-	.write_bulk_callback =	visor_write_bulk_callback,
-	.read_bulk_callback =	visor_read_bulk_callback,
 	.read_int_callback =	visor_read_int_callback,
 };
 
@@ -221,16 +208,11 @@ static struct usb_serial_driver clie_5_device = {
 	.num_ports =		2,
 	.open =			visor_open,
 	.close =		visor_close,
-	.throttle =		visor_throttle,
-	.unthrottle =		visor_unthrottle,
+	.throttle =		usb_serial_generic_throttle,
+	.unthrottle =		usb_serial_generic_unthrottle,
 	.attach =		clie_5_attach,
 	.probe =		visor_probe,
 	.calc_num_ports =	visor_calc_num_ports,
-	.release =		visor_release,
-	.write =		visor_write,
-	.write_room =		visor_write_room,
-	.write_bulk_callback =	visor_write_bulk_callback,
-	.read_bulk_callback =	visor_read_bulk_callback,
 	.read_int_callback =	visor_read_int_callback,
 };
 
@@ -246,38 +228,16 @@ static struct usb_serial_driver clie_3_5_device = {
 	.num_ports =		1,
 	.open =			visor_open,
 	.close =		visor_close,
-	.throttle =		visor_throttle,
-	.unthrottle =		visor_unthrottle,
+	.throttle =		usb_serial_generic_throttle,
+	.unthrottle =		usb_serial_generic_unthrottle,
 	.attach =		clie_3_5_startup,
-	.release =		visor_release,
-	.write =		visor_write,
-	.write_room =		visor_write_room,
-	.write_bulk_callback =	visor_write_bulk_callback,
-	.read_bulk_callback =	visor_read_bulk_callback,
 };
 
-struct visor_private {
-	spinlock_t lock;
-	int bytes_in;
-	int bytes_out;
-	int outstanding_urbs;
-	unsigned char throttled;
-	unsigned char actually_throttled;
-};
-
-/* number of outstanding urbs to prevent userspace DoS from happening */
-#define URB_UPPER_LIMIT	42
-
-static int stats;
-
 /******************************************************************************
  * Handspring Visor specific driver functions
  ******************************************************************************/
 static int visor_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
-	struct usb_serial *serial = port->serial;
-	struct visor_private *priv = usb_get_serial_port_data(port);
-	unsigned long flags;
 	int result = 0;
 
 	dbg("%s - port %d", __func__, port->number);
@@ -288,26 +248,10 @@ static int visor_open(struct tty_struct *tty, struct usb_serial_port *port)
 		return -ENODEV;
 	}
 
-	spin_lock_irqsave(&priv->lock, flags);
-	priv->bytes_in = 0;
-	priv->bytes_out = 0;
-	priv->throttled = 0;
-	spin_unlock_irqrestore(&priv->lock, flags);
-
 	/* Start reading from the device */
-	usb_fill_bulk_urb(port->read_urb, serial->dev,
-			   usb_rcvbulkpipe(serial->dev,
-					    port->bulk_in_endpointAddress),
-			   port->read_urb->transfer_buffer,
-			   port->read_urb->transfer_buffer_length,
-			   visor_read_bulk_callback, port);
-	result = usb_submit_urb(port->read_urb, GFP_KERNEL);
-	if (result) {
-		dev_err(&port->dev,
-			"%s - failed submitting read urb, error %d\n",
-							__func__, result);
+	result = usb_serial_generic_open(tty, port);
+	if (result)
 		goto exit;
-	}
 
 	if (port->interrupt_in_urb) {
 		dbg("%s - adding interrupt input for treo", __func__);
@@ -324,13 +268,12 @@ exit:
 
 static void visor_close(struct usb_serial_port *port)
 {
-	struct visor_private *priv = usb_get_serial_port_data(port);
 	unsigned char *transfer_buffer;
 
 	dbg("%s - port %d", __func__, port->number);
 
 	/* shutdown our urbs */
-	usb_kill_urb(port->read_urb);
+	usb_serial_generic_close(port);
 	usb_kill_urb(port->interrupt_in_urb);
 
 	mutex_lock(&port->serial->disc_mutex);
@@ -347,192 +290,6 @@ static void visor_close(struct usb_serial_port *port)
 		}
 	}
 	mutex_unlock(&port->serial->disc_mutex);
-
-	if (stats)
-		dev_info(&port->dev, "Bytes In = %d  Bytes Out = %d\n",
-			 priv->bytes_in, priv->bytes_out);
-}
-
-
-static int visor_write(struct tty_struct *tty, struct usb_serial_port *port,
-					const unsigned char *buf, int count)
-{
-	struct visor_private *priv = usb_get_serial_port_data(port);
-	struct usb_serial *serial = port->serial;
-	struct urb *urb;
-	unsigned char *buffer;
-	unsigned long flags;
-	int status;
-
-	dbg("%s - port %d", __func__, port->number);
-
-	spin_lock_irqsave(&priv->lock, flags);
-	if (priv->outstanding_urbs > URB_UPPER_LIMIT) {
-		spin_unlock_irqrestore(&priv->lock, flags);
-		dbg("%s - write limit hit", __func__);
-		return 0;
-	}
-	priv->outstanding_urbs++;
-	spin_unlock_irqrestore(&priv->lock, flags);
-
-	buffer = kmalloc(count, GFP_ATOMIC);
-	if (!buffer) {
-		dev_err(&port->dev, "out of memory\n");
-		count = -ENOMEM;
-		goto error_no_buffer;
-	}
-
-	urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!urb) {
-		dev_err(&port->dev, "no more free urbs\n");
-		count = -ENOMEM;
-		goto error_no_urb;
-	}
-
-	memcpy(buffer, buf, count);
-
-	usb_serial_debug_data(debug, &port->dev, __func__, count, buffer);
-
-	usb_fill_bulk_urb(urb, serial->dev,
-			   usb_sndbulkpipe(serial->dev,
-					    port->bulk_out_endpointAddress),
-			   buffer, count,
-			   visor_write_bulk_callback, port);
-
-	/* send it down the pipe */
-	status = usb_submit_urb(urb, GFP_ATOMIC);
-	if (status) {
-		dev_err(&port->dev,
-		   "%s - usb_submit_urb(write bulk) failed with status = %d\n",
-							__func__, status);
-		count = status;
-		goto error;
-	} else {
-		spin_lock_irqsave(&priv->lock, flags);
-		priv->bytes_out += count;
-		spin_unlock_irqrestore(&priv->lock, flags);
-	}
-
-	/* we are done with this urb, so let the host driver
-	 * really free it when it is finished with it */
-	usb_free_urb(urb);
-
-	return count;
-error:
-	usb_free_urb(urb);
-error_no_urb:
-	kfree(buffer);
-error_no_buffer:
-	spin_lock_irqsave(&priv->lock, flags);
-	--priv->outstanding_urbs;
-	spin_unlock_irqrestore(&priv->lock, flags);
-	return count;
-}
-
-
-static int visor_write_room(struct tty_struct *tty)
-{
-	struct usb_serial_port *port = tty->driver_data;
-	struct visor_private *priv = usb_get_serial_port_data(port);
-	unsigned long flags;
-
-	dbg("%s - port %d", __func__, port->number);
-
-	/*
-	 * We really can take anything the user throws at us
-	 * but let's pick a nice big number to tell the tty
-	 * layer that we have lots of free space, unless we don't.
-	 */
-
-	spin_lock_irqsave(&priv->lock, flags);
-	if (priv->outstanding_urbs > URB_UPPER_LIMIT * 2 / 3) {
-		spin_unlock_irqrestore(&priv->lock, flags);
-		dbg("%s - write limit hit", __func__);
-		return 0;
-	}
-	spin_unlock_irqrestore(&priv->lock, flags);
-
-	return 2048;
-}
-
-
-static void visor_write_bulk_callback(struct urb *urb)
-{
-	struct usb_serial_port *port = urb->context;
-	struct visor_private *priv = usb_get_serial_port_data(port);
-	int status = urb->status;
-	unsigned long flags;
-
-	/* free up the transfer buffer, as usb_free_urb() does not do this */
-	kfree(urb->transfer_buffer);
-
-	dbg("%s - port %d", __func__, port->number);
-
-	if (status)
-		dbg("%s - nonzero write bulk status received: %d",
-		    __func__, status);
-
-	spin_lock_irqsave(&priv->lock, flags);
-	--priv->outstanding_urbs;
-	spin_unlock_irqrestore(&priv->lock, flags);
-
-	usb_serial_port_softint(port);
-}
-
-
-static void visor_read_bulk_callback(struct urb *urb)
-{
-	struct usb_serial_port *port = urb->context;
-	struct visor_private *priv = usb_get_serial_port_data(port);
-	unsigned char *data = urb->transfer_buffer;
-	int status = urb->status;
-	struct tty_struct *tty;
-	int result;
-	int available_room = 0;
-
-	dbg("%s - port %d", __func__, port->number);
-
-	if (status) {
-		dbg("%s - nonzero read bulk status received: %d",
-		    __func__, status);
-		return;
-	}
-
-	usb_serial_debug_data(debug, &port->dev, __func__,
-						urb->actual_length, data);
-
-	if (urb->actual_length) {
-		tty = tty_port_tty_get(&port->port);
-		if (tty) {
-			tty_insert_flip_string(tty, data,
-						urb->actual_length);
-			tty_flip_buffer_push(tty);
-			tty_kref_put(tty);
-		}
-		spin_lock(&priv->lock);
-		if (tty)
-			priv->bytes_in += available_room;
-
-	} else {
-		spin_lock(&priv->lock);
-	}
-
-	/* Continue trying to always read if we should */
-	if (!priv->throttled) {
-		usb_fill_bulk_urb(port->read_urb, port->serial->dev,
-				   usb_rcvbulkpipe(port->serial->dev,
-					   port->bulk_in_endpointAddress),
-				   port->read_urb->transfer_buffer,
-				   port->read_urb->transfer_buffer_length,
-				   visor_read_bulk_callback, port);
-		result = usb_submit_urb(port->read_urb, GFP_ATOMIC);
-		if (result)
-			dev_err(&port->dev,
-			    "%s - failed resubmitting read urb, error %d\n",
-							__func__, result);
-	} else
-		priv->actually_throttled = 1;
-	spin_unlock(&priv->lock);
 }
 
 static void visor_read_int_callback(struct urb *urb)
@@ -576,41 +333,6 @@ exit:
 							__func__, result);
 }
 
-static void visor_throttle(struct tty_struct *tty)
-{
-	struct usb_serial_port *port = tty->driver_data;
-	struct visor_private *priv = usb_get_serial_port_data(port);
-
-	dbg("%s - port %d", __func__, port->number);
-	spin_lock_irq(&priv->lock);
-	priv->throttled = 1;
-	spin_unlock_irq(&priv->lock);
-}
-
-
-static void visor_unthrottle(struct tty_struct *tty)
-{
-	struct usb_serial_port *port = tty->driver_data;
-	struct visor_private *priv = usb_get_serial_port_data(port);
-	int result, was_throttled;
-
-	dbg("%s - port %d", __func__, port->number);
-	spin_lock_irq(&priv->lock);
-	priv->throttled = 0;
-	was_throttled = priv->actually_throttled;
-	priv->actually_throttled = 0;
-	spin_unlock_irq(&priv->lock);
-
-	if (was_throttled) {
-		port->read_urb->dev = port->serial->dev;
-		result = usb_submit_urb(port->read_urb, GFP_KERNEL);
-		if (result)
-			dev_err(&port->dev,
-				"%s - failed submitting read urb, error %d\n",
-							__func__, result);
-	}
-}
-
 static int palm_os_3_probe(struct usb_serial *serial,
 						const struct usb_device_id *id)
 {
@@ -778,28 +500,6 @@ static int visor_calc_num_ports(struct usb_serial *serial)
 	return num_ports;
 }
 
-static int generic_startup(struct usb_serial *serial)
-{
-	struct usb_serial_port **ports = serial->port;
-	struct visor_private *priv;
-	int i;
-
-	for (i = 0; i < serial->num_ports; ++i) {
-		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-		if (!priv) {
-			while (i-- != 0) {
-				priv = usb_get_serial_port_data(ports[i]);
-				usb_set_serial_port_data(ports[i], NULL);
-				kfree(priv);
-			}
-			return -ENOMEM;
-		}
-		spin_lock_init(&priv->lock);
-		usb_set_serial_port_data(ports[i], priv);
-	}
-	return 0;
-}
-
 static int clie_3_5_startup(struct usb_serial *serial)
 {
 	struct device *dev = &serial->dev->dev;
@@ -850,7 +550,7 @@ static int clie_3_5_startup(struct usb_serial *serial)
 		goto out;
 	}
 
-	result = generic_startup(serial);
+	result = 0;
 out:
 	kfree(data);
 
@@ -868,7 +568,7 @@ static int treo_attach(struct usb_serial *serial)
 		(le16_to_cpu(serial->dev->descriptor.idVendor)
 						== KYOCERA_VENDOR_ID)) ||
 		(serial->num_interrupt_in == 0))
-		goto generic_startup;
+		return 0;
 
 	dbg("%s", __func__);
 
@@ -898,8 +598,7 @@ static int treo_attach(struct usb_serial *serial)
 	COPY_PORT(serial->port[1], swap_port);
 	kfree(swap_port);
 
-generic_startup:
-	return generic_startup(serial);
+	return 0;
 }
 
 static int clie_5_attach(struct usb_serial *serial)
@@ -922,20 +621,7 @@ static int clie_5_attach(struct usb_serial *serial)
 	serial->port[0]->bulk_out_endpointAddress =
 				serial->port[1]->bulk_out_endpointAddress;
 
-	return generic_startup(serial);
-}
-
-static void visor_release(struct usb_serial *serial)
-{
-	struct visor_private *priv;
-	int i;
-
-	dbg("%s", __func__);
-
-	for (i = 0; i < serial->num_ports; i++) {
-		priv = usb_get_serial_port_data(serial->port[i]);
-		kfree(priv);
-	}
+	return 0;
 }
 
 static int __init visor_init(void)
@@ -1019,8 +705,6 @@ MODULE_LICENSE("GPL");
 
 module_param(debug, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(debug, "Debug enabled or not");
-module_param(stats, bool, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(stats, "Enables statistics or not");
 
 module_param(vendor, ushort, 0);
 MODULE_PARM_DESC(vendor, "User specified vendor ID");