PERFORCE change 212585 for review

Robert Watson rwatson at FreeBSD.org
Sun Jun 10 17:32:21 UTC 2012


http://p4web.freebsd.org/@@212585?ac=10

Change 212585 by rwatson at 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);


More information about the p4-projects mailing list