Subject: port-i386/1082: /sys/dev/isa/com.c bugfixes and improvements
To: None <gnats-admin@sun-lamp.cs.berkeley.edu>
From: Felix A. Croes <felix@Simplex.NL>
List: netbsd-bugs
Date: 05/28/1995 05:35:04
>Number: 1082
>Category: port-i386
>Synopsis: com.c: add input fifo and get handshaking to work
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: gnats-admin (GNATS administrator)
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sun May 28 05:35:03 1995
>Originator: Felix A. Croes
>Organization:
>Release: NetBSD-current as of May 28 13:41:00 MET
>Environment:
NetBSD 1.0A i386
System: NetBSD XS1.Simplex.NL 1.0A NetBSD 1.0A (SIMPLEX_TMP) #17: Mon Apr 24 01:19:22 MET DST 1995 felix@XS1.Simplex.NL:/usr/src/sys/arch/i386/compile/SIMPLEX_TMP i386
>Description:
Problems:
- comintr passes on incoming characters to the tty level immediately,
so a series of input interrupts is likely to lead to silo overflows
- fifos are not drained when a com port is opened
- critical code in comclose is done without disabling interrupts;
moreover the tty state is not cleared on a close, resulting in
hanging processes waiting for tty output to be done on a device
already closed
- ioctl TIOCSDTR and TIOCCDTR should not affect RTS if RTS/CTS
handshaking is used; RTS is messed up in many other places also
- the tty level should handle MDMBUF, not the device
- handshaking check erroneously disabled with #if 0 in comstart
>How-To-Repeat:
These are all known bugs.
>Fix:
Patches included below, general description:
- add a fifo for input, so the interrupt doesn't drive the tty level
immediately. The input is passed on to the tty level by a polling
routine
- if handshaking is enabled, turn off RTS when the high water mark
in the input fifo is reached
- fix up all the cases in the code where RTS is messed up
- code cleanup and fixes for the other problems mentioned above
The patches were tested for a month on a 486DX2/66 running NetBSD
1.0A, with 8 14k4 modem lines (2 ASTs). Before the patches were
applied, silo overflows were frequent and rts/cts handshaking did
not work. Since they were applied, these problems were corrected.
The input fifo is fast enough that RTS is not turned off in normal
operation on our machine.
I've taken a good look at the FreeBSD serial driver before making
these patches. My fixes include most of the important FreeBSD
ones, but I've avoided the hacks (such as bypassing the tty level
in raw mode and placing characters directly in the tty queue).
The FreeBSD driver maintains a complicated internal state so the
fastest way of performing some action is always known. I've not
copied that, either.
*** com.c.orig Sun May 28 12:15:00 1995
--- com.c Sun May 28 12:35:24 1995
***************
*** 62,73 ****
--- 62,79 ----
#include <dev/isa/comreg.h>
#include <dev/ic/ns16550.h>
+ #define RS_IBUFSIZE (2 * 256)
+ #define RS_IHIGHWATER ((3 * RS_IBUFSIZE) / 4)
+
struct com_softc {
struct device sc_dev;
void *sc_ih;
struct tty *sc_tty;
int sc_overflows;
+ int sc_floods;
+ int sc_errors;
+
int sc_iobase;
u_char sc_hwflags;
#define COM_HW_NOIEN 0x01
***************
*** 79,84 ****
--- 85,93 ----
#define COM_SW_CRTSCTS 0x04
#define COM_SW_MDMBUF 0x08
u_char sc_msr, sc_mcr;
+
+ u_char *sc_ibuf, *sc_ibufp, *sc_ibufhigh, *sc_ibufend;
+ u_char sc_ibuf1[RS_IBUFSIZE], sc_ibuf2[RS_IBUFSIZE];
};
int comprobe __P((struct device *, void *, void *));
***************
*** 87,92 ****
--- 96,102 ----
int comclose __P((dev_t, int, int, struct proc *));
void comdiag __P((void *));
int comintr __P((void *));
+ void compoll __P((void *));
int comparam __P((struct tty *, struct termios *));
void comstart __P((struct tty *));
***************
*** 182,188 ****
--- 192,202 ----
struct cfdata *cf = sc->sc_dev.dv_cfdata;
int iobase = ia->ia_iobase;
struct tty *tp;
+ static int pollstarted;
+ sc->sc_ibufp = sc->sc_ibuf = sc->sc_ibuf1;
+ sc->sc_ibufhigh = sc->sc_ibuf1 + RS_IHIGHWATER;
+ sc->sc_ibufend = sc->sc_ibuf1 + RS_IBUFSIZE;
sc->sc_iobase = iobase;
sc->sc_hwflags = cf->cf_flags & COM_HW_NOIEN;
sc->sc_swflags = 0;
***************
*** 241,246 ****
--- 255,266 ----
sc->sc_hwflags |= COM_HW_CONSOLE;
sc->sc_swflags |= COM_SW_SOFTCAR;
}
+
+ if (!pollstarted) {
+ /* start polling */
+ pollstarted = 1;
+ compoll(NULL);
+ }
}
int
***************
*** 290,300 ****
ttsetwater(tp);
iobase = sc->sc_iobase;
! /* Set the FIFO threshold based on the receive speed. */
! if (sc->sc_hwflags & COM_HW_FIFO)
! outb(iobase + com_fifo,
! FIFO_ENABLE | FIFO_RCV_RST | FIFO_XMT_RST |
! (tp->t_ispeed <= 1200 ? FIFO_TRIGGER_1 : FIFO_TRIGGER_8));
/* flush any pending I/O */
(void) inb(iobase + com_lsr);
(void) inb(iobase + com_data);
--- 310,330 ----
ttsetwater(tp);
iobase = sc->sc_iobase;
! if (sc->sc_hwflags & COM_HW_FIFO) {
! /* Drain FIFOs. */
! for (;;) {
! outb(iobase + com_fifo,
! FIFO_ENABLE | FIFO_RCV_RST | FIFO_XMT_RST |
! (tp->t_ispeed <= 4800 ?
! FIFO_TRIGGER_1 : FIFO_TRIGGER_14));
! delay(100);
! if (!(inb(iobase + com_lsr) & LSR_RXRDY))
! break;
! outb(iobase + com_fifo, 0);
! delay(100);
! (void) inb(iobase + com_data);
! }
! }
/* flush any pending I/O */
(void) inb(iobase + com_lsr);
(void) inb(iobase + com_data);
***************
*** 346,353 ****
--- 376,385 ----
struct com_softc *sc = comcd.cd_devs[unit];
struct tty *tp = sc->sc_tty;
int iobase = sc->sc_iobase;
+ int s;
(*linesw[tp->t_line].l_close)(tp, flag);
+ s = spltty();
#ifdef KGDB
/* do not disable interrupts if debugging */
if (kgdb_dev != makedev(commajor, unit))
***************
*** 360,365 ****
--- 392,399 ----
/* XXX perhaps only clear DTR */
outb(iobase + com_mcr, 0);
}
+ tp->t_state &= ~(TS_BUSY | TS_FLUSH);
+ splx(s);
ttyclose(tp);
#ifdef notyet /* XXXX */
if (unit != comconsole) {
***************
*** 446,455 ****
bic(iobase + com_cfcr, CFCR_SBREAK);
break;
case TIOCSDTR:
! outb(iobase + com_mcr, sc->sc_mcr |= (MCR_DTR | MCR_RTS));
break;
case TIOCCDTR:
! outb(iobase + com_mcr, sc->sc_mcr &= ~(MCR_DTR | MCR_RTS));
break;
case TIOCMSET:
sc->sc_mcr &= ~(MCR_DTR | MCR_RTS);
--- 480,493 ----
bic(iobase + com_cfcr, CFCR_SBREAK);
break;
case TIOCSDTR:
! outb(iobase + com_mcr,
! sc->sc_mcr |= (tp->t_cflag & CRTSCTS) ?
! MCR_DTR : (MCR_DTR | MCR_RTS));
break;
case TIOCCDTR:
! outb(iobase + com_mcr,
! sc->sc_mcr &= (tp->t_cflag & CRTSCTS) ?
! ~MCR_DTR : ~(MCR_DTR | MCR_RTS));
break;
case TIOCMSET:
sc->sc_mcr &= ~(MCR_DTR | MCR_RTS);
***************
*** 536,541 ****
--- 574,580 ----
int iobase = sc->sc_iobase;
int ospeed = comspeed(t->c_ospeed);
u_char cfcr;
+ unsigned long oldmdm;
int s;
/* check requested parameters */
***************
*** 581,587 ****
if (sc->sc_hwflags & COM_HW_FIFO)
outb(iobase + com_fifo,
FIFO_ENABLE | FIFO_RCV_RST | FIFO_XMT_RST |
! (t->c_ispeed <= 1200 ? FIFO_TRIGGER_1 : FIFO_TRIGGER_8));
}
outb(iobase + com_cfcr, cfcr | CFCR_DLAB);
--- 620,626 ----
if (sc->sc_hwflags & COM_HW_FIFO)
outb(iobase + com_fifo,
FIFO_ENABLE | FIFO_RCV_RST | FIFO_XMT_RST |
! (t->c_ispeed <= 4800 ? FIFO_TRIGGER_1 : FIFO_TRIGGER_14));
}
outb(iobase + com_cfcr, cfcr | CFCR_DLAB);
***************
*** 603,640 ****
}
}
- /*
- * If CTS is off and CRTSCTS is changed, we must toggle TS_TTSTOP.
- * XXX should be done at tty layer.
- */
- if ((sc->sc_msr & MSR_CTS) == 0 &&
- (tp->t_cflag & CRTSCTS) != (t->c_cflag & CRTSCTS)) {
- if ((t->c_cflag & CRTSCTS) == 0) {
- tp->t_state &= ~TS_TTSTOP;
- (*linesw[tp->t_line].l_start)(tp);
- } else
- tp->t_state |= TS_TTSTOP;
- }
-
- /*
- * If DCD is off and MDMBUF is changed, we must toggle TS_TTSTOP.
- * XXX should be done at tty layer.
- */
- if ((sc->sc_swflags & COM_SW_SOFTCAR) == 0 &&
- (sc->sc_msr & MSR_DCD) == 0 &&
- (tp->t_cflag & MDMBUF) != (t->c_cflag & MDMBUF)) {
- if ((t->c_cflag & MDMBUF) == 0) {
- tp->t_state &= ~TS_TTSTOP;
- (*linesw[tp->t_line].l_start)(tp);
- } else
- tp->t_state |= TS_TTSTOP;
- }
-
/* and copy to tty */
tp->t_ispeed = t->c_ispeed;
tp->t_ospeed = t->c_ospeed;
tp->t_cflag = t->c_cflag;
splx(s);
return 0;
}
--- 642,664 ----
}
}
/* and copy to tty */
tp->t_ispeed = t->c_ispeed;
tp->t_ospeed = t->c_ospeed;
+ oldmdm = tp->t_cflag & MDMBUF;
tp->t_cflag = t->c_cflag;
+ /*
+ * If DCD is off and MDMBUF is changed, let the tty level decide if
+ * the line should be turned off.
+ */
+ if ((sc->sc_swflags & COM_SW_SOFTCAR) == 0 &&
+ (sc->sc_msr & MSR_DCD) == 0 && oldmdm != (t->c_cflag & MDMBUF) &&
+ (*linesw[tp->t_line].l_modem)(tp, oldmdm) == 0)
+ outb(iobase + com_mcr,
+ sc->sc_mcr &= (tp->t_cflag & CRTSCTS) ?
+ ~MCR_DTR : ~(MCR_DTR | MCR_RTS));
+
splx(s);
return 0;
}
***************
*** 648,659 ****
int s;
s = spltty();
! if (tp->t_state & (TS_TTSTOP | TS_BUSY))
goto out;
! #if 0 /* XXXX I think this is handled adequately by commint() and comparam(). */
! if (tp->t_cflag & CRTSCTS && (sc->sc_mcr & MSR_CTS) == 0)
goto out;
- #endif
if (tp->t_outq.c_cc <= tp->t_lowat) {
if (tp->t_state & TS_ASLEEP) {
tp->t_state &= ~TS_ASLEEP;
--- 672,681 ----
int s;
s = spltty();
! if (tp->t_cflag & CRTSCTS && (sc->sc_msr & MSR_CTS) == 0)
goto out;
! if (tp->t_state & (TS_TTSTOP | TS_BUSY))
goto out;
if (tp->t_outq.c_cc <= tp->t_lowat) {
if (tp->t_state & TS_ASLEEP) {
tp->t_state &= ~TS_ASLEEP;
***************
*** 694,699 ****
--- 716,787 ----
splx(s);
}
+ static int comevents; /* # input characters */
+
+ void
+ compoll(arg)
+ void *arg;
+ {
+ int s;
+ int unit;
+ struct com_softc *sc;
+ struct tty *tp;
+
+ timeout(compoll, NULL, (hz > 100) ? hz / 100 : 1);
+
+ if (comevents > 0) {
+ s = splsofttty();
+ for (unit = 0; unit < comcd.cd_ndevs; unit++) {
+ sc = comcd.cd_devs[unit];
+ if (sc != NULL && sc->sc_ibufp > sc->sc_ibuf) {
+ int softs;
+ u_char *ibufp, *ibufend;
+ int c;
+
+ tp = com_tty[unit];
+
+ /* switch input buffers */
+ softs = spltty();
+ ibufp = sc->sc_ibuf;
+ ibufend = sc->sc_ibufp;
+ sc->sc_ibuf = (sc->sc_ibuf == sc->sc_ibuf1) ?
+ sc->sc_ibuf2 : sc->sc_ibuf1;
+ sc->sc_ibufp = sc->sc_ibuf;
+ sc->sc_ibufhigh = sc->sc_ibuf + RS_IHIGHWATER;
+ sc->sc_ibufend = sc->sc_ibuf + RS_IBUFSIZE;
+ comevents -= (ibufend - ibufp) / 2;
+ splx(softs);
+
+ if (tp == NULL || !(tp->t_state & TS_ISOPEN))
+ continue;
+
+ /* reallow input if needed */
+ if (tp->t_cflag & CRTSCTS &&
+ (sc->sc_mcr & MCR_RTS) == 0)
+ outb(sc->sc_iobase + com_mcr,
+ sc->sc_mcr |= MCR_RTS);
+
+ /* process input */
+ while (ibufp < ibufend) {
+ c = *ibufp++;
+ if (*ibufp & (LSR_BI | LSR_FE))
+ c |= TTY_FE;
+ else if (*ibufp & LSR_PE)
+ c |= TTY_PE;
+ if (*ibufp & LSR_OE) {
+ sc->sc_overflows++;
+ if (sc->sc_errors++ == 0)
+ timeout(comdiag, sc, 60 * hz);
+ }
+ ibufp++;
+ (*linesw[tp->t_line].l_rint)(c, tp);
+ }
+ }
+ }
+ splx(s);
+ }
+ }
+
static inline void
comeint(sc, stat)
struct com_softc *sc;
***************
*** 719,734 ****
#endif
return;
}
! if (stat & (LSR_BI | LSR_FE))
! c |= TTY_FE;
! else if (stat & LSR_PE)
! c |= TTY_PE;
! if (stat & LSR_OE) {
! if (sc->sc_overflows++ == 0)
timeout(comdiag, sc, 60 * hz);
}
- /* XXXX put in FIFO and process later */
- (*linesw[tp->t_line].l_rint)(c, tp);
}
static inline void
--- 807,823 ----
#endif
return;
}
! if (sc->sc_ibufp >= sc->sc_ibufend) {
! sc->sc_floods++;
! if (sc->sc_errors++ == 0)
timeout(comdiag, sc, 60 * hz);
+ } else {
+ *(sc->sc_ibufp)++ = c;
+ *(sc->sc_ibufp)++ = stat;
+ if (sc->sc_ibufp == sc->sc_ibufhigh && tp->t_cflag & CRTSCTS)
+ outb(iobase + com_mcr, sc->sc_mcr &= ~MCR_RTS);
+ comevents++;
}
}
static inline void
***************
*** 743,763 ****
delta = msr ^ sc->sc_msr;
sc->sc_msr = msr;
! if (delta & MSR_DCD && (sc->sc_swflags & COM_SW_SOFTCAR) == 0) {
! if (msr & MSR_DCD)
! (void)(*linesw[tp->t_line].l_modem)(tp, 1);
! else if ((*linesw[tp->t_line].l_modem)(tp, 0) == 0)
! outb(iobase + com_mcr,
! sc->sc_mcr &= ~(MCR_DTR | MCR_RTS));
! }
! if (delta & MSR_CTS && tp->t_cflag & CRTSCTS) {
/* the line is up and we want to do rts/cts flow control */
! if (msr & MSR_CTS) {
! tp->t_state &= ~TS_TTSTOP;
! (*linesw[tp->t_line].l_start)(tp);
! } else
! tp->t_state |= TS_TTSTOP;
! }
}
void
--- 832,846 ----
delta = msr ^ sc->sc_msr;
sc->sc_msr = msr;
! if (delta & MSR_DCD && (sc->sc_swflags & COM_SW_SOFTCAR) == 0 &&
! (*linesw[tp->t_line].l_modem)(tp, msr & MSR_DCD) == 0)
! outb(iobase + com_mcr,
! sc->sc_mcr &= (tp->t_cflag & CRTSCTS) ?
! ~MCR_DTR : ~(MCR_DTR | MCR_RTS));
!
! if (delta & msr & MSR_CTS && tp->t_cflag & CRTSCTS)
/* the line is up and we want to do rts/cts flow control */
! (*linesw[tp->t_line].l_start)(tp);
}
void
***************
*** 765,781 ****
void *arg;
{
struct com_softc *sc = arg;
! int overflows;
int s;
s = spltty();
overflows = sc->sc_overflows;
sc->sc_overflows = 0;
splx(s);
if (overflows)
log(LOG_WARNING, "%s: %d silo overflow%s\n",
sc->sc_dev.dv_xname, overflows, overflows == 1 ? "" : "s");
}
int
--- 848,869 ----
void *arg;
{
struct com_softc *sc = arg;
! int overflows, floods;
int s;
s = spltty();
overflows = sc->sc_overflows;
+ floods = sc->sc_floods;
sc->sc_overflows = 0;
+ sc->sc_floods = 0;
splx(s);
if (overflows)
log(LOG_WARNING, "%s: %d silo overflow%s\n",
sc->sc_dev.dv_xname, overflows, overflows == 1 ? "" : "s");
+ if (floods)
+ log(LOG_WARNING, "%s: %d ibuf overflow%s\n",
+ sc->sc_dev.dv_xname, floods, floods == 1 ? "" : "s");
}
int
***************
*** 794,810 ****
for (;;) {
if (code & IIR_RXRDY) {
tp = sc->sc_tty;
- /* XXXX put in FIFO and process later */
while (code = (inb(iobase + com_lsr) & LSR_RCV_MASK)) {
! if (code == LSR_RXRDY) {
code = inb(iobase + com_data);
! if (tp->t_state & TS_ISOPEN)
! (*linesw[tp->t_line].l_rint)(code, tp);
#ifdef KGDB
! else {
! if (kgdb_dev == makedev(commajor, unit) &&
! code == FRAME_END)
! kgdb_connect(0);
}
#endif
} else
--- 882,908 ----
for (;;) {
if (code & IIR_RXRDY) {
tp = sc->sc_tty;
while (code = (inb(iobase + com_lsr) & LSR_RCV_MASK)) {
! if (code & LSR_RXRDY) {
code = inb(iobase + com_data);
! if (sc->sc_ibufp >= sc->sc_ibufend) {
! sc->sc_floods++;
! if (sc->sc_errors++ == 0)
! timeout(comdiag, sc, 60 * hz);
! } else {
! *(sc->sc_ibufp)++ = code;
! *(sc->sc_ibufp)++ = 0;
! if (sc->sc_ibufp == sc->sc_ibufhigh &&
! tp->t_cflag & CRTSCTS)
! outb(iobase + com_mcr,
! sc->sc_mcr &= ~MCR_RTS);
! comevents++;
! }
#ifdef KGDB
! if (!(tp->t_state & TS_ISOPEN) &&
! kgdb_dev == makedev(commajor, unit) &&
! code == FRAME_END)
! kgdb_connect(0);
}
#endif
} else
>Audit-Trail:
>Unformatted: