>Number:         2046
>Category:       kern
>Synopsis:       Workaround in com device driver for broken UARTs
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Wed Feb  7 20:50:15 1996
>Last-Modified:
>Originator:     Pete Bentley
>Organization:
Demon Internet
	
>Release:        5 Feb 96
>Environment:
	i386, various PCI motherboards
System: NetBSD netbsd.eng.demon.net 1.1A NetBSD 1.1A (NETBSD-ENG) #0: Fri Jan 19 08:27:23 GMT 1996 ronald@netbsd.eng.demon.net:/sys/arch/i386/compile/NETBSD-ENG i386


>Description:
Various 16550 clones (esp. on some PCI motherboard chipsets) lock up if
the baud rate divisor latches are accessed when the transmit FIFO is not
empty.  Changing the line settings (eg tcseta()) while the output queue
is not empty is likely to tickle this hardware bug and result in a hung
terminal line and an unkillable process sleeping for the output to complete.

>How-To-Repeat:
Often can be repeated simply by typing quickly when logging in.
Certainly running bash (which saves and restores line settings
around each command) on a serial line with such a UART will hang
it very quickly.

>Fix:
Correct fix is to wait for the 16550 FIFO (but not the whole tty output
queue) to drain before accessing the baud rate divisor latches. Code to
implement this in the FreeBSD sio driver runs to about 50 lines which
do not easily fit the structure of NetBSD com.c

Unidiff for a patch which works in 99% of all cases follows. What it does
is keep a soft copy of the contents of the divisor latches and only
reprogram the latches if the value needs to be changed (current comparam()
routine always sets them regardless).  Thus there is only a risk of hanging
the terminal line when changing the vaud rate (which is never on many
directly connected lines)


--- /sys/dev/isa/com.c	Thu Jan 18 15:11:51 1996
+++ com.c	Wed Feb  7 12:58:43 1996
@@ -113,6 +113,7 @@
 #define	COM_SW_MDMBUF	0x08
 	u_char sc_msr, sc_mcr, sc_lcr;
 	u_char sc_dtr;
+	int sc_rate;
 
 	u_char *sc_ibuf, *sc_ibufp, *sc_ibufhigh, *sc_ibufend;
 	u_char sc_ibufs[2][COM_IBUFSIZE];
@@ -394,6 +395,7 @@
 
 		s = spltty();
 
+		sc->sc_rate = 0;			/* Force rate to be set on open */
 		comparam(tp, &tp->t_termios);
 		ttsetwater(tp);
 
@@ -732,12 +734,20 @@
 	}
 
 	if (ospeed != 0) {
-		outb(iobase + com_lcr, lcr | LCR_DLAB);
-		outb(iobase + com_dlbl, ospeed);
-		outb(iobase + com_dlbh, ospeed >> 8);
-		outb(iobase + com_lcr, lcr);
-		SET(sc->sc_mcr, MCR_DTR);
-		outb(iobase + com_mcr, sc->sc_mcr);
+		/*
+		 * Only set divisors if the speed has really changed. Works around
+		 * bug in some UARTs which freeze if you touch the divisor latches
+		 * while the FIFO is non empty
+		 */
+		if ( ospeed != sc->sc_rate ) {
+			outb(iobase + com_lcr, lcr | LCR_DLAB);
+			outb(iobase + com_dlbl, ospeed);
+			outb(iobase + com_dlbh, ospeed >> 8);
+			outb(iobase + com_lcr, lcr);
+			SET(sc->sc_mcr, MCR_DTR);
+			outb(iobase + com_mcr, sc->sc_mcr);
+			sc->sc_rate = ospeed;
+		}
 	} else
 		outb(iobase + com_lcr, lcr);
 

>Audit-Trail:
>Unformatted: