diff --git a/ChangeLog b/ChangeLog index 6457739f49..ce0dbed524 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2667,7 +2667,8 @@ file was opened in binary mode. * lib/stdio/lib_fopen.c: Correct an error in parsing open mode string. The plus sign may not appear right after the basic mode. For example, "r+", "rb+", - and "r+b" are all value open strings and mean the same thing. + and "r+b" are all valid open strings and mean the same thing. * lib/stdio/lib_fopen.c: Correct return errno value from f_open() and f_fdopen() if the open mode string is invalid. - + * drivers/serial/serial.c: Do not disable Rx interrupts on each byte. + Rather, only disable Rx interrupts when the Rx ring buffer may be empty. diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 7aa342d9d9..c43060b2b9 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -311,6 +311,7 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen FAR uart_dev_t *dev = inode->i_private; irqstate_t flags; ssize_t recvd = 0; + int16_t tail; /* Only one user can be accessing dev->recv.tail at once */ @@ -323,19 +324,31 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen while (recvd < buflen) { - /* Check if there is more data to return in the circular buffer */ + /* Check if there is more data to return in the circular buffer. + * NOTE: Rx interrupt handling logic may aynchronously increment + * the head index but must not modify the tail index. The tail + * index is only modified in this function. Therefore, no + * special handshaking is required here. + */ - uart_disablerxint(dev); - if (dev->recv.head != dev->recv.tail) + tail = dev->recv.tail; + if (dev->recv.head != tail) { - *buffer++ = dev->recv.buffer[dev->recv.tail]; + /* Take the next character from the tail of the buffer */ + + *buffer++ = dev->recv.buffer[tail]; recvd++; - if (++(dev->recv.tail) >= dev->recv.size) + /* Increment the tail index. Most operations are done using the + * local variable 'tail' so that the final dev->recv.tail update + * is atomic. + */ + + if (++tail >= dev->recv.size) { - dev->recv.tail = 0; + tail = 0; } - uart_enablerxint(dev); + dev->recv.tail = tail; } #ifdef CONFIG_DEV_SERIAL_FULLBLOCKS @@ -354,8 +367,6 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen { recvd = -EAGAIN; } - - uart_enablerxint(dev); break; } #else @@ -369,7 +380,6 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen * received up to the wait condition. */ - uart_enablerxint(dev); break; } @@ -383,7 +393,6 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen /* Break out of the loop returning -EAGAIN */ recvd = -EAGAIN; - uart_enablerxint(dev); break; } #endif @@ -391,16 +400,44 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen else { - /* Wait for some characters to be sent from the buffer with the RX interrupt - * re-enabled. Interrupts are disabled briefly to assure that the following - * operations are atomic. + /* Disable Rx interrupts and test again... */ + + uart_disablerxint(dev); + + /* If the Rx ring buffer still empty? Bytes may have been addded + * between the last time that we checked and when we disabled Rx + * interrupts. */ - flags = irqsave(); - dev->recvwaiting = true; - uart_enablerxint(dev); - uart_takesem(&dev->recvsem); - irqrestore(flags); + if (dev->recv.head == dev->recv.tail) + { + /* Yes.. the buffer is still empty. Wait for some characters + * to be received into the buffer with the RX interrupt re- + * enabled. All interrupts are disabled briefly to assure + * that the following operations are atomic. + */ + + flags = irqsave(); + dev->recvwaiting = true; + uart_enablerxint(dev); + + /* Now wait with the Rx interrupt re-enabled. NuttX will + * automatically re-enable global interrupts when this + * thread goes to sleep. + */ + + uart_takesem(&dev->recvsem); + irqrestore(flags); + } + else + { + /* No... the ring buffer is no longer empty. Just re-enable Rx + * interrupts and accept the new data on the next time through + * the loop. + */ + + uart_enablerxint(dev); + } } } diff --git a/include/fcntl.h b/include/fcntl.h index 1af4c3372d..382c5be16b 100644 --- a/include/fcntl.h +++ b/include/fcntl.h @@ -71,7 +71,7 @@ #define O_RSYNC 0 /* Synchronize input on read */ #define O_ACCMODE 0 /* Required by POSIX */ #define O_NOCTTY 0 /* Required by POSIX */ -#defone O_TEXT 0 /* Open the file in text (translated) mode. */ +#define O_TEXT 0 /* Open the file in text (translated) mode. */ /* This is the highest bit number used in the open flags bitset. Bits above * this bit number may be used within NuttX for other, internal purposes.