[PATCH 0/4]: USB: ftdi_sio: fix regression in 2.6.31 and clean up

[PATCH 0/4]: USB: ftdi_sio: fix regression in 2.6.31 and clean up

[Johan Hovold]

Hi, 

These patches clean up the ftdi_sio driver and fixes

 1) a long outstanding bug manifesting itself as 
 
 	BUG: sleeping function called from invalid context at kernel/mutex.c:280

 2) a couple of regressions in 2.6.31 (stalled reads and unthrottle race)
    due to changes in the tty layer.

Please have a look at it so we can get something back-ported to stable as the
ftdi_sio driver is currently completly broken and unusable due to the stalled
reads.

Note that the patches do not add suspend/resume support to the driver (but the
clean up should make it easier to implement).

Thanks to Alan Cox, Oliver Neukum, and Alan Stern for all comments and
suggestions so far.

Thanks,
Johan


drivers/usb/serial/ftdi_sio.c |  422 +++++++++++++----------------------------
 1 files changed, 129 insertions(+), 293 deletions(-)

[PATCH 1/4] USB: ftdi_sio: remove tty->low_latency

[Johan Hovold]

Fixes tty_flip_buffer_push being called from hard interrupt context with
low_latency set.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/usb/serial/ftdi_sio.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 4f883b1..0ac2c2f 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1234,7 +1234,6 @@ static int set_serial_info(struct tty_struct *tty,
 					(new_serial.flags & ASYNC_FLAGS));
 	priv->custom_divisor = new_serial.custom_divisor;
 
-	tty->low_latency = (priv->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
 	write_latency_timer(port);
 
 check_and_exit:
@@ -1704,9 +1703,6 @@ static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port)
 	priv->rx_bytes = 0;
 	spin_unlock_irqrestore(&priv->rx_lock, flags);
 
-	if (tty)
-		tty->low_latency = (priv->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
-
 	write_latency_timer(port);
 
 	/* No error checking for this (will get errors later anyway) */
-- 
1.6.5.rc2

[PATCH 2/4] USB: ftdi_sio: remove unused rx_byte counter

[Johan Hovold]

Remove unused rx_byte counter which is never exposed as noted by Alan
Cox.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/usb/serial/ftdi_sio.c |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 0ac2c2f..bfb23d6 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -81,7 +81,6 @@ struct ftdi_private {
 	struct delayed_work rx_work;
 	struct usb_serial_port *port;
 	int rx_processed;
-	unsigned long rx_bytes;
 
 	__u16 interface;	/* FT2232C, FT2232H or FT4232H port interface
 				   (0 for FT232/245) */
@@ -1699,9 +1698,6 @@ static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port)
 	spin_lock_irqsave(&priv->tx_lock, flags);
 	priv->tx_bytes = 0;
 	spin_unlock_irqrestore(&priv->tx_lock, flags);
-	spin_lock_irqsave(&priv->rx_lock, flags);
-	priv->rx_bytes = 0;
-	spin_unlock_irqrestore(&priv->rx_lock, flags);
 
 	write_latency_timer(port);
 
@@ -2014,8 +2010,6 @@ static void ftdi_read_bulk_callback(struct urb *urb)
 	struct usb_serial_port *port = urb->context;
 	struct tty_struct *tty;
 	struct ftdi_private *priv;
-	unsigned long countread;
-	unsigned long flags;
 	int status = urb->status;
 
 	if (urb->number_of_packets > 0) {
@@ -2054,13 +2048,6 @@ static void ftdi_read_bulk_callback(struct urb *urb)
 		goto out;
 	}
 
-	/* count data bytes, but not status bytes */
-	countread = urb->actual_length;
-	countread -= 2 * DIV_ROUND_UP(countread, priv->max_packet_size);
-	spin_lock_irqsave(&priv->rx_lock, flags);
-	priv->rx_bytes += countread;
-	spin_unlock_irqrestore(&priv->rx_lock, flags);
-
 	ftdi_process_read(&priv->rx_work.work);
 out:
 	tty_kref_put(tty);
-- 
1.6.5.rc2

[PATCH 3/4] USB: ftdi_sio: clean up read completion handler

[Johan Hovold]

Remove superfluous error checks in completion handler:

 - No need to check private data and urb pointers as we check urb-status
   before dereferencing priv (which is not freed until urb has been killed
   on close).
 - No need to check tty as it is checked again when processing.
 - No need to check urb->number_of_packets on bulk urb.

Note that both private data and tty are checked again before processing
(possibly from work queue which also is cancelled on close).

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/usb/serial/ftdi_sio.c |   28 +---------------------------
 1 files changed, 1 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index bfb23d6..75c84d9 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -2008,39 +2008,14 @@ static int ftdi_chars_in_buffer(struct tty_struct *tty)
 static void ftdi_read_bulk_callback(struct urb *urb)
 {
 	struct usb_serial_port *port = urb->context;
-	struct tty_struct *tty;
 	struct ftdi_private *priv;
 	int status = urb->status;
 
-	if (urb->number_of_packets > 0) {
-		dev_err(&port->dev, "%s transfer_buffer_length %d "
-			"actual_length %d number of packets %d\n", __func__,
-			urb->transfer_buffer_length,
-			urb->actual_length, urb->number_of_packets);
-		dev_err(&port->dev, "%s transfer_flags %x\n", __func__,
-			urb->transfer_flags);
-	}
-
 	dbg("%s - port %d", __func__, port->number);
 
 	if (port->port.count <= 0)
 		return;
 
-	tty = tty_port_tty_get(&port->port);
-	if (!tty) {
-		dbg("%s - bad tty pointer - exiting", __func__);
-		return;
-	}
-
-	priv = usb_get_serial_port_data(port);
-	if (!priv) {
-		dbg("%s - bad port private data pointer - exiting", __func__);
-		goto out;
-	}
-
-	if (urb != port->read_urb)
-		dev_err(&port->dev, "%s - Not my urb!\n", __func__);
-
 	if (status) {
 		/* This will happen at close every time so it is a dbg not an
 		   err */
@@ -2048,9 +2023,8 @@ static void ftdi_read_bulk_callback(struct urb *urb)
 		goto out;
 	}
 
+	priv = usb_get_serial_port_data(port);
 	ftdi_process_read(&priv->rx_work.work);
-out:
-	tty_kref_put(tty);
 } /* ftdi_read_bulk_callback */
 
 
-- 
1.6.5.rc2

[PATCH 4/4] USB: ftdi_sio: re-implement read processing

[Johan Hovold]

 - Re-structure read processing.
 - Kill obsolete work queue and always push to tty in completion handler.
 - Use tty_insert_flip_string instead of per character push when
   possible.
 - Fix stalled-read regression in 2.6.31 by using urb status to
   determine when port is closed rather than port count.
 - Fix race with open/close by checking ASYNCB_INITIALIZED in
   unthrottle.
 - Kill private rx_flag and lock and use throttle flags in
   usb_serial_port instead.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/usb/serial/ftdi_sio.c |  383 ++++++++++++++---------------------------
 1 files changed, 131 insertions(+), 252 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 75c84d9..9c60d6d 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -76,12 +76,7 @@ struct ftdi_private {
 	unsigned long last_dtr_rts;	/* saved modem control outputs */
 	wait_queue_head_t delta_msr_wait; /* Used for TIOCMIWAIT */
 	char prev_status, diff_status;        /* Used for TIOCMIWAIT */
-	__u8 rx_flags;		/* receive state flags (throttling) */
-	spinlock_t rx_lock;	/* spinlock for receive state */
-	struct delayed_work rx_work;
 	struct usb_serial_port *port;
-	int rx_processed;
-
 	__u16 interface;	/* FT2232C, FT2232H or FT4232H port interface
 				   (0 for FT232/245) */
 
@@ -736,10 +731,6 @@ static const char *ftdi_chip_name[] = {
 /* Constants for read urb and write urb */
 #define BUFSZ 512
 
-/* rx_flags */
-#define THROTTLED		0x01
-#define ACTUALLY_THROTTLED	0x02
-
 /* Used for TIOCMIWAIT */
 #define FTDI_STATUS_B0_MASK	(FTDI_RS0_CTS | FTDI_RS0_DSR | FTDI_RS0_RI | FTDI_RS0_RLSD)
 #define FTDI_STATUS_B1_MASK	(FTDI_RS_BI)
@@ -762,7 +753,7 @@ static int  ftdi_write_room(struct tty_struct *tty);
 static int  ftdi_chars_in_buffer(struct tty_struct *tty);
 static void ftdi_write_bulk_callback(struct urb *urb);
 static void ftdi_read_bulk_callback(struct urb *urb);
-static void ftdi_process_read(struct work_struct *work);
+static void ftdi_process_read(struct usb_serial_port *port);
 static void ftdi_set_termios(struct tty_struct *tty,
 			struct usb_serial_port *port, struct ktermios *old);
 static int  ftdi_tiocmget(struct tty_struct *tty, struct file *file);
@@ -1525,7 +1516,6 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
 	}
 
 	kref_init(&priv->kref);
-	spin_lock_init(&priv->rx_lock);
 	spin_lock_init(&priv->tx_lock);
 	init_waitqueue_head(&priv->delta_msr_wait);
 	/* This will push the characters through immediately rather
@@ -1547,7 +1537,6 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
 		port->read_urb->transfer_buffer_length = BUFSZ;
 	}
 
-	INIT_DELAYED_WORK(&priv->rx_work, ftdi_process_read);
 	priv->port = port;
 
 	/* Free port's existing write urb and transfer buffer. */
@@ -1684,6 +1673,26 @@ static int ftdi_sio_port_remove(struct usb_serial_port *port)
 	return 0;
 }
 
+static int ftdi_submit_read_urb(struct usb_serial_port *port, gfp_t mem_flags)
+{
+	struct urb *urb = port->read_urb;
+	struct usb_serial *serial = port->serial;
+	int result;
+
+	usb_fill_bulk_urb(urb, serial->dev,
+			   usb_rcvbulkpipe(serial->dev,
+					port->bulk_in_endpointAddress),
+			   urb->transfer_buffer,
+			   urb->transfer_buffer_length,
+			   ftdi_read_bulk_callback, port);
+	result = usb_submit_urb(urb, mem_flags);
+	if (result)
+		dev_err(&port->dev,
+			"%s - failed submitting read urb, error %d\n",
+							__func__, result);
+	return result;
+}
+
 static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port)
 { /* ftdi_open */
 	struct usb_device *dev = port->serial->dev;
@@ -1717,23 +1726,14 @@ static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port)
 		ftdi_set_termios(tty, port, tty->termios);
 
 	/* Not throttled */
-	spin_lock_irqsave(&priv->rx_lock, flags);
-	priv->rx_flags &= ~(THROTTLED | ACTUALLY_THROTTLED);
-	spin_unlock_irqrestore(&priv->rx_lock, flags);
+	spin_lock_irqsave(&port->lock, flags);
+	port->throttled = 0;
+	port->throttle_req = 0;
+	spin_unlock_irqrestore(&port->lock, flags);
 
 	/* Start reading from the device */
-	priv->rx_processed = 0;
-	usb_fill_bulk_urb(port->read_urb, dev,
-			usb_rcvbulkpipe(dev, port->bulk_in_endpointAddress),
-			port->read_urb->transfer_buffer,
-				port->read_urb->transfer_buffer_length,
-			ftdi_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);
-	else
+	result = ftdi_submit_read_urb(port, GFP_KERNEL);
+	if (!result)
 		kref_get(&priv->kref);
 
 	return result;
@@ -1779,10 +1779,6 @@ static void ftdi_close(struct usb_serial_port *port)
 
 	dbg("%s", __func__);
 
-
-	/* cancel any scheduled reading */
-	cancel_delayed_work_sync(&priv->rx_work);
-
 	/* shutdown our bulk read */
 	usb_kill_urb(port->read_urb);
 	kref_put(&priv->kref, ftdi_sio_priv_release);
@@ -2005,236 +2001,121 @@ static int ftdi_chars_in_buffer(struct tty_struct *tty)
 	return buffered;
 }
 
-static void ftdi_read_bulk_callback(struct urb *urb)
+static int ftdi_process_packet(struct tty_struct *tty,
+		struct usb_serial_port *port, struct ftdi_private *priv,
+		char *packet, int len)
 {
-	struct usb_serial_port *port = urb->context;
-	struct ftdi_private *priv;
-	int status = urb->status;
-
-	dbg("%s - port %d", __func__, port->number);
-
-	if (port->port.count <= 0)
-		return;
-
-	if (status) {
-		/* This will happen at close every time so it is a dbg not an
-		   err */
-		dbg("(this is ok on close) nonzero read bulk status received: %d", status);
-		goto out;
-	}
-
-	priv = usb_get_serial_port_data(port);
-	ftdi_process_read(&priv->rx_work.work);
-} /* ftdi_read_bulk_callback */
-
-
-static void ftdi_process_read(struct work_struct *work)
-{ /* ftdi_process_read */
-	struct ftdi_private *priv =
-		container_of(work, struct ftdi_private, rx_work.work);
-	struct usb_serial_port *port = priv->port;
-	struct urb *urb;
-	struct tty_struct *tty;
-	char error_flag;
-	unsigned char *data;
-
 	int i;
-	int result;
-	int need_flip;
-	int packet_offset;
-	unsigned long flags;
+	char status;
+	char flag;
+	char *ch;
 
 	dbg("%s - port %d", __func__, port->number);
 
-	if (port->port.count <= 0)
-		return;
-
-	tty = tty_port_tty_get(&port->port);
-	if (!tty) {
-		dbg("%s - bad tty pointer - exiting", __func__);
-		return;
+	if (len < 2) {
+		dbg("malformed packet");
+		return 0;
 	}
 
-	priv = usb_get_serial_port_data(port);
-	if (!priv) {
-		dbg("%s - bad port private data pointer - exiting", __func__);
-		goto out;
+	/* Compare new line status to the old one, signal if different/
+	   N.B. packet may be processed more than once, but differences
+	   are only processed once.  */
+	status = packet[0] & FTDI_STATUS_B0_MASK;
+	if (status != priv->prev_status) {
+		priv->diff_status |= status ^ priv->prev_status;
+		wake_up_interruptible(&priv->delta_msr_wait);
+		priv->prev_status = status;
 	}
 
-	urb = port->read_urb;
-	if (!urb) {
-		dbg("%s - bad read_urb pointer - exiting", __func__);
-		goto out;
+	/*
+	 * Although the device uses a bitmask and hence can have multiple
+	 * errors on a packet - the order here sets the priority the error is
+	 * returned to the tty layer.
+	 */
+	flag = TTY_NORMAL;
+	if (packet[1] & FTDI_RS_OE) {
+		flag = TTY_OVERRUN;
+		dbg("OVERRRUN error");
 	}
-
-	data = urb->transfer_buffer;
-
-	if (priv->rx_processed) {
-		dbg("%s - already processed: %d bytes, %d remain", __func__,
-				priv->rx_processed,
-				urb->actual_length - priv->rx_processed);
-	} else {
-		/* The first two bytes of every read packet are status */
-		if (urb->actual_length > 2)
-			usb_serial_debug_data(debug, &port->dev, __func__,
-						urb->actual_length, data);
-		else
-			dbg("Status only: %03oo %03oo", data[0], data[1]);
+	if (packet[1] & FTDI_RS_BI) {
+		flag = TTY_BREAK;
+		dbg("BREAK received");
+		usb_serial_handle_break(port);
+	}
+	if (packet[1] & FTDI_RS_PE) {
+		flag = TTY_PARITY;
+		dbg("PARITY error");
+	}
+	if (packet[1] & FTDI_RS_FE) {
+		flag = TTY_FRAME;
+		dbg("FRAMING error");
 	}
 
-
-	/* TO DO -- check for hung up line and handle appropriately: */
-	/*   send hangup  */
-	/* See acm.c - you do a tty_hangup  - eg tty_hangup(tty) */
-	/* if CD is dropped and the line is not CLOCAL then we should hangup */
-
-	need_flip = 0;
-	for (packet_offset = priv->rx_processed;
-		packet_offset < urb->actual_length; packet_offset += priv->max_packet_size) {
-		int length;
-
-		/* Compare new line status to the old one, signal if different/
-		   N.B. packet may be processed more than once, but differences
-		   are only processed once.  */
-		char new_status = data[packet_offset + 0] &
-						FTDI_STATUS_B0_MASK;
-		if (new_status != priv->prev_status) {
-			priv->diff_status |=
-				new_status ^ priv->prev_status;
-			wake_up_interruptible(&priv->delta_msr_wait);
-			priv->prev_status = new_status;
-		}
-
-		length = min_t(u32, priv->max_packet_size, urb->actual_length-packet_offset)-2;
-		if (length < 0) {
-			dev_err(&port->dev, "%s - bad packet length: %d\n",
-				__func__, length+2);
-			length = 0;
-		}
-
-		if (priv->rx_flags & THROTTLED) {
-			dbg("%s - throttled", __func__);
-			break;
-		}
-		if (tty_buffer_request_room(tty, length) < length) {
-			/* break out & wait for throttling/unthrottling to
-			   happen */
-			dbg("%s - receive room low", __func__);
-			break;
+	len -= 2;
+	if (!len)
+		return 0;	/* status only */
+	ch = packet + 2;
+
+	if (!(port->console && port->sysrq) && flag == TTY_NORMAL)
+		tty_insert_flip_string(tty, ch, len);
+	else {
+		for (i = 0; i < len; i++, ch++) {
+			if (!usb_serial_handle_sysrq_char(tty, port, *ch))
+				tty_insert_flip_char(tty, *ch, flag);
 		}
+	}
+	return len;
+}
 
-		/* Handle errors and break */
-		error_flag = TTY_NORMAL;
-		/* Although the device uses a bitmask and hence can have
-		   multiple errors on a packet - the order here sets the
-		   priority the error is returned to the tty layer  */
+static void ftdi_process_read(struct usb_serial_port *port)
+{
+	struct urb *urb = port->read_urb;
+	struct tty_struct *tty;
+	struct ftdi_private *priv = usb_get_serial_port_data(port);
+	char *data = (char *)urb->transfer_buffer;
+	int i;
+	int len;
+	int count = 0;
 
-		if (data[packet_offset+1] & FTDI_RS_OE) {
-			error_flag = TTY_OVERRUN;
-			dbg("OVERRRUN error");
-		}
-		if (data[packet_offset+1] & FTDI_RS_BI) {
-			error_flag = TTY_BREAK;
-			dbg("BREAK received");
-			usb_serial_handle_break(port);
-		}
-		if (data[packet_offset+1] & FTDI_RS_PE) {
-			error_flag = TTY_PARITY;
-			dbg("PARITY error");
-		}
-		if (data[packet_offset+1] & FTDI_RS_FE) {
-			error_flag = TTY_FRAME;
-			dbg("FRAMING error");
-		}
-		if (length > 0) {
-			for (i = 2; i < length+2; i++) {
-				/* Note that the error flag is duplicated for
-				   every character received since we don't know
-				   which character it applied to */
-				if (!usb_serial_handle_sysrq_char(tty, port,
-						data[packet_offset + i]))
-					tty_insert_flip_char(tty,
-						data[packet_offset + i],
-						error_flag);
-			}
-			need_flip = 1;
-		}
+	tty = tty_port_tty_get(&port->port);
+	if (!tty)
+		return;
 
-#ifdef NOT_CORRECT_BUT_KEEPING_IT_FOR_NOW
-		/* if a parity error is detected you get status packets forever
-		   until a character is sent without a parity error.
-		   This doesn't work well since the application receives a
-		   never ending stream of bad data - even though new data
-		   hasn't been sent. Therefore I (bill) have taken this out.
-		   However - this might make sense for framing errors and so on
-		   so I am leaving the code in for now.
-		*/
-		else {
-			if (error_flag != TTY_NORMAL) {
-				dbg("error_flag is not normal");
-				/* In this case it is just status - if that is
-				   an error send a bad character */
-				if (tty->flip.count >= TTY_FLIPBUF_SIZE)
-					tty_flip_buffer_push(tty);
-				tty_insert_flip_char(tty, 0xff, error_flag);
-				need_flip = 1;
-			}
-		}
-#endif
-	} /* "for(packet_offset=0..." */
+	for (i = 0; i < urb->actual_length; i += priv->max_packet_size) {
+		len = min_t(int, urb->actual_length - i, priv->max_packet_size);
+		count += ftdi_process_packet(tty, port, priv, &data[i], len);
+	}
 
-	/* Low latency */
-	if (need_flip)
+	if (count)
 		tty_flip_buffer_push(tty);
+	tty_kref_put(tty);
+}
 
-	if (packet_offset < urb->actual_length) {
-		/* not completely processed - record progress */
-		priv->rx_processed = packet_offset;
-		dbg("%s - incomplete, %d bytes processed, %d remain",
-				__func__, packet_offset,
-				urb->actual_length - packet_offset);
-		/* check if we were throttled while processing */
-		spin_lock_irqsave(&priv->rx_lock, flags);
-		if (priv->rx_flags & THROTTLED) {
-			priv->rx_flags |= ACTUALLY_THROTTLED;
-			spin_unlock_irqrestore(&priv->rx_lock, flags);
-			dbg("%s - deferring remainder until unthrottled",
-					__func__);
-			goto out;
-		}
-		spin_unlock_irqrestore(&priv->rx_lock, flags);
-		/* if the port is closed stop trying to read */
-		if (port->port.count > 0)
-			/* delay processing of remainder */
-			schedule_delayed_work(&priv->rx_work, 1);
-		else
-			dbg("%s - port is closed", __func__);
-		goto out;
-	}
-
-	/* urb is completely processed */
-	priv->rx_processed = 0;
+static void ftdi_read_bulk_callback(struct urb *urb)
+{
+	struct usb_serial_port *port = urb->context;
+	unsigned long flags;
 
-	/* if the port is closed stop trying to read */
-	if (port->port.count > 0) {
-		/* Continue trying to always read  */
-		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,
-			ftdi_read_bulk_callback, port);
+	dbg("%s - port %d", __func__, port->number);
 
-		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);
+	if (urb->status) {
+		dbg("%s - nonzero read bulk status received: %d",
+						__func__, urb->status);
+		return;
 	}
-out:
-	tty_kref_put(tty);
-} /* ftdi_process_read */
 
+	usb_serial_debug_data(debug, &port->dev, __func__,
+				urb->actual_length, urb->transfer_buffer);
+	ftdi_process_read(port);
+
+	spin_lock_irqsave(&port->lock, flags);
+	port->throttled = port->throttle_req;
+	if (!port->throttled) {
+		spin_unlock_irqrestore(&port->lock, flags);
+		ftdi_submit_read_urb(port, GFP_ATOMIC);
+	} else
+		spin_unlock_irqrestore(&port->lock, flags);
+}
 
 static void ftdi_break_ctl(struct tty_struct *tty, int break_state)
 {
@@ -2566,33 +2447,31 @@ static int ftdi_ioctl(struct tty_struct *tty, struct file *file,
 static void ftdi_throttle(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	struct ftdi_private *priv = usb_get_serial_port_data(port);
 	unsigned long flags;
 
 	dbg("%s - port %d", __func__, port->number);
 
-	spin_lock_irqsave(&priv->rx_lock, flags);
-	priv->rx_flags |= THROTTLED;
-	spin_unlock_irqrestore(&priv->rx_lock, flags);
+	spin_lock_irqsave(&port->lock, flags);
+	port->throttle_req = 1;
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
-
-static void ftdi_unthrottle(struct tty_struct *tty)
+void ftdi_unthrottle(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	struct ftdi_private *priv = usb_get_serial_port_data(port);
-	int actually_throttled;
+	int was_throttled;
 	unsigned long flags;
 
 	dbg("%s - port %d", __func__, port->number);
 
-	spin_lock_irqsave(&priv->rx_lock, flags);
-	actually_throttled = priv->rx_flags & ACTUALLY_THROTTLED;
-	priv->rx_flags &= ~(THROTTLED | ACTUALLY_THROTTLED);
-	spin_unlock_irqrestore(&priv->rx_lock, flags);
+	spin_lock_irqsave(&port->lock, flags);
+	was_throttled = port->throttled;
+	port->throttled = port->throttle_req = 0;
+	spin_unlock_irqrestore(&port->lock, flags);
 
-	if (actually_throttled)
-		schedule_delayed_work(&priv->rx_work, 0);
+	/* Resubmit urb if throttled and open. */
+	if (was_throttled && test_bit(ASYNCB_INITIALIZED, &port->port.flags))
+		ftdi_submit_read_urb(port, GFP_KERNEL);
 }
 
 static int __init ftdi_init(void)
-- 
1.6.5.rc2

Re: [PATCH 0/4]: USB: ftdi_sio: fix regression in 2.6.31 and clean up

[Greg KH]

On Wed, Oct 07, 2009 at 08:05:03PM +0200, Johan Hovold wrote:
> Hi, 
> 
> These patches clean up the ftdi_sio driver and fixes
> 
>  1) a long outstanding bug manifesting itself as 
>  
>  	BUG: sleeping function called from invalid context at kernel/mutex.c:280
> 
>  2) a couple of regressions in 2.6.31 (stalled reads and unthrottle race)
>     due to changes in the tty layer.
> 
> Please have a look at it so we can get something back-ported to stable as the
> ftdi_sio driver is currently completly broken and unusable due to the stalled
> reads.
> 
> Note that the patches do not add suspend/resume support to the driver (but the
> clean up should make it easier to implement).
> 
> Thanks to Alan Cox, Oliver Neukum, and Alan Stern for all comments and
> suggestions so far.

Very nice, thanks for doing this work, I really appreciate it.  I'll
queue it up as soon as possible.

thanks,

greg k-h