[PATCH] rtems_termios_puts: Copy and write more than one char at once
Hi, I'd like to send some work upstream that I've made for a Blackfin BF537 based device. The following patch was necessary for a DMA-based high speed UART driver(*) for Blackfin - it allows to start transmission on serial interfaces with more than just one char at a time. Notes: This already reworked patch obsoletes the one attached to https://www.rtems.org/bugzilla/show_bug.cgi?id=2185 I'm unsure if the amount of bytes to sent, given as an argument to the final call of (*tty->handler.write)(), is properly chosen. It was always fixed to one before. (*)The UART driver itself is not yet ready for upstream, because it lives still in my BSP only and has some specific mechanisms which are not really suited for general use (such as using an extra GPIO for end-of-transfer signalling). Kolja Kolja Waschk (1): rtems_termios_puts: Copy and write more than one char at once cpukit/libcsupport/src/termios.c | 74 +++- 1 file changed, 51 insertions(+), 23 deletions(-) -- 1.9.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH] rtems_termios_puts: Copy and write more than one char at once
---
cpukit/libcsupport/src/termios.c | 74 +++-
1 file changed, 51 insertions(+), 23 deletions(-)
diff --git a/cpukit/libcsupport/src/termios.c b/cpukit/libcsupport/src/termios.c
index 2448ea1..d32ab74 100644
--- a/cpukit/libcsupport/src/termios.c
+++ b/cpukit/libcsupport/src/termios.c
@@ -879,7 +879,7 @@ rtems_termios_ioctl (void *arg)
tty->rawInBufSemaphoreTimeout = RTEMS_NO_TIMEOUT;
tty->rawInBufSemaphoreFirstTimeout = RTEMS_NO_TIMEOUT;
} else {
- tty->vtimeTicks = tty->termios.c_cc[VTIME] *
+ tty->vtimeTicks = tty->termios.c_cc[VTIME] *
rtems_clock_get_ticks_per_second() / 10;
if (tty->termios.c_cc[VTIME]) {
tty->rawInBufSemaphoreOptions = RTEMS_WAIT;
@@ -984,26 +984,19 @@ rtems_termios_puts (
unsigned int newHead;
rtems_interrupt_lock_context lock_context;
rtems_status_code sc;
+ size_t partlen;
if (tty->handler.mode == TERMIOS_POLLED) {
(*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;
+/* 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 +1007,56 @@ rtems_termios_puts (
rtems_fatal_error_occurred (sc);
rtems_termios_interrupt_lock_acquire (tty, &lock_context);
}
-tty->rawOutBuf.theBuf[tty->rawOutBuf.Head] = *buf++;
+
+/* Copy as much chars as fit until current tail or end of ring buffer */
+partlen = len;
+if (tty->rawOutBuf.Tail > tty->rawOutBuf.Head) {
+ /* Available space is contiguous from Head to Tail */
+ size_t available = tty->rawOutBuf.Tail - tty->rawOutBuf.Head - 1;
+ if (partlen > available)
+partlen = available;
+} else {
+ /* Available space wraps at buffer end. To keep code simple, utilize
+ only the part from Head to end during this iteration */
+ size_t available = tty->rawOutBuf.Size - tty->rawOutBuf.Head;
+ if (partlen > available)
+partlen = available;
+}
+
+/* To minimize latency, the memcpy should be done
+ * with interrupts enabled (TBD) */
+memcpy(&tty->rawOutBuf.theBuf[tty->rawOutBuf.Head], buf, partlen);
+newHead = tty->rawOutBuf.Head + partlen;
+if (newHead >= tty->rawOutBuf.Size)
+ newHead -= tty->rawOutBuf.Size;
tty->rawOutBuf.Head = newHead;
+buf += partlen;
+
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);
+ if (tty->flow_ctrl & FL_MDXOF) {
+/* Write only one byte at a time if using XON/XOFF flow ctrl */
+/* 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;
+}
} else {
-/* remember that output has been stopped due to flow ctrl*/
-tty->flow_ctrl |= FL_OSTOP;
+size_t nwaiting;
+if (tty->rawOutBuf.Head > tty->rawOutBuf.Tail)
+ nwaiting = tty->rawOutBuf.Head - tty->rawOutBuf.Tail;
+else
+ nwaiting = tty->rawOutBuf.Size - tty->rawOutBuf.Tail;
+
+(*tty->handler.write)(
+ tty, &tty->rawOutBuf.theBuf[tty->rawOutBuf.Tail], nwaiting);
}
tty->rawOutBufState = rob_busy;
}
rtems_termios_interrupt_lock_release (tty, &lock_context);
-len--;
+len -= partlen;
}
}
--
1.9.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] rtems_termios_puts: Copy and write more than one char at once
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 (
+ struct rtems_termios_tty *tty, unsigned int newTail, bool stopXmit)
+{
+ int nToSend;
+
+ /* if XOFF was received, do not start output resp. stop */
+ 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;
+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;
+}
+
+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);
}
+
rtems_termios_interrupt_lock_release (tty, &lock_context);
-len--;
+
+buf += nToCopy;
+len -= nToCopy;
}
}
@@ -1678,35 +1734,12 @@ rtems_te
Re: [PATCH] rtems_termios_puts: Copy and write more than one char at once
Hi Sebastian, + int nToSend; Integer type is wrong. Yes, just what the original refill_transmitter uses and expects: nToSend is passed as return value to rtems_refill_transmitter, which also returns it to its caller as an int... + (void)rtems_termios_start_xmit (tty, tty->rawOutBuf.Tail, false); Please don't use this (void). Only one of the two callers (rtems_refill_transmitter) is interested in that information. Is it preferred to omit the "(void)" in the other or rather do not pass the info as a return value, but e.g. into a variable pointed to by a further argument? http://stackoverflow.com/questions/11888594/ignoring-return-values-in-c Thanks, Kolja -- mr.k.waschk - ixo.de - hamburg, germany ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Console driver that needs extra TIMER
Hi, I'm developing a DMA-based UART driver for Blackfin. It requires an extra timer to trigger data processing after a transmission ended. This increases the amount of timers required by every application that uses it for the console. E.g. most of the tests won't even start without modifications. So effectively I have to fall back to a polling driver for console if I want to run the tests unmodified, or is there any mechanism to generally increment the amount of timers for all tests? Thanks, Kolja ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] rtems_termios_puts: Copy and write more than one char at once
Renamed startXmit(), nToSend is unsigned, just check FL_ORCVXOF, no (void) cast
anymore, compute nToSend in single if/else if/else.
---
cpukit/libcsupport/src/termios.c | 135 ---
1 file changed, 85 insertions(+), 50 deletions(-)
diff --git a/cpukit/libcsupport/src/termios.c b/cpukit/libcsupport/src/termios.c
index 2448ea1..216834c 100644
--- a/cpukit/libcsupport/src/termios.c
+++ b/cpukit/libcsupport/src/termios.c
@@ -974,6 +974,49 @@ rtems_termios_ioctl (void *arg)
}
/*
+ * Send as many chars at once as possible to device-specific code.
+ * If transmitting==true then assume transmission is already running and
+ * an explicit write(0) is needed if output has to stop for flow control.
+ */
+static unsigned int
+startXmit (
+ struct rtems_termios_tty *tty,
+ unsigned int newTail,
+ bool transmitting
+)
+{
+ unsigned int nToSend;
+
+ tty->rawOutBufState = rob_busy;
+
+ /* if XOFF was received, do not (re)start output */
+ if (tty->flow_ctrl & FL_ORCVXOF) {
+/* set flag, that output has been stopped */
+tty->flow_ctrl |= FL_OSTOP;
+nToSend = 0;
+/* stop transmitter */
+if (transmitting) {
+ (*tty->handler.write) (tty, NULL, 0);
+}
+ } else {
+/* 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;
+else if (newTail > tty->rawOutBuf.Head)
+ nToSend = tty->rawOutBuf.Size - newTail;
+else
+ nToSend = tty->rawOutBuf.Head - newTail;
+
+(*tty->handler.write)(
+tty, &tty->rawOutBuf.theBuf[newTail], nToSend);
+ }
+
+ return nToSend;
+}
+
+/*
* Send characters to device-specific code
*/
void
@@ -989,21 +1032,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 +1052,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 it simple, utilize
+ only the free space 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;
+ startXmit (tty, tty->rawOutBuf.Tail, false);
}
+
rtems_termios_interrupt_lock_release (tty, &lock_context);
-len--;
+
+buf += nToCopy;
+len -= nToCopy;
}
}
@@ -1678,35 +1736,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_
Re: Console driver that needs extra TIMER
[...] DMA-based UART driver for Blackfin. It requires an extra timer to trigger data processing after a transmission ended. [...] Don't you get an end of DMA TX interrupt? No, it's about RX - the driver doesn't know beforehand how many bytes are expected and thus can configure DMA only to continously receive into a ring buffer. There are buffer-complete interrupts after every so many bytes, but not related to the actual received data content. As an alternative to the timer, to avoid the latency it introduces, the driver can generate/interpret RTS/CTS signals as frame sync signals with interrupts, but these aren't always connected and usually not supported by the communication partner. Kolja ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: Console driver that needs extra TIMER
If you are working with rtems.git, you could probably just add another
BSP setting in confdefs.h for this and push the patch out for
Thanks for that hint. There already seems to be such mechanism in place for
SHARED_MEMORY_DRIVER:
confdefs.h
...
rtems_api_configuration_table Configuration_RTEMS_API = {
CONFIGURE_TASKS,
CONFIGURE_NOTEPADS_ENABLED,
CONFIGURE_MAXIMUM_TIMERS + CONFIGURE_TIMER_FOR_SHARED_MEMORY_DRIVER,
CONFIGURE_SEMAPHORES,
...
At least confdefs allows to add something to CONFIGURE_MAXIMUM_TIMERS so I can
patch it for running the tests, and soon make it properly dependent on some
BSP setting.
Thanks,
Kolja
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
