Re: Serial driver hangs

[Paul Fulghum]

On Fri, 2004-10-01 at 10:22, Roland CaÃebohm wrote:
> Yes, I think you are right, if the system is to slow to fetch 
> the data fast enough the buffer will be sometime full. And if 
> it would be possible to use the second flip buffer then, this 
> buffer would be full too sometime.
> It would just take a little longer till data got lost. But if 
> I want that I could just make the buffers larger.
> 
> ...
> 
> I have just tested it, but unfortunately I've got a very bad 
> result. :-( In my test case (2 port with 921600 baud) I get 
> very much data loss.

Roland:

Can I impose on you to try the following patches?

The differences here are:
* don't call flush_to_ldisc() directly from ISR
* flush_to_ldisc() keeps flushing flip buffers until empty

These patches are for testing purposes only
and not intended for general use.
I would like to see how your high speed setup reacts.

Thanks in advance.

-- 
Paul Fulghum
paulkf@xxxxxxxxxxxxx

--- a/drivers/char/serial.c	2004-09-30 15:25:17.000000000 -0500
+++ b/drivers/char/serial.c	2004-10-01 10:42:33.000000000 -0500
@@ -572,9 +572,17 @@ static _INLINE_ void receive_chars(struc
 	icount = &info->state->icount;
 	do {
 		if (tty->flip.count >= TTY_FLIPBUF_SIZE) {
-			tty->flip.tqueue.routine((void *) tty);
-			if (tty->flip.count >= TTY_FLIPBUF_SIZE)
-				return;		// if TTY_DONT_FLIP is set
+			/* no room in flip buffer, discard rx FIFO contents to clear IRQ
+			 * *FIXME* Hardware with auto flow control
+			 * would benefit from leaving the data in the FIFO and
+			 * disabling the rx IRQ until space becomes available.
+			 */
+			do {
+				serial_inp(info, UART_RX);
+				icount->overrun++;
+				*status = serial_inp(info, UART_LSR);
+			} while ((*status & UART_LSR_DR) && (max_count-- > 0));
+			return;		// if TTY_DONT_FLIP is set
 		}
 		ch = serial_inp(info, UART_RX);
 		*tty->flip.char_buf_ptr = ch;
--- a/drivers/char/tty_io.c	2004-04-14 08:05:29.000000000 -0500
+++ b/drivers/char/tty_io.c	2004-10-01 10:49:45.000000000 -0500
@@ -1944,32 +1944,34 @@ static void flush_to_ldisc(void *private
 	int		count;
 	unsigned long flags;
 
-	if (test_bit(TTY_DONT_FLIP, &tty->flags)) {
-		queue_task(&tty->flip.tqueue, &tq_timer);
-		return;
-	}
-	if (tty->flip.buf_num) {
-		cp = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
-		fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
-		tty->flip.buf_num = 0;
-
-		save_flags(flags); cli();
-		tty->flip.char_buf_ptr = tty->flip.char_buf;
-		tty->flip.flag_buf_ptr = tty->flip.flag_buf;
-	} else {
-		cp = tty->flip.char_buf;
-		fp = tty->flip.flag_buf;
-		tty->flip.buf_num = 1;
-
-		save_flags(flags); cli();
-		tty->flip.char_buf_ptr = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
-		tty->flip.flag_buf_ptr = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
-	}
-	count = tty->flip.count;
-	tty->flip.count = 0;
-	restore_flags(flags);
+	while(tty->flip.count) {
+		if (test_bit(TTY_DONT_FLIP, &tty->flags)) {
+			queue_task(&tty->flip.tqueue, &tq_timer);
+			return;
+		}
+		if (tty->flip.buf_num) {
+			cp = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
+			fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
+			tty->flip.buf_num = 0;
+
+			save_flags(flags); cli();
+			tty->flip.char_buf_ptr = tty->flip.char_buf;
+			tty->flip.flag_buf_ptr = tty->flip.flag_buf;
+		} else {
+			cp = tty->flip.char_buf;
+			fp = tty->flip.flag_buf;
+			tty->flip.buf_num = 1;
+
+			save_flags(flags); cli();
+			tty->flip.char_buf_ptr = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
+			tty->flip.flag_buf_ptr = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
+		}
+		count = tty->flip.count;
+		tty->flip.count = 0;
+		restore_flags(flags);
 	
-	tty->ldisc.receive_buf(tty, cp, fp, count);
+		tty->ldisc.receive_buf(tty, cp, fp, count);
+	}
 }
 
 /*


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/