Subject: Re: Request for review
To: Andrew Doran <ad@interlude.eu.org>
From: Bill Studenmund <wrstuden@netbsd.org>
List: tech-kern
Date: 09/20/2002 10:13:31
On Fri, 20 Sep 2002, Andrew Doran wrote:

> Hi,
>
> This patch handles the kooky wiring on DEC SCCs, and will mean that the
> alpha TCWSCONS config can be got rid of. It's a rehash of a patch Ken
> Hornstein posted a few years ago.
>
> There are maybe another 20 or 30 places around the tree where cs_ctl_chan
> needs to be set by MD code; those aren't shown here.
>
> Any objections?

Yes. Why does cs_ctl_chan == NULL turn stuff off? I think the correct
semantics are that cs_ctl_chan == NULL means look in the normal place for
RTS and DTS. That way for the reg 5 stuff, you can do:

p = (cs->cs_ctl_chan) ? cs->cs_ctl_chan : cs;

and twiddle p->cs_creg[5].

By requiring cs_ctl_chan be set, you ripple the consequences of this
change out to ALL the other drivers. If they can just leave it NULL, no
change needed.

> Index: z8530sc.c
> ===================================================================
> RCS file: /cvsroot/syssrc/sys/dev/ic/z8530sc.c,v
> retrieving revision 1.16
> diff -u -r1.16 z8530sc.c
> --- z8530sc.c	2001/11/13 13:14:46	1.16
> +++ z8530sc.c	2002/09/20 04:38:13
> @@ -132,7 +132,7 @@
>  zs_loadchannelregs(cs)
>  	struct zs_chanstate *cs;
>  {
> -	u_char *reg;
> +	u_char *reg, v;
>
>  	zs_write_csr(cs, ZSM_RESET_ERR); /* XXX: reset error condition */
>
> @@ -144,9 +144,15 @@
>  	zs_iflush(cs);	/* XXX */
>  #endif
>
> -	if (memcmp((caddr_t)cs->cs_preg, (caddr_t)cs->cs_creg, 16) == 0)
> -	    return;	/* only change if values are different */
> +	if (cs->cs_ctl_chan != NULL)
> +		v = ((cs->cs_ctl_chan->cs_creg[5] & (ZSWR5_RTS | ZSWR5_DTR)) !=
> +		    (cs->cs_ctl_chan->cs_preg[5] & (ZSWR5_RTS | ZSWR5_DTR)));
> +	else
> +		v = 0;
>
> +	if (memcmp((caddr_t)cs->cs_preg, (caddr_t)cs->cs_creg, 16) == 0 && !v)
> +		return;	/* only change if values are different */
> +
>  	/* Copy "pending" regs to "current" */
>  	memcpy((caddr_t)cs->cs_creg, (caddr_t)cs->cs_preg, 16);
>  	reg = cs->cs_creg;	/* current regs */
> @@ -215,6 +221,13 @@
>  	/* char size, enable (RX/TX)*/
>  	zs_write_reg(cs, 3, reg[3]);
>  	zs_write_reg(cs, 5, reg[5]);
> +
> +	/* Write the status bits on the alternate channel also. */
> +	if (cs->cs_ctl_chan != NULL && cs->cs_ctl_chan != cs) {
> +		v = cs->cs_ctl_chan->cs_preg[5];
> +		cs->cs_ctl_chan->cs_creg[5] = v;
> +		zs_write_reg(cs->cs_ctl_chan, 5, v);
> +	}
>
>  	/* interrupt enables: RX, TX, STATUS */
>  	zs_write_reg(cs, 1, reg[1]);
> Index: z8530sc.h
> ===================================================================
> RCS file: /cvsroot/syssrc/sys/dev/ic/z8530sc.h,v
> retrieving revision 1.16
> diff -u -r1.16 z8530sc.h
> --- z8530sc.h	2002/09/06 13:23:12	1.16
> +++ z8530sc.h	2002/09/20 04:38:13
> @@ -83,6 +83,13 @@
>  	int	cs_defcflag;		/* default cflag */
>
>  	/*
> +	 * For strange systems that have oddly wired serial ports, we
> +	 * provide a pointer to the channel state of the port that has
> +	 * our status lines on it
> +	 */
> +	struct zs_chanstate *cs_ctl_chan;
> +
> +	/*
>  	 * We must keep a copy of the write registers as they are
>  	 * mostly write-only and we sometimes need to set and clear
>  	 * individual bits (e.g., in WR3).  Not all of these are
> Index: z8530tty.c
> ===================================================================
> RCS file: /cvsroot/syssrc/sys/dev/ic/z8530tty.c,v
> retrieving revision 1.80
> diff -u -r1.80 z8530tty.c
> --- z8530tty.c	2002/09/06 13:23:12	1.80
> +++ z8530tty.c	2002/09/20 04:38:15
> @@ -1235,13 +1235,13 @@
>  {
>  	struct zs_chanstate *cs = zst->zst_cs;
>
> -	if (cs->cs_wr5_dtr == 0)
> +	if (cs->cs_wr5_dtr == 0 || cs->cs_ctl_chan == NULL)
>  		return;
>
>  	if (onoff)
> -		SET(cs->cs_preg[5], cs->cs_wr5_dtr);
> +		SET(cs->cs_ctl_chan->cs_preg[5], cs->cs_wr5_dtr);
>  	else
> -		CLR(cs->cs_preg[5], cs->cs_wr5_dtr);
> +		CLR(cs->cs_ctl_chan->cs_preg[5], cs->cs_wr5_dtr);
>
>  	if (!cs->cs_heldchange) {
>  		if (zst->zst_tx_busy) {
> @@ -1262,6 +1262,9 @@
>  	struct zs_chanstate *cs = zst->zst_cs;
>  	u_char zsbits;
>
> +	if (cs->cs_ctl_chan == NULL)
> +		return;
> +
>  	zsbits = 0;
>  	if (ISSET(ttybits, TIOCM_DTR))
>  		SET(zsbits, ZSWR5_DTR);
> @@ -1270,16 +1273,16 @@
>
>  	switch (how) {
>  	case TIOCMBIC:
> -		CLR(cs->cs_preg[5], zsbits);
> +		CLR(cs->cs_ctl_chan->cs_preg[5], zsbits);
>  		break;
>
>  	case TIOCMBIS:
> -		SET(cs->cs_preg[5], zsbits);
> +		SET(cs->cs_ctl_chan->cs_preg[5], zsbits);
>  		break;
>
>  	case TIOCMSET:
> -		CLR(cs->cs_preg[5], ZSWR5_RTS | ZSWR5_DTR);
> -		SET(cs->cs_preg[5], zsbits);
> +		CLR(cs->cs_ctl_chan->cs_preg[5], ZSWR5_RTS | ZSWR5_DTR);
> +		SET(cs->cs_ctl_chan->cs_preg[5], zsbits);
>  		break;
>  	}
>
> @@ -1301,11 +1304,13 @@
>  	u_char zsbits;
>  	int ttybits = 0;
>
> -	zsbits = cs->cs_preg[5];
> -	if (ISSET(zsbits, ZSWR5_DTR))
> -		SET(ttybits, TIOCM_DTR);
> -	if (ISSET(zsbits, ZSWR5_RTS))
> -		SET(ttybits, TIOCM_RTS);
> +	if (cs->cs_ctl_chan != NULL) {
> +		zsbits = cs->cs_ctl_chan->cs_preg[5];
> +		if (ISSET(zsbits, ZSWR5_DTR))
> +			SET(ttybits, TIOCM_DTR);
> +		if (ISSET(zsbits, ZSWR5_RTS))
> +			SET(ttybits, TIOCM_RTS);
> +	}
>
>  	zsbits = cs->cs_rr0;
>  	if (ISSET(zsbits, ZSRR0_DCD))
> @@ -1365,17 +1370,17 @@
>  {
>  	struct zs_chanstate *cs = zst->zst_cs;
>
> -	if (cs->cs_wr5_rts == 0)
> +	if (cs->cs_wr5_rts == 0 || cs->cs_ctl_chan == NULL)
>  		return;
>
>  	if (ISSET(zst->zst_rx_flags, RX_ANY_BLOCK)) {
> -		CLR(cs->cs_preg[5], cs->cs_wr5_rts);
> -		CLR(cs->cs_creg[5], cs->cs_wr5_rts);
> +		CLR(cs->cs_ctl_chan->cs_preg[5], cs->cs_wr5_rts);
> +		CLR(cs->cs_ctl_chan->cs_creg[5], cs->cs_wr5_rts);
>  	} else {
> -		SET(cs->cs_preg[5], cs->cs_wr5_rts);
> -		SET(cs->cs_creg[5], cs->cs_wr5_rts);
> +		SET(cs->cs_ctl_chan->cs_preg[5], cs->cs_wr5_rts);
> +		SET(cs->cs_ctl_chan->cs_creg[5], cs->cs_wr5_rts);
>  	}
> -	zs_write_reg(cs, 5, cs->cs_creg[5]);
> +	zs_write_reg(cs->cs_ctl_chan, 5, cs->cs_cs_ctl_chan->creg[5]);
>  }
>
>
> Index: zs_ioasic.c
> ===================================================================
> RCS file: /cvsroot/syssrc/sys/dev/tc/zs_ioasic.c,v
> retrieving revision 1.11
> diff -u -r1.11 zs_ioasic.c
> --- zs_ioasic.c	2002/09/06 13:23:37	1.11
> +++ zs_ioasic.c	2002/09/20 04:38:55
> @@ -317,7 +317,6 @@
>  			zs_write_reg(cs, 9, 0);
>  		}
>
> -#ifdef notyet /* XXX thorpej */
>  		/*
>  		 * Set up the flow/modem control channel pointer to
>  		 * deal with the weird wiring on the TC Alpha and
> @@ -327,7 +326,6 @@
>  			cs->cs_ctl_chan = zs->zsc_cs[0];
>  		else
>  			cs->cs_ctl_chan = NULL;
> -#endif
>
>  		/*
>  		 * Look for a child driver for this channel.
>