xiaoxiang781216 commented on code in PR #16466:
URL: https://github.com/apache/nuttx/pull/16466#discussion_r2135965309
##########
include/nuttx/serial/serial.h:
##########
@@ -123,14 +123,25 @@
/* This structure defines one serial I/O buffer.
* The serial infrastructure will initialize the 'sem' field but all other
* fields must be initialized by the caller of uart_register().
+ *
+ * Maximum buffer size is reduced to 8 bits on architectures where 16bit
+ * load takes two instructions and is therefore not atomic. This prevents
+ * corrupted read if the value is changed in an interrupt handler while
+ * being loaded in non-interrupt code.
*/
struct uart_buffer_s
{
mutex_t lock; /* Used to control exclusive access to the buffer */
+#ifndef CONFIG_ARCH_LD_16BIT_NOT_ATOMIC
volatile int16_t head; /* Index to the head [IN] index in the buffer */
volatile int16_t tail; /* Index to the tail [OUT] index in the buffer */
int16_t size; /* The allocated size of the buffer */
+#else
+ volatile uint8_t head; /* Index to the head [IN] index in the buffer */
Review Comment:
could we extract a common typedef to avoid #ifdef/#else/#endif spread the
code base?
##########
include/nuttx/serial/serial.h:
##########
@@ -123,14 +123,25 @@
/* This structure defines one serial I/O buffer.
* The serial infrastructure will initialize the 'sem' field but all other
* fields must be initialized by the caller of uart_register().
+ *
+ * Maximum buffer size is reduced to 8 bits on architectures where 16bit
Review Comment:
move to help of ARCH_LD_16BIT_NOT_ATOMIC
##########
drivers/serial/Kconfig-usart:
##########
@@ -130,13 +132,15 @@ menu "USART1 Configuration"
config USART1_RXBUFSIZE
int "Receive buffer size"
+ default 128 if ARCH_LD_16BIT_NOT_ATOMIC
default 256
Review Comment:
could we directly change the default value to 255?
##########
drivers/serial/Kconfig-usart:
##########
@@ -705,13 +723,15 @@ menu "USART9 Configuration"
config USART9_RXBUFSIZE
int "Receive buffer size"
+ default 128 if ARCH_LD_16BIT_NOT_ATOMIC
default 256
---help---
Characters are buffered as they are received. This specifies
the size of the receive buffer.
config USART9_TXBUFSIZE
int "Transmit buffer size"
+ default 128 if ARCH_LD_16BIT_NOT_ATOMIC
Review Comment:
can we change to ARCH_HAVE_8BIT_BUFSIZE and put into serial/Kconfig?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
