From nobody@FreeBSD.org Thu Jul 15 15:08:10 2010 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 78B7D106564A for ; Thu, 15 Jul 2010 15:08:10 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 6887E8FC14 for ; Thu, 15 Jul 2010 15:08:10 +0000 (UTC) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id o6FF8Auh037965 for ; Thu, 15 Jul 2010 15:08:10 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id o6FF88pk037964; Thu, 15 Jul 2010 15:08:08 GMT (envelope-from nobody) Message-Id: <201007151508.o6FF88pk037964@www.freebsd.org> Date: Thu, 15 Jul 2010 15:08:08 GMT From: Gene Stark To: freebsd-gnats-submit@FreeBSD.org Subject: Vgetty play voice file fails with 8.0 RELEASE/uart works with 6.3/sio X-Send-Pr-Version: www-3.1 X-GNATS-Notify: >Number: 148644 >Category: kern >Synopsis: [uart] [patch] vgetty play voice file fails with 8.0 RELEASE/uart works with 6.3/sio >Confidential: no >Severity: serious >Priority: high >Responsible: marcel >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Jul 15 15:10:04 UTC 2010 >Closed-Date: Fri Jan 28 20:18:40 UTC 2011 >Last-Modified: Fri Jan 28 20:18:40 UTC 2011 >Originator: Gene Stark >Release: 8.0 >Organization: Stony Brook University >Environment: >Description: I attempted to upgrade from FreeBSD 6.3 to FreeBSD 8.0 release. In the process of doing so, I found that vgetty 0.9.32 from ports/comms/mgetty+sendfax (mgetty 1.1.35) no longer worked properly. Specifically, when it answers the phone in voice mode and attempts to play the greeting file, it plays correctly for a second or so, then lapses into garbled sound. The modem (US Robotics Sportster 33.6 Voice Faxmodem) becomes unresponsive and it is not possible to record an incoming message. I of course had to change cuad1 (sio driver) to cuau1 (uart driver) in the process of doing the upgrade. The failure mode to me appears like a data overrun on the channel from the computer to the modem, perhaps caused by a failure of hardware flow control in the uart driver. I did some amount of debugging to try to identify the problem. I didn't see any way to get debugging output from the uart driver, so I used ktrace on vgetty. It reaches the point where it is playing the voice data, outputs the data to the modem via several write calls that send 800 bytes at a time, and reports the total amount of voice data transmitted. At the point where it expects a reply from the modem, it does not get one. Presumably this is because the modem has become confused by the data overrun. There is no other indication from the modem that something has gone wrong other than the garbled audio and the fact that it becomes unresponsive. This was the show-stopper for me. I am reverting to FreeBSD 6.3 for now. I attempted to compile an 8.0 kernel with the sio driver in and the uart driver, but it seems that this is not supported any more. So I can't offer information about whether simply changing back to the old driver solves the problem. >How-To-Repeat: >Fix: >Release-Note: >Audit-Trail: Responsible-Changed-From-To: freebsd-bugs->ed Responsible-Changed-By: remko Responsible-Changed-When: Thu Jul 15 17:23:00 UTC 2010 Responsible-Changed-Why: Ed, this might be related to your MPSafeTTY rewrite. http://www.freebsd.org/cgi/query-pr.cgi?pr=148644 From: John Wehle To: bug-followup@FreeBSD.org Cc: freebsd@starkeffect.com Subject: Re: misc/148644: Vgetty play voice file fails with 8.0 RELEASE/uart works with 6.3/sio Date: Fri, 16 Jul 2010 03:11:16 -0400 (EDT) I encountered a similar problem using vgetty after upgrading from FreeBSD i386 7.3 to 8.1. Upon code inspection it appears that the uart driver (which replaces the sio driver used in FreeBSD 7.3) fails to check that CTS is asserted prior to transmitting characters. The enclosed lightly tested patch seems to resolve the problem. -- John ------------------------8<------------------------------8<--------------- --- dev/uart/uart_tty.c.ORIGINAL 2010-06-13 22:09:06.000000000 -0400 +++ dev/uart/uart_tty.c 2010-07-16 02:08:36.000000000 -0400 @@ -169,6 +169,11 @@ uart_tty_outwakeup(struct tty *tp) if (sc->sc_txbusy) return; + if ((tp->t_termios.c_cflag & CCTS_OFLOW) + && ! sc->sc_hwoflow + && ! (sc->sc_hwsig & SER_CTS)) + return; + sc->sc_txdatasz = ttydisc_getc(tp, sc->sc_txbuf, sc->sc_txfifosz); if (sc->sc_txdatasz != 0) UART_TRANSMIT(sc); @@ -316,11 +321,8 @@ uart_tty_intr(void *arg) sig = pend & SER_INT_SIGMASK; if (sig & SER_DDCD) ttydisc_modem(tp, sig & SER_DCD); - if ((sig & SER_DCTS) && (tp->t_termios.c_cflag & CCTS_OFLOW) && - !sc->sc_hwoflow) { - if (sig & SER_CTS) - uart_tty_outwakeup(tp); - } + if (sig & SER_DCTS) + uart_tty_outwakeup(tp); } if (pend & SER_INT_TXIDLE) ------------------------------------------------------------------------- | Feith Systems | Voice: 1-215-646-8000 | Email: john@feith.com | | John Wehle | Fax: 1-215-540-5495 | | ------------------------------------------------------------------------- From: Gene Stark To: bug-followup@FreeBSD.org Cc: John Wehle Subject: Re: misc/148644: Vgetty play voice file fails with 8.0 RELEASE/uart works with 6.3/sio Date: Fri, 16 Jul 2010 12:51:53 -0400 John Wehle writes: > I encountered a similar problem using vgetty after upgrading from > FreeBSD i386 7.3 to 8.1. > > Upon code inspection it appears that the uart driver (which replaces > the sio driver used in FreeBSD 7.3) fails to check that CTS is asserted > prior to transmitting characters. > > The enclosed lightly tested patch seems to resolve the problem. Yes, that works for me, thanks! Surprising that this bug is there, considering how long the uart driver seems to have been around. I played around with the driver for awhile, verified that the output overrun was indeed the problem, and that this patch fixes it. Perhaps it is obvious to somebody who already knows this code, but it might not hurt add a comment as in the version of the patch. It took me awhile to figure out that "sc_hwoflow" means "Hardware is deemed capable of doing *automatic* CTS flow control", and that CCTS_OFLOW means "application wants CTS output flow control". The situation that is a problem is when the application wants output flow control, but the hardware is not capable of doing it automatically. In fact, this is currently *always* the case, because the indication of the hardware capability is commented out in uart_dev_ns8250.c. On the other hand, presumably if the hardware *is* capable of doing automatic output flow control and the application wants output flow control, then one *doesn't* want to pay attention to CTS in the interrupt service routine, because this might prevent the FIFO from being filled when it could be, resulting in possible loss of performance. - Gene Stark Index: uart_tty.c =================================================================== RCS file: /huge/cvs/src/sys/dev/uart/uart_tty.c,v retrieving revision 1.33.2.2.2.1 diff -u -r1.33.2.2.2.1 uart_tty.c --- uart_tty.c 25 Oct 2009 01:10:29 -0000 1.33.2.2.2.1 +++ uart_tty.c 16 Jul 2010 16:31:45 -0000 @@ -169,6 +169,16 @@ if (sc->sc_txbusy) return; + /* + * If CTS output flow control is desired, + * and the hardware does not support automatic flow control, + * then don't emit output if CTS is clear. + */ + if ((tp->t_termios.c_cflag & CCTS_OFLOW) + && ! sc->sc_hwoflow + && ! (sc->sc_hwsig & SER_CTS)) + return; + sc->sc_txdatasz = ttydisc_getc(tp, sc->sc_txbuf, sc->sc_txfifosz); if (sc->sc_txdatasz != 0) UART_TRANSMIT(sc); @@ -316,11 +326,8 @@ sig = pend & SER_INT_SIGMASK; if (sig & SER_DDCD) ttydisc_modem(tp, sig & SER_DCD); - if ((sig & SER_DCTS) && (tp->t_termios.c_cflag & CCTS_OFLOW) && - !sc->sc_hwoflow) { - if (sig & SER_CTS) - uart_tty_outwakeup(tp); - } + if (sig & SER_DCTS) + uart_tty_outwakeup(tp); } if (pend & SER_INT_TXIDLE) Responsible-Changed-From-To: ed->marcel Responsible-Changed-By: ed Responsible-Changed-When: Fri Oct 8 09:23:00 UTC 2010 Responsible-Changed-Why: Marcel, could you take a look at this? http://www.freebsd.org/cgi/query-pr.cgi?pr=148644 State-Changed-From-To: open->patched State-Changed-By: marcel State-Changed-When: Mon Jan 24 18:34:40 UTC 2011 State-Changed-Why: Patch committed to -current. MFC planned in 3 days. Thanks for the patch and sotty for the delay. http://www.freebsd.org/cgi/query-pr.cgi?pr=148644 From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/148644: commit references a PR Date: Mon, 24 Jan 2011 18:34:24 +0000 (UTC) Author: marcel Date: Mon Jan 24 18:34:16 2011 New Revision: 217800 URL: http://svn.freebsd.org/changeset/base/217800 Log: In uart_tty_outwakeup(), check CTS/RTS flow control settings and prevent sending data when CTS is de-asserted. In uart_tty_intr(), call uart_tty_outwakeup() when the CTS signal changed, knowing that uart_tty_outwakeup() will do the right thing for flow control. This avoids redundant conditionals. PR: kern/148644 Submitted by: John Wehle MFC after: 3 days Modified: head/sys/dev/uart/uart_tty.c Modified: head/sys/dev/uart/uart_tty.c ============================================================================== --- head/sys/dev/uart/uart_tty.c Mon Jan 24 18:24:28 2011 (r217799) +++ head/sys/dev/uart/uart_tty.c Mon Jan 24 18:34:16 2011 (r217800) @@ -168,6 +168,14 @@ uart_tty_outwakeup(struct tty *tp) if (sc->sc_txbusy) return; + /* + * Respect RTS/CTS (output) flow control if enabled and not already + * handled by hardware. + */ + if ((tp->t_termios.c_cflag & CCTS_OFLOW) && !sc->sc_hwoflow && + !(sc->sc_hwsig & SER_CTS)) + return; + sc->sc_txdatasz = ttydisc_getc(tp, sc->sc_txbuf, sc->sc_txfifosz); if (sc->sc_txdatasz != 0) UART_TRANSMIT(sc); @@ -315,11 +323,8 @@ uart_tty_intr(void *arg) sig = pend & SER_INT_SIGMASK; if (sig & SER_DDCD) ttydisc_modem(tp, sig & SER_DCD); - if ((sig & SER_DCTS) && (tp->t_termios.c_cflag & CCTS_OFLOW) && - !sc->sc_hwoflow) { - if (sig & SER_CTS) - uart_tty_outwakeup(tp); - } + if (sig & SER_DCTS) + uart_tty_outwakeup(tp); } if (pend & SER_INT_TXIDLE) _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/148644: commit references a PR Date: Fri, 28 Jan 2011 00:22:10 +0000 (UTC) Author: marcel Date: Fri Jan 28 00:22:03 2011 New Revision: 218000 URL: http://svn.freebsd.org/changeset/base/218000 Log: In uart_tty_outwakeup(), check CTS/RTS flow control settings and prevent sending data when CTS is de-asserted. In uart_tty_intr(), call uart_tty_outwakeup() when the CTS signal changed, knowing that uart_tty_outwakeup() will do the right thing for flow control. This avoids redundant conditionals. PR: kern/148644 Submitted by: John Wehle Modified: stable/8/sys/dev/uart/uart_tty.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) Modified: stable/8/sys/dev/uart/uart_tty.c ============================================================================== --- stable/8/sys/dev/uart/uart_tty.c Thu Jan 27 23:28:00 2011 (r217999) +++ stable/8/sys/dev/uart/uart_tty.c Fri Jan 28 00:22:03 2011 (r218000) @@ -169,6 +169,14 @@ uart_tty_outwakeup(struct tty *tp) if (sc->sc_txbusy) return; + /* + * Respect RTS/CTS (output) flow control if enabled and not already + * handled by hardware. + */ + if ((tp->t_termios.c_cflag & CCTS_OFLOW) && !sc->sc_hwoflow && + !(sc->sc_hwsig & SER_CTS)) + return; + sc->sc_txdatasz = ttydisc_getc(tp, sc->sc_txbuf, sc->sc_txfifosz); if (sc->sc_txdatasz != 0) UART_TRANSMIT(sc); @@ -316,11 +324,8 @@ uart_tty_intr(void *arg) sig = pend & SER_INT_SIGMASK; if (sig & SER_DDCD) ttydisc_modem(tp, sig & SER_DCD); - if ((sig & SER_DCTS) && (tp->t_termios.c_cflag & CCTS_OFLOW) && - !sc->sc_hwoflow) { - if (sig & SER_CTS) - uart_tty_outwakeup(tp); - } + if (sig & SER_DCTS) + uart_tty_outwakeup(tp); } if (pend & SER_INT_TXIDLE) _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" State-Changed-From-To: patched->closed State-Changed-By: marcel State-Changed-When: Fri Jan 28 20:17:27 UTC 2011 State-Changed-Why: It's too late for getting this fixed in 8.2-RELEASE. 8-STABLE has been fixed though... http://www.freebsd.org/cgi/query-pr.cgi?pr=148644 >Unformatted: