Subject: Re: sys/dev/ic/z8530tty.c 1.52 -> 1.53 is broken.
To: Charles M. Hannum <root@ihack.net>
From: Dr. Bill Studenmund <wrstuden@loki.stanford.edu>
List: tech-kern
Date: 01/25/1999 11:02:57
On Mon, 25 Jan 1999, Charles M. Hannum wrote:

> 
> Uh, so I just looked at your `fix', and it only does one part
> (actually, the least important part!) of what I said was needed.
> You're still not updating rr0 (etc.), so it can't *possibly* work
> right.
> 
> *sigh*

Charles,

My "fix" reverts the zsparam changes to how they were in 1.52, which
everyone says works. So empirically it can possibly work right. :-)

I agree that we should be setting rr0 here, and am happy to discuss how to
do it. But I think the current change should be fine in the mean time as
it leaves things how they've been. As I feel a bit flamed, I'd like to
point out that things have been as they are since version 1.35 of the
file, which you checked in on 03-Nov-97. If there's crow to be eaten over
this, I think there's enough for both of us to have nice big slices. :-)

Right now I am unable to test patches (my 8500 doesn't boot NetBSD, and my
IIsi doesn't boot :-( ).

So here's a proposed fix:

Index: z8530tty.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/z8530tty.c,v
retrieving revision 1.55
diff -u -r1.55 z8530tty.c
--- z8530tty.c  1999/01/25 17:53:13     1.55
+++ z8530tty.c  1999/01/25 18:51:21
@@ -928,6 +928,12 @@
                zst->zst_r_lowat = zstty_rbuf_lowat;
        }
 
+       /*
+        * Fetch current state of rr0 - it might have changed if
interrupts
+        * were off.
+        */
+       cs->cs_rr0 = zs_read_csr(cs);
+
        splx(s);
 
        /*
@@ -935,8 +941,7 @@
         * CLOCAL or MDMBUF.  We don't hang up here; we only do that by
         * explicit request.
         */
-       (void) (*linesw[tp->t_line].l_modem)(tp,
-                       ISSET(cs->cs_rr0, ZSRR0_DCD));
+       (void) (*linesw[tp->t_line].l_modem)(tp, ISSET(cs->cs_rr0,
ZSRR0_DCD));
 
        if (!ISSET(cflag, CHWFLOW)) {
                if (zst->zst_tx_stopped) {


In addition to compacting the *linesw call, this patch explicitly
initializes rr0 just before we lower interrupts.

Can someone test it? As stated above, I can't at the moment, and I'd like
to before we commit it (though it mightn't seem like it at the moment, I
do try to test things before committing).

Take care,

Bill