From 85a1a3cc98fc81b7d0bc8a8f5e1551b89c770425 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 18 Oct 2017 06:49:11 -0600 Subject: [PATCH] drivers/usbdev: Correct input flow control logic when watermarks are not enabled. Problem not by and change based on suggestion by Juha Niskanen. --- drivers/serial/Kconfig | 4 ++-- drivers/serial/serial.c | 20 +++++++++++++++++++- drivers/usbdev/cdcacm.c | 41 +++++++++++++++++++++-------------------- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index a6d8c2f120..f44613d391 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -518,7 +518,7 @@ if SERIAL_IFLOWCONTROL_WATERMARKS config SERIAL_IFLOWCONTROL_LOWER_WATERMARK int "RX lower Watermark (percent)" default 10 - range 0 100 + range 1 99 ---help--- Call the rxflowcontrol method then there are this amount (or or less) data buffered in the serial drivers RX buffer. This is expressed @@ -528,7 +528,7 @@ config SERIAL_IFLOWCONTROL_LOWER_WATERMARK config SERIAL_IFLOWCONTROL_UPPER_WATERMARK int "RX upper Watermark (percent)" default 90 - range 0 100 + range 1 99 ---help--- Call the rxflowcontrol method then there are this amount (or more) data buffered in the serial drivers RX buffer. This is expressed diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 98ad621ab8..ecf21006a2 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -70,6 +70,24 @@ #define uart_putc(ch) up_putc(ch) +/* Check watermark levels */ + +#if defined(CONFIG_SERIAL_IFLOWCONTROL) && \ + defined(CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS) +# if CONFIG_SERIAL_IFLOWCONTROL_LOWER_WATERMARK < 1 +# warning CONFIG_SERIAL_IFLOWCONTROL_LOWER_WATERMARK too small +# endif +# if CONFIG_SERIAL_IFLOWCONTROL_UPPER_WATERMARK > 99 +# warning CONFIG_SERIAL_IFLOWCONTROL_UPPER_WATERMARK too large +# endif +# if CONFIG_SERIAL_IFLOWCONTROL_LOWER_WATERMARK >= CONFIG_SERIAL_IFLOWCONTROL_UPPER_WATERMARK +# warning CONFIG_SERIAL_IFLOWCONTROL_LOWER_WATERMARK too large +# warning Must be less than CONFIG_SERIAL_IFLOWCONTROL_UPPER_WATERMARK +# endif +#endif + +/* Timing */ + #define HALF_SECOND_MSEC 500 #define HALF_SECOND_USEC 500000L @@ -995,7 +1013,7 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen (void)uart_rxflowcontrol(dev, nbuffered, false); } #else - /* If the RX buffer empty */ + /* Is the RX buffer empty? */ if (rxbuf->head == rxbuf->tail) { diff --git a/drivers/usbdev/cdcacm.c b/drivers/usbdev/cdcacm.c index 2fa840f5b1..652c4a7ea1 100644 --- a/drivers/usbdev/cdcacm.c +++ b/drivers/usbdev/cdcacm.c @@ -515,7 +515,10 @@ static int cdcacm_recvpacket(FAR struct cdcacm_dev_s *priv, } #ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS - /* Pre-calcuate the watermark level that we will need to test against. */ + /* Pre-calcuate the watermark level that we will need to test against. + * Note that the range of the the upper watermark is from 1 to 99 percent + * and that the actual capacity of the RX biffer is (recv->size - 1). + */ watermark = (CONFIG_SERIAL_IFLOWCONTROL_UPPER_WATERMARK * recv->size) / 100; #endif @@ -535,8 +538,8 @@ static int cdcacm_recvpacket(FAR struct cdcacm_dev_s *priv, while (nexthead != recv->tail && nbytes < reqlen) { -#ifdef CONFIG_SERIAL_IFLOWCONTROL -#ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS +#if defined(CONFIG_SERIAL_IFLOWCONTROL) && \ + defined(CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS) unsigned int nbuffered; /* How many bytes are buffered */ @@ -565,21 +568,6 @@ static int cdcacm_recvpacket(FAR struct cdcacm_dev_s *priv, break; } } -#else - /* Check if RX buffer is full and allow serial low-level driver to pause - * processing. This allows proper utilization of hardware flow control. - */ - - if (nexthead == rxbuf->tail) - { - if (cdcuart_rxflowcontrol(&priv->serdev, recv->size, true)) - { - /* Low-level driver activated RX flow control, exit loop now. */ - - break; - } - } -#endif #endif /* Copy one byte to the head of the circular RX buffer */ @@ -604,10 +592,23 @@ static int cdcacm_recvpacket(FAR struct cdcacm_dev_s *priv, recv->head = currhead; +#if defined(CONFIG_SERIAL_IFLOWCONTROL) && \ + !defined(CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS) + /* Check if RX buffer became full and allow serial low-level driver to + * pause processing. This allows proper utilization of hardware flow + * control when there are no watermarks. + */ + + if (nexthead == recv->tail) + { + (void)cdcuart_rxflowcontrol(&priv->serdev, recv->size, true); + } +#endif + /* If data was added to the incoming serial buffer, then wake up any * threads is waiting for incoming data. If we are running in an interrupt - * handler, then the serial driver will not run until the interrupt handler - * returns. + * handler, then the serial driver will not run until the interrupt + * handler returns. */ if (nbytes > 0)