Subject: Re: port-i386/1082: /sys/dev/isa/com.c bugfixes and improvements
To: None <felix@Simplex.NL>
From: Charles M. Hannum <mycroft@ai.mit.edu>
List: netbsd-bugs
Date: 06/04/1995 07:38:54
A few comments:
1) Your patch is against a fairly old com.c.
2) Your modified comdiag() doesn't reset sc_errors, so you will not
observe any further overflows.
3) There are a few places that can be faster; e.g. by insuring that a
value is cached in a register. Also, comeint() can simply go away.
4) Your change to comparam() is slightly wrong; the second arg to
l_modem() in that case should always be 0. Otherwise, TS_CARR_ON may
be incorrectly set when turning off MDMBUF.
5) The way you use `comevents' seems more complicated (and thus
slower) than necessary.
Enclosed is a new set of diffs against the -current com.c. I can't
test this very easily, so it would be nice if someone else did.
Index: com.c
===================================================================
RCS file: /a/cvsroot/src/sys/dev/isa/com.c,v
retrieving revision 1.56
diff -c -2 -r1.56 com.c
*** com.c 1995/06/01 21:26:51 1.56
--- com.c 1995/06/04 11:32:41
***************
*** 63,66 ****
--- 63,69 ----
#include <dev/ic/ns16550.h>
+ #define COM_IBUFSIZE (2 * 256)
+ #define COM_IHIGHWATER ((3 * COM_IBUFSIZE) / 4)
+
struct com_softc {
struct device sc_dev;
***************
*** 69,72 ****
--- 72,78 ----
int sc_overflows;
+ int sc_floods;
+ int sc_errors;
+
int sc_iobase;
u_char sc_hwflags;
***************
*** 80,83 ****
--- 86,92 ----
#define COM_SW_MDMBUF 0x08
u_char sc_msr, sc_mcr;
+
+ u_char *sc_ibuf, *sc_ibufp, *sc_ibufhigh, *sc_ibufend;
+ u_char sc_ibufs[2][COM_IBUFSIZE];
};
***************
*** 88,91 ****
--- 97,101 ----
void comdiag __P((void *));
int comintr __P((void *));
+ void compoll __P((void *));
int comparam __P((struct tty *, struct termios *));
void comstart __P((struct tty *));
***************
*** 103,106 ****
--- 113,118 ----
int comconsinit;
int commajor;
+ int comsopen = 0;
+ int comevents = 0;
#ifdef KGDB
***************
*** 263,268 ****
return ENXIO;
- s = spltty();
-
if (!sc->sc_tty)
tp = sc->sc_tty = ttymalloc();
--- 275,278 ----
***************
*** 287,293 ****
--- 297,313 ----
tp->t_lflag = TTYDEF_LFLAG;
tp->t_ispeed = tp->t_ospeed = comdefaultrate;
+
+ s = spltty();
+
comparam(tp, &tp->t_termios);
ttsetwater(tp);
+ if (comsopen++ == 0)
+ timeout(compoll, NULL, 1);
+
+ sc->sc_ibufp = sc->sc_ibuf = sc->sc_ibufs[0];
+ sc->sc_ibufhigh = sc->sc_ibuf + COM_IHIGHWATER;
+ sc->sc_ibufend = sc->sc_ibuf + COM_IBUFSIZE;
+
iobase = sc->sc_iobase;
/* Set the FIFO threshold based on the receive speed. */
***************
*** 297,302 ****
(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);
/* you turn me on, baby */
sc->sc_mcr = MCR_DTR | MCR_RTS;
--- 317,322 ----
(tp->t_ispeed <= 1200 ? FIFO_TRIGGER_1 : FIFO_TRIGGER_8));
/* flush any pending I/O */
! while (inb(iobase + com_lsr) & LSR_RXRDY)
! (void) inb(iobase + com_data);
/* you turn me on, baby */
sc->sc_mcr = MCR_DTR | MCR_RTS;
***************
*** 314,320 ****
tp->t_state &= ~TS_CARR_ON;
} else if (tp->t_state&TS_XCLUDE && p->p_ucred->cr_uid != 0) {
- splx(s);
return EBUSY;
! }
/* wait for carrier if necessary */
--- 334,340 ----
tp->t_state &= ~TS_CARR_ON;
} else if (tp->t_state&TS_XCLUDE && p->p_ucred->cr_uid != 0) {
return EBUSY;
! } else
! s = spltty();
/* wait for carrier if necessary */
***************
*** 347,364 ****
struct tty *tp = sc->sc_tty;
int iobase = sc->sc_iobase;
(*linesw[tp->t_line].l_close)(tp, flag);
! #ifdef KGDB
! /* do not disable interrupts if debugging */
! if (kgdb_dev != makedev(commajor, unit))
! #endif
! {
! bic(iobase + com_cfcr, CFCR_SBREAK);
! outb(iobase + com_ier, 0);
! if (tp->t_cflag & HUPCL &&
! (sc->sc_swflags & COM_SW_SOFTCAR) == 0)
! /* XXX perhaps only clear DTR */
! outb(iobase + com_mcr, 0);
! }
ttyclose(tp);
#ifdef notyet /* XXXX */
--- 367,385 ----
struct tty *tp = sc->sc_tty;
int iobase = sc->sc_iobase;
+ int s;
(*linesw[tp->t_line].l_close)(tp, flag);
! s = spltty();
! bic(iobase + com_cfcr, CFCR_SBREAK);
! outb(iobase + com_ier, 0);
! if (tp->t_cflag & HUPCL &&
! (sc->sc_swflags & COM_SW_SOFTCAR) == 0) {
! /* XXX perhaps only clear DTR */
! outb(iobase + com_mcr, 0);
! }
! tp->t_state &= ~(TS_BUSY | TS_FLUSH);
! if (--comsopen == 0)
! untimeout(compoll, NULL);
! splx(s);
ttyclose(tp);
#ifdef notyet /* XXXX */
***************
*** 447,454 ****
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:
--- 468,477 ----
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:
***************
*** 537,540 ****
--- 560,564 ----
int ospeed = comspeed(t->c_ospeed);
u_char cfcr;
+ tcflag_t oldcflag;
int s;
***************
*** 604,639 ****
}
- /*
- * 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;
--- 628,649 ----
}
/* and copy to tty */
tp->t_ispeed = t->c_ispeed;
tp->t_ospeed = t->c_ospeed;
+ oldcflag = tp->t_cflag;
tp->t_cflag = t->c_cflag;
+ /*
+ * If DCD is off and MDMBUF is changed, ask the tty layer if we should
+ * stop the device.
+ */
+ if ((sc->sc_msr & MSR_DCD) == 0 &&
+ (sc->sc_swflags & COM_SW_SOFTCAR) == 0 &&
+ (oldcflag & MDMBUF) != (tp->t_cflag & MDMBUF) &&
+ (*linesw[tp->t_line].l_modem)(tp, 0) == 0) {
+ outb(iobase + com_mcr,
+ (tp->t_cflag & CRTSCTS) ? ~MCR_DTR : ~(MCR_DTR | MCR_RTS));
+ }
+
splx(s);
return 0;
***************
*** 651,658 ****
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) {
--- 661,666 ----
***************
*** 693,733 ****
}
- static inline void
- comeint(sc, stat)
- struct com_softc *sc;
- int stat;
- {
- struct tty *tp = sc->sc_tty;
- int iobase = sc->sc_iobase;
- int c;
-
- c = inb(iobase + com_data);
- #ifdef DDB
- if ((stat & LSR_BI) && (sc->sc_dev.dv_unit == comconsole)) {
- Debugger();
- return;
- }
- #endif
- if ((tp->t_state & TS_ISOPEN) == 0) {
- #ifdef KGDB
- /* we don't care about parity errors */
- if (((stat & (LSR_BI | LSR_FE | LSR_PE)) == LSR_PE) &&
- kgdb_dev == makedev(commajor, unit) && c == FRAME_END)
- kgdb_connect(0); /* trap into kgdb */
- #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);
- }
-
void
comdiag(arg)
--- 701,704 ----
***************
*** 735,749 ****
{
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");
}
--- 706,795 ----
{
struct com_softc *sc = arg;
! int overflows, floods;
int s;
s = spltty();
+ sc->sc_errors = 0;
overflows = sc->sc_overflows;
sc->sc_overflows = 0;
+ floods = sc->sc_floods;
+ sc->sc_floods = 0;
splx(s);
! log(LOG_WARNING, "%s: %d silo overflow%s, %d ibuf overflow%s\n",
! sc->sc_dev.dv_xname,
! overflows, overflows == 1 ? "" : "s",
! floods, floods == 1 ? "" : "s");
! }
!
! void
! compoll(arg)
! void *arg;
! {
! int unit;
! struct com_softc *sc;
! struct tty *tp;
! register u_char *ibufp;
! u_char *ibufend;
! register int c;
! int s;
! static int lsrmap[8] = {
! 0, TTY_PE,
! TTY_FE, TTY_PE|TTY_FE,
! TTY_FE, TTY_PE|TTY_FE,
! TTY_FE, TTY_PE|TTY_FE
! };
!
! s = spltty();
! if (comevents == 0) {
! splx(s);
! goto out;
! }
! comevents = 0;
! splx(s);
!
! for (unit = 0; unit < comcd.cd_ndevs; unit++) {
! sc = comcd.cd_devs[unit];
! if (sc == 0 || sc->sc_ibufp == sc->sc_ibuf)
! continue;
!
! tp = sc->sc_tty;
!
! s = spltty();
!
! ibufp = sc->sc_ibuf;
! ibufend = sc->sc_ibufp;
!
! sc->sc_ibufp = sc->sc_ibuf = (ibufp == sc->sc_ibufs[0]) ?
! sc->sc_ibufs[1] : sc->sc_ibufs[0];
! sc->sc_ibufhigh = sc->sc_ibuf + COM_IHIGHWATER;
! sc->sc_ibufend = sc->sc_ibuf + COM_IBUFSIZE;
!
! if (tp == 0 || (tp->t_state & TS_ISOPEN) == 0) {
! splx(s);
! continue;
! }
!
! if ((tp->t_cflag & CRTSCTS) != 0 &&
! (sc->sc_mcr & MCR_RTS) == 0)
! outb(sc->sc_iobase + com_mcr, sc->sc_mcr |= MCR_RTS);
!
! splx(s);
!
! while (ibufp < ibufend) {
! c = *ibufp++;
! if (*ibufp & LSR_OE) {
! sc->sc_overflows++;
! if (sc->sc_errors++ == 0)
! timeout(comdiag, sc, 60 * hz);
! }
! /* This is ugly, but fast. */
! c |= lsrmap[(*ibufp++ & (LSR_BI|LSR_FE|LSR_PE)) >> 2];
! (*linesw[tp->t_line].l_rint)(c, tp);
! }
! }
!
! out:
! timeout(compoll, NULL, 1);
}
***************
*** 766,784 ****
if (lsr & LSR_RCV_MASK) {
! /* XXXX put in FIFO and process later */
do {
! if (lsr & (LSR_BI|LSR_FE|LSR_PE|LSR_OE))
! comeint(sc, lsr);
! else {
! data = inb(iobase + com_data);
! if (tp->t_state & TS_ISOPEN)
! (*linesw[tp->t_line].l_rint)(data, tp);
! #ifdef KGDB
! else {
! if (kgdb_dev == makedev(commajor, unit) &&
! data == FRAME_END)
! kgdb_connect(0);
! }
#endif
}
lsr = inb(iobase + com_lsr);
--- 812,838 ----
if (lsr & LSR_RCV_MASK) {
! register u_char *p = sc->sc_ibuf;
!
! comevents = 1;
do {
! #ifdef DDB
! if ((lsr & LSR_BI) != 0 &&
! sc->sc_dev.dv_unit == comconsole) {
! Debugger();
! continue;
! }
#endif
+ data = inb(iobase + com_data);
+ if (p >= sc->sc_ibufend) {
+ sc->sc_floods++;
+ if (sc->sc_errors++ == 0)
+ timeout(comdiag, sc, 60 * hz);
+ } else {
+ *p++ = data;
+ *p++ = lsr;
+ if (p == sc->sc_ibufhigh &&
+ (tp->t_cflag & CRTSCTS) != 0)
+ outb(iobase + com_mcr,
+ sc->sc_mcr &= ~MCR_RTS);
}
lsr = inb(iobase + com_lsr);
***************
*** 802,819 ****
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) != 0) {
/* 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;
}
}
--- 856,869 ----
delta = msr ^ sc->sc_msr;
sc->sc_msr = msr;
! if ((delta & MSR_DCD) != 0 &&
! (sc->sc_swflags & COM_SW_SOFTCAR) == 0 &&
! (*linesw[tp->t_line].l_modem)(tp, (msr & MSR_DCD) != 0) == 0) {
! outb(iobase + com_mcr,
! (tp->t_cflag & CRTSCTS) ? ~MCR_DTR : ~(MCR_DTR | MCR_RTS));
}
! if ((delta & msr & MSR_CTS) != 0 &&
! (tp->t_cflag & CRTSCTS) != 0) {
/* the line is up and we want to do rts/cts flow control */
! (*linesw[tp->t_line].l_start)(tp);
}
}