Hello Kolja,
looks good, but I have some minor stuff.
On 14/08/14 13:10, Kolja Waschk wrote:
Beside copying more than one char at once, rtems_termios_refill_transmitter()
and rtems_termios_puts() now use a common helper that (after checking ORCVXOF)
calls write() with as much chars as are available consecutively in rawOutBuf.
Integer types used in new rtems_termios_start_xmit might not be ideal but
resemble what is used in existing rtems_refill_transmitter code.
---
cpukit/libcsupport/src/termios.c | 133 ++++++++++++++++++++++++---------------
1 file changed, 83 insertions(+), 50 deletions(-)
diff --git a/cpukit/libcsupport/src/termios.c b/cpukit/libcsupport/src/termios.c
index 2448ea1..f14f138 100644
--- a/cpukit/libcsupport/src/termios.c
+++ b/cpukit/libcsupport/src/termios.c
@@ -974,6 +974,47 @@ rtems_termios_ioctl (void *arg)
}
/*
+ * Send as many chars at once as possible to device-specific code.
+ * If stopXmit==true then explicitly write(0) to stop transmitter.
+ */
+static int
+rtems_termios_start_xmit (
The internal functions use normally CamelCase in this file.
+ struct rtems_termios_tty *tty, unsigned int newTail, bool stopXmit)
+{
+ int nToSend;
Integer type is wrong.
+
+ /* if XOFF was received, do not start output resp. stop */
+ if ((tty->flow_ctrl & (FL_MDXON | FL_ORCVXOF))
+ == (FL_MDXON | FL_ORCVXOF)) {
Previously we had this and
if (!(tty->flow_ctrl & FL_ORCVXOF)) {
I would use this, since FL_ORCVXOF is only set if FL_MDXON is set.
+ /* Buffer not empty, but output stops due to XOFF */
+ /* set flag, that output has been stopped */
+ tty->flow_ctrl |= FL_OSTOP;
+ nToSend = 0;
+ tty->rawOutBufState = rob_busy;
+ if (stopXmit) {
+ (*tty->handler.write) (tty, NULL, 0);
+ }
+ } else {
+ if (newTail > tty->rawOutBuf.Head)
+ nToSend = tty->rawOutBuf.Size - newTail;
+ else
+ nToSend = tty->rawOutBuf.Head - newTail;
+ /* when flow control XON or XOF, don't send blocks of data */
+ /* to allow fast reaction on incoming flow ctrl and low latency*/
+ /* for outgoing flow control */
+ if (tty->flow_ctrl & (FL_MDXON | FL_MDXOF)) {
+ nToSend = 1;
+ }
Yes, the this is the original code, but I think it is a bit odd to discard a
value computed directly above. I would rather do the buffer nToSend
computation in an else block.
+
+ tty->rawOutBufState = rob_busy;
+ (*tty->handler.write)(
+ tty, &tty->rawOutBuf.theBuf[newTail], nToSend);
+ }
+
+ return nToSend;
+}
+
+/*
* Send characters to device-specific code
*/
void
@@ -989,21 +1030,16 @@ rtems_termios_puts (
(*tty->handler.write)(tty, buf, len);
return;
}
- newHead = tty->rawOutBuf.Head;
+
while (len) {
- /*
- * Performance improvement could be made here.
- * Copy multiple bytes to raw buffer:
- * if (len > 1) && (space to buffer end, or tail > 1)
- * ncopy = MIN (len, space to buffer end or tail)
- * memcpy (raw buffer, buf, ncopy)
- * buf += ncopy
- * len -= ncopy
- *
- * To minimize latency, the memcpy should be done
- * with interrupts enabled.
- */
- newHead = (newHead + 1) % tty->rawOutBuf.Size;
+ size_t nToCopy;
+ size_t nAvail;
+
+ /* Check space for at least one char */
+ newHead = tty->rawOutBuf.Head + 1;
+ if (newHead >= tty->rawOutBuf.Size)
+ newHead -= tty->rawOutBuf.Size;
+
rtems_termios_interrupt_lock_acquire (tty, &lock_context);
while (newHead == tty->rawOutBuf.Tail) {
tty->rawOutBufState = rob_wait;
@@ -1014,21 +1050,41 @@ rtems_termios_puts (
rtems_fatal_error_occurred (sc);
rtems_termios_interrupt_lock_acquire (tty, &lock_context);
}
- tty->rawOutBuf.theBuf[tty->rawOutBuf.Head] = *buf++;
+
+ /* Determine free space up to current tail or end of ring buffer */
+ nToCopy = len;
+ if (tty->rawOutBuf.Tail > tty->rawOutBuf.Head) {
+ /* Available space is contiguous from Head to Tail */
+ nAvail = tty->rawOutBuf.Tail - tty->rawOutBuf.Head - 1;
+ } else {
+ /* Available space wraps at buffer end. To keep code simple, utilize
+ only the part from Head to end during this iteration */
+ nAvail = tty->rawOutBuf.Size - tty->rawOutBuf.Head;
+ /* Head may not touch Tail after wraparound */
+ if (tty->rawOutBuf.Tail == 0)
+ nAvail--;
+ }
+ if (nToCopy > nAvail)
+ nToCopy = nAvail;
+
+ /* To minimize latency, the memcpy could be done
+ * with interrupts enabled or with limit on nToCopy (TBD)
+ */
+ memcpy(&tty->rawOutBuf.theBuf[tty->rawOutBuf.Head], buf, nToCopy);
+
+ newHead = tty->rawOutBuf.Head + nToCopy;
+ if (newHead >= tty->rawOutBuf.Size)
+ newHead -= tty->rawOutBuf.Size;
tty->rawOutBuf.Head = newHead;
+
if (tty->rawOutBufState == rob_idle) {
- /* check, whether XOFF has been received */
- if (!(tty->flow_ctrl & FL_ORCVXOF)) {
- (*tty->handler.write)(
- tty, &tty->rawOutBuf.theBuf[tty->rawOutBuf.Tail],1);
- } else {
- /* remember that output has been stopped due to flow ctrl*/
- tty->flow_ctrl |= FL_OSTOP;
- }
- tty->rawOutBufState = rob_busy;
+ (void)rtems_termios_start_xmit (tty, tty->rawOutBuf.Tail, false);
Please don't use this (void).
}
+
rtems_termios_interrupt_lock_release (tty, &lock_context);
- len--;
+
+ buf += nToCopy;
+ len -= nToCopy;
}
}
@@ -1678,35 +1734,12 @@ rtems_termios_refill_transmitter (struct
rtems_termios_tty *tty)
if ( tty->tty_snd.sw_pfn != NULL) {
(*tty->tty_snd.sw_pfn)(&tty->termios, tty->tty_snd.sw_arg);
}
- }
- /* check, whether output should stop due to received XOFF */
- else if ((tty->flow_ctrl & (FL_MDXON | FL_ORCVXOF))
- == (FL_MDXON | FL_ORCVXOF)) {
- /* Buffer not empty, but output stops due to XOFF */
- /* set flag, that output has been stopped */
- tty->flow_ctrl |= FL_OSTOP;
- tty->rawOutBufState = rob_busy; /*apm*/
- (*tty->handler.write) (tty, NULL, 0);
- nToSend = 0;
} else {
/*
- * Buffer not empty, start tranmitter
+ * Buffer not empty, check flow control, start transmitter
*/
- if (newTail > tty->rawOutBuf.Head)
- nToSend = tty->rawOutBuf.Size - newTail;
- else
- nToSend = tty->rawOutBuf.Head - newTail;
- /* when flow control XON or XOF, don't send blocks of data */
- /* to allow fast reaction on incoming flow ctrl and low latency*/
- /* for outgoing flow control */
- if (tty->flow_ctrl & (FL_MDXON | FL_MDXOF)) {
- nToSend = 1;
- }
- tty->rawOutBufState = rob_busy; /*apm*/
- (*tty->handler.write)(
- tty, &tty->rawOutBuf.theBuf[newTail], nToSend);
+ nToSend = rtems_termios_start_xmit (tty, newTail, true);
}
- tty->rawOutBuf.Tail = newTail; /*apm*/
}
rtems_termios_interrupt_lock_release (tty, &lock_context);
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
