Hi,
The ftdi_sio driver receives up to 496 bytes of data at a time (512 byte
transfer minus 16 status bytes) and this can cause data to be lost in
the tty line discipline handling, even if hardware flow control is enabled.
The attached patch, originally by Daniel Smertnig with some changes by
myself works arounf this problem by processing the received data in
chunks and delaying further processing via a work queue when the
connection is about to be throttled by the line discipline.
It has been tested by Daniel and myself using large, non-canonical
transfers at high baud rate with a slow reading process and hardware
flow control enabled. (Maybe some additional small change will be
required to avoid losing data for canonical transfers.)
Daniel's original description of the problem is in the attached patch,
signed off by myself and Daniel.
Please consider applying this patch. TIA.
--
-=( Ian Abbott @ MEV Ltd. E-mail: <[EMAIL PROTECTED]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
ftdi_sio: Avoid losing bytes at tty-ldisc.
This patch was originally developed by Daniel Smertnig. I
(Ian Abbott) made a few changes. It has been tested by both
Daniel and I, at least for raw, non-canonical receive data
processing.
Here is Daniel's original description of the patch:
===
During a project in which I was using a FTDI 232BM to
transmit data at relative high speeds (625kBit/s), I
noticed a problem where data was lost even if flow
control was enabled: The FTDI-Driver receives 512 Bytes
of data over USB at a time, which consists of 8 64-Byte
packets. Subtracting the 2 bytes of status information
included in each packet this gives 496 "real" data
bytes per read.
This data is passed (indirectly, via the flip buffers)
to the tty line discipline which takes care of
throttling when there the free buffer space reaches
TTY_THRESHOLD_THROTTLE (128). Because the FTDI driver
processes up to 496 bytes at a time, throttling won't
happen in time and the line discipline will discard the
remaining bytes.
To avoid this the patch passes data in 62-byte blocks
to the tty layer and checks the available space in the
ldisc-buffers. If there isn't enough free space,
processing the rest of the data is delayed using a
workqueue.
Note: The original problem should be easily
reproducible with a userspace program which does slow &
small reads.
===
Signed-off-by: Ian Abbott <[EMAIL PROTECTED]>
Signed-off-by: Daniel Smertnig <[EMAIL PROTECTED]>
diff -ur linux-2.6.12-rc5/drivers/usb/serial/ftdi_sio.c linux-2.6.12-rc5-new/drivers/usb/serial/ftdi_sio.c
--- linux-2.6.12-rc5/drivers/usb/serial/ftdi_sio.c 2005-06-01 15:35:40.000000000 +0100
+++ linux-2.6.12-rc5-new/drivers/usb/serial/ftdi_sio.c 2005-06-01 15:36:18.000000000 +0100
@@ -22,6 +22,10 @@
* The prelimilary port to the 2.6 kernel was by Rus V. Brushkoff. I have
* fixed a couple of things.
*
+ * (01/Jun/2004) Daniel Smertnig & Ian Abbott
+ * Avoid received data being discarded by line discipline when throttled
+ * even though the FIFO has not overrun.
+ *
* (27/May/2004) Ian Abbott
* Improved throttling code, mostly stolen from the WhiteHEAT driver.
*
@@ -264,7 +268,7 @@
/*
* Version Information
*/
-#define DRIVER_VERSION "v1.4.1"
+#define DRIVER_VERSION "v1.4.2"
#define DRIVER_AUTHOR "Greg Kroah-Hartman <[EMAIL PROTECTED]>, Bill Ryder <[EMAIL PROTECTED]>, Kuba Ober <[EMAIL PROTECTED]>"
#define DRIVER_DESC "USB FTDI Serial Converters Driver"
@@ -684,6 +688,8 @@
char prev_status, diff_status; /* Used for TIOCMIWAIT */
__u8 rx_flags; /* receive state flags (throttling) */
spinlock_t rx_lock; /* spinlock for receive state */
+ struct work_struct rx_work;
+ int rx_processed;
__u16 interface; /* FT2232C port interface (0 for FT232/245) */
@@ -714,7 +720,7 @@
static int ftdi_chars_in_buffer (struct usb_serial_port *port);
static void ftdi_write_bulk_callback (struct urb *urb, struct pt_regs *regs);
static void ftdi_read_bulk_callback (struct urb *urb, struct pt_regs *regs);
-static void ftdi_process_read (struct usb_serial_port *port);
+static void ftdi_process_read (void *param);
static void ftdi_set_termios (struct usb_serial_port *port, struct termios * old);
static int ftdi_tiocmget (struct usb_serial_port *port, struct file *file);
static int ftdi_tiocmset (struct usb_serial_port *port, struct file * file, unsigned int set, unsigned int clear);
@@ -1384,6 +1390,8 @@
port->read_urb->transfer_buffer_length = BUFSZ;
}
+ INIT_WORK(&priv->rx_work, ftdi_process_read, port);
+
/* Free port's existing write urb and transfer buffer. */
if (port->write_urb) {
usb_free_urb (port->write_urb);
@@ -1614,6 +1622,7 @@
spin_unlock_irqrestore(&priv->rx_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,
@@ -1664,6 +1673,10 @@
err("Error from RTS LOW urb");
}
} /* Note change no line if hupcl is off */
+
+ /* cancel any scheduled reading */
+ cancel_delayed_work(&priv->rx_work);
+ flush_scheduled_work();
/* shutdown our bulk read */
if (port->read_urb)
@@ -1859,23 +1872,14 @@
return;
}
- /* If throttled, delay receive processing until unthrottled. */
- spin_lock(&priv->rx_lock);
- if (priv->rx_flags & THROTTLED) {
- dbg("Deferring read urb processing until unthrottled");
- priv->rx_flags |= ACTUALLY_THROTTLED;
- spin_unlock(&priv->rx_lock);
- return;
- }
- spin_unlock(&priv->rx_lock);
-
ftdi_process_read(port);
} /* ftdi_read_bulk_callback */
-static void ftdi_process_read (struct usb_serial_port *port)
+static void ftdi_process_read (void *param)
{ /* ftdi_process_read */
+ struct usb_serial_port *port = (struct usb_serial_port*)param;
struct urb *urb;
struct tty_struct *tty;
struct ftdi_private *priv;
@@ -1886,6 +1890,7 @@
int result;
int need_flip;
int packet_offset;
+ unsigned long flags;
dbg("%s - port %d", __FUNCTION__, port->number);
@@ -1912,12 +1917,18 @@
data = urb->transfer_buffer;
- /* The first two bytes of every read packet are status */
- if (urb->actual_length > 2) {
- usb_serial_debug_data(debug, &port->dev, __FUNCTION__, urb->actual_length, data);
+ if (priv->rx_processed) {
+ dbg("%s - already processed: %d bytes, %d remain", __FUNCTION__,
+ priv->rx_processed,
+ urb->actual_length - priv->rx_processed);
} else {
- dbg("Status only: %03oo %03oo",data[0],data[1]);
- }
+ /* The first two bytes of every read packet are status */
+ if (urb->actual_length > 2) {
+ usb_serial_debug_data(debug, &port->dev, __FUNCTION__, urb->actual_length, data);
+ } else {
+ dbg("Status only: %03oo %03oo",data[0],data[1]);
+ }
+ }
/* TO DO -- check for hung up line and handle appropriately: */
@@ -1926,8 +1937,12 @@
/* if CD is dropped and the line is not CLOCAL then we should hangup */
need_flip = 0;
- for (packet_offset=0; packet_offset < urb->actual_length; packet_offset += PKTSZ) {
+ for (packet_offset = priv->rx_processed; packet_offset < urb->actual_length; packet_offset += PKTSZ) {
+ 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. */
if (priv != NULL) {
char new_status = data[packet_offset+0] & FTDI_STATUS_B0_MASK;
if (new_status != priv->prev_status) {
@@ -1937,6 +1952,35 @@
}
}
+ length = min(PKTSZ, urb->actual_length-packet_offset)-2;
+ if (length < 0) {
+ err("%s - bad packet length: %d", __FUNCTION__, length+2);
+ length = 0;
+ }
+
+ /* have to make sure we don't overflow the buffer
+ with tty_insert_flip_char's */
+ if (tty->flip.count+length > TTY_FLIPBUF_SIZE) {
+ tty_flip_buffer_push(tty);
+ need_flip = 0;
+
+ if (tty->flip.count != 0) {
+ /* flip didn't work, this happens when ftdi_process_read() is
+ * called from ftdi_unthrottle, because TTY_DONT_FLIP is set */
+ dbg("%s - flip buffer push failed", __FUNCTION__);
+ break;
+ }
+ }
+ if (priv->rx_flags & THROTTLED) {
+ dbg("%s - throttled", __FUNCTION__);
+ break;
+ }
+ if (tty->ldisc.receive_room(tty)-tty->flip.count < length) {
+ /* break out & wait for throttling/unthrottling to happen */
+ dbg("%s - receive room low", __FUNCTION__);
+ break;
+ }
+
/* Handle errors and break */
error_flag = TTY_NORMAL;
/* Although the device uses a bitmask and hence can have multiple */
@@ -1959,13 +2003,8 @@
error_flag = TTY_FRAME;
dbg("FRAMING error");
}
- if (urb->actual_length > packet_offset + 2) {
- for (i = 2; (i < PKTSZ) && ((i+packet_offset) < urb->actual_length); ++i) {
- /* have to make sure we don't overflow the buffer
- with tty_insert_flip_char's */
- if(tty->flip.count >= TTY_FLIPBUF_SIZE) {
- tty_flip_buffer_push(tty);
- }
+ 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 */
@@ -2002,6 +2041,35 @@
tty_flip_buffer_push(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",
+ __FUNCTION__, 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",
+ __FUNCTION__);
+ return;
+ }
+ spin_unlock_irqrestore(&priv->rx_lock, flags);
+ /* if the port is closed stop trying to read */
+ if (port->open_count > 0){
+ /* delay processing of remainder */
+ schedule_delayed_work(&priv->rx_work, 1);
+ } else {
+ dbg("%s - port is closed", __FUNCTION__);
+ }
+ return;
+ }
+
+ /* urb is completely processed */
+ priv->rx_processed = 0;
+
/* if the port is closed stop trying to read */
if (port->open_count > 0){
/* Continue trying to always read */
@@ -2441,7 +2509,7 @@
spin_unlock_irqrestore(&priv->rx_lock, flags);
if (actually_throttled)
- ftdi_process_read(port);
+ schedule_work(&priv->rx_work);
}
static int __init ftdi_init (void)
