From owner-p4-projects@FreeBSD.ORG Sun Jun 10 17:32:20 2012 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id A13461065679; Sun, 10 Jun 2012 17:32:20 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5A3FF1065670 for ; Sun, 10 Jun 2012 17:32:20 +0000 (UTC) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from skunkworks.freebsd.org (skunkworks.freebsd.org [IPv6:2001:4f8:fff6::2d]) by mx1.freebsd.org (Postfix) with ESMTP id 42BDC8FC1B for ; Sun, 10 Jun 2012 17:32:20 +0000 (UTC) Received: from skunkworks.freebsd.org (localhost [127.0.0.1]) by skunkworks.freebsd.org (8.14.4/8.14.4) with ESMTP id q5AHWK2w004230 for ; Sun, 10 Jun 2012 17:32:20 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by skunkworks.freebsd.org (8.14.4/8.14.4/Submit) id q5AHWJqG004227 for perforce@freebsd.org; Sun, 10 Jun 2012 17:32:19 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Sun, 10 Jun 2012 17:32:19 GMT Message-Id: <201206101732.q5AHWJqG004227@skunkworks.freebsd.org> X-Authentication-Warning: skunkworks.freebsd.org: perforce set sender to bb+lists.freebsd.perforce@cyrus.watson.org using -f From: Robert Watson To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 212585 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Jun 2012 17:32:21 -0000 http://p4web.freebsd.org/@@212585?ac=10 Change 212585 by rwatson@rwatson_svr_ctsrd_mipsbuild on 2012/06/10 17:31:47 Continue refining the Altera JTAG UART driver by replacing pure blocking writes with a test-for writability to avoid the writing thread hanging if the FIFO is full. Use interrupts, or if they are unavailable, polling, to wait for space to become available, instead. More generally, tidy up event handling and try to combing interrupt-driven and polling paths as much as possible (but no more). The low-level console still uses blocking writes, which can lead to boot-time stalls if the JTAG able is not connected. However, TTY output should now never lead to a kernel stall, just stalling of the user process trying to write to the TTY. Affected files ... .. //depot/projects/ctsrd/beribsd/src/sys/dev/altera/jtag_uart/altera_jtag_uart_tty.c#4 edit Differences ... ==== //depot/projects/ctsrd/beribsd/src/sys/dev/altera/jtag_uart/altera_jtag_uart_tty.c#4 (text+ko) ==== @@ -131,49 +131,66 @@ return (0); } +static char +aju_read(struct altera_jtag_uart_softc *sc) +{ + + AJU_LOCK_ASSERT(sc); + + while (!aju_readable(sc)); + *sc->ajus_buffer_validp = 0; + return (*sc->ajus_buffer_datap); +} + +/* + * Routines for enabling and disabling interrupts for read and write. + */ static void -aju_write(struct altera_jtag_uart_softc *sc, char ch) +aju_intr_readable_enable(struct altera_jtag_uart_softc *sc) +{ + uint32_t v; + + AJU_LOCK_ASSERT(sc); + + v = aju_control_read(sc); + v |= ALTERA_JTAG_UART_CONTROL_RE; + aju_control_write(sc, v); +} + +static void +aju_intr_writable_enable(struct altera_jtag_uart_softc *sc) { + uint32_t v; AJU_LOCK_ASSERT(sc); - while (!aju_writable(sc)); - aju_data_write(sc, ch); + v = aju_control_read(sc); + v |= ALTERA_JTAG_UART_CONTROL_WE; + aju_control_write(sc, v); } -static char -aju_read(struct altera_jtag_uart_softc *sc) +static void +aju_intr_writable_disable(struct altera_jtag_uart_softc *sc) { + uint32_t v; AJU_LOCK_ASSERT(sc); - while (!aju_readable(sc)); - *sc->ajus_buffer_validp = 0; - return (*sc->ajus_buffer_datap); + v = aju_control_read(sc); + v &= ~ALTERA_JTAG_UART_CONTROL_WE; + aju_control_write(sc, v); } static void -aju_outwakeup(struct tty *tp) +aju_intr_disable(struct altera_jtag_uart_softc *sc) { - struct altera_jtag_uart_softc *sc; - int len; - u_char ch; + uint32_t v; + + AJU_LOCK_ASSERT(sc); - /* - * XXXRW: Would be nice not to do blocking writes to the UART here, - * rescheduling on our timer tick if work remains to be done. - * - * XXXRW: Possibly, if full, defer to interrupt context. - */ - sc = tty_softc(tp); - for (;;) { - len = ttydisc_getc(tp, &ch, sizeof(ch)); - if (len == 0) - break; - AJU_LOCK(sc); - aju_write(sc, ch); - AJU_UNLOCK(sc); - } + v = aju_control_read(sc); + v &= ~(ALTERA_JTAG_UART_CONTROL_RE | ALTERA_JTAG_UART_CONTROL_WE); + aju_control_write(sc, v); } /* @@ -182,14 +199,13 @@ * up with, or without, IRQs allocated. */ static void -aju_handle_input(struct altera_jtag_uart_softc *sc) +aju_handle_input(struct altera_jtag_uart_softc *sc, struct tty *tp) { - struct tty *tp; int c; - tp = sc->ajus_ttyp; - tty_lock(tp); - AJU_LOCK(sc); + tty_lock_assert(tp, MA_OWNED); + AJU_LOCK_ASSERT(sc); + while (aju_readable(sc)) { c = aju_read(sc); AJU_UNLOCK(sc); @@ -201,53 +217,102 @@ } AJU_UNLOCK(sc); ttydisc_rint_done(tp); - tty_unlock(tp); + AJU_LOCK(sc); } +/* + * Send output to the UART until either there's none left to send, or we run + * out of room and need to await an interrupt so that we can start sending + * again. + * + * XXXRW: It would be nice to query WSPACE at the beginning and write to the + * FIFO in bugger chunks. + */ static void -aju_timeout(void *v) +aju_handle_output(struct altera_jtag_uart_softc *sc, struct tty *tp) { - struct altera_jtag_uart_softc *sc = v; + uint8_t ch; + + tty_lock_assert(tp, MA_OWNED); + AJU_LOCK_ASSERT(sc); - aju_handle_input(sc); - callout_reset(&sc->ajus_callout, aju_pollinterval, aju_timeout, sc); + AJU_UNLOCK(sc); + while (ttydisc_getc_poll(tp) != 0) { + AJU_LOCK(sc); + if (aju_writable(sc)) { + AJU_UNLOCK(sc); + if (ttydisc_getc(tp, &ch, sizeof(ch)) != sizeof(ch)) + panic("%s: ttydisc_getc", __func__); + AJU_LOCK(sc); + aju_data_write(sc, ch); + } else { + aju_intr_writable_enable(sc); + return; + } + AJU_UNLOCK(sc); + } + AJU_LOCK(sc); + aju_intr_writable_disable(sc); } static void -aju_intr(void *v) +aju_outwakeup(struct tty *tp) { - struct altera_jtag_uart_softc *sc = v; + struct altera_jtag_uart_softc *sc = tty_softc(tp); + + tty_lock_assert(tp, MA_OWNED); - /* - * XXXRW: Just receive in the interrupt path for now; possibly we - * should check the control flags. - */ - aju_handle_input(sc); + AJU_LOCK(sc); + aju_handle_output(sc, tp); + AJU_UNLOCK(sc); } static void -aju_intr_enable(struct altera_jtag_uart_softc *sc) +aju_timeout(void *arg) { - uint32_t v; + struct altera_jtag_uart_softc *sc = arg; + struct tty *tp = sc->ajus_ttyp; + + tty_lock(tp); + AJU_LOCK(sc); - AJU_LOCK_ASSERT(sc); + /* + * It would be convenient if we could share code with aju_intr() here + * by testing the control register for ALTERA_JTAG_UART_CONTROL_RI and + * ALTERA_JTAG_UART_CONTROL_WI. Unfortunately, it's not clear that + * this is supported, so do all the work to poll for both input and + * output. + */ + aju_handle_input(sc, tp); + aju_handle_output(sc, tp); - v = aju_control_read(sc); - v |= ALTERA_JTAG_UART_CONTROL_RE; - v &= ~ALTERA_JTAG_UART_CONTROL_WE; - aju_control_write(sc, v); + /* + * Reschedule next poll attempt. There's some argument that we should + * do adaptive polling based on the expectation of I/O: is something + * pending in the output buffer, or have we recently had input, but we + * don't. + */ + callout_reset(&sc->ajus_callout, aju_pollinterval, aju_timeout, sc); + AJU_UNLOCK(sc); + tty_unlock(tp); } static void -aju_intr_disable(struct altera_jtag_uart_softc *sc) +aju_intr(void *arg) { + struct altera_jtag_uart_softc *sc = arg; + struct tty *tp = sc->ajus_ttyp; uint32_t v; - AJU_LOCK_ASSERT(sc); - + tty_lock(tp); + AJU_LOCK(sc); v = aju_control_read(sc); - v &= ~(ALTERA_JTAG_UART_CONTROL_RE | ALTERA_JTAG_UART_CONTROL_WE); - aju_control_write(sc, v); + if (v & ALTERA_JTAG_UART_CONTROL_RI) + aju_handle_input(sc, tp); + if (v & ALTERA_JTAG_UART_CONTROL_WI) + aju_handle_output(sc, tp); + AJU_UNLOCK(sc); + tty_unlock(tp); } int @@ -307,7 +372,7 @@ */ if (sc->ajus_irq_res != NULL) { AJU_LOCK(sc); - aju_intr_enable(sc); + aju_intr_readable_enable(sc); AJU_UNLOCK(sc); } else { callout_init(&sc->ajus_callout, CALLOUT_MPSAFE);