sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

drivers/usbdev/cdcacm.c: Change ordering of some operations to avoid races; Add missing uppder watermark logic that is normally in serial_io.c but must be duplicated in cdcacm.c; update comments

Browse Source
This commit is contained in:
Gregory Nutt 2017-09-27 08:46:49 -06:00
parent 8ad1e72536
commit f58a68b5c8
1 changed files with 94 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

130
drivers/usbdev/cdcacm.c
View File

@ -472,6 +472,9 @@ static int cdcacm_recvpacket(FAR struct cdcacm_dev_s *priv,
FAR struct uart_buffer_s *recv; FAR struct uart_buffer_s *recv;
FAR struct usbdev_req_s *req; FAR struct usbdev_req_s *req;
FAR uint8_t *reqbuf; FAR uint8_t *reqbuf;
#ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS
unsigned int watermark;
#endif
uint16_t reqlen; uint16_t reqlen;
uint16_t currhead; uint16_t currhead;
uint16_t nexthead; uint16_t nexthead;
@ -511,6 +514,12 @@ static int cdcacm_recvpacket(FAR struct cdcacm_dev_s *priv,
nexthead = 0; nexthead = 0;
} }
#ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS
/* Pre-calcuate the watermark level that we will need to test against. */
watermark = (CONFIG_SERIAL_IFLOWCONTROL_UPPER_WATERMARK * recv->size) / 100;
#endif
/* Then copy data into the RX buffer until either: (1) all of the data has /* Then copy data into the RX buffer until either: (1) all of the data has
* been copied, or (2) the RX buffer is full. * been copied, or (2) the RX buffer is full.
* *
@ -526,6 +535,53 @@ static int cdcacm_recvpacket(FAR struct cdcacm_dev_s *priv,
while (nexthead != recv->tail && nbytes < reqlen) while (nexthead != recv->tail && nbytes < reqlen)
{ {
#ifdef CONFIG_SERIAL_IFLOWCONTROL
#ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS
unsigned int nbuffered;
/* How many bytes are buffered */
if (recv->head >= recv->tail)
{
nbuffered = recv->head - recv->tail;
}
else
{
nbuffered = recv->size - recv->tail + recv->head;
}
/* Is the level now above the watermark level that we need to report? */
if (nbuffered >= watermark)
{
/* Let the lower level driver know that the watermark level has been
* crossed. It will probably activate RX flow control.
*/
if (cdcuart_rxflowcontrol(&priv->serdev, nbuffered, true))
{
/* Low-level driver activated RX flow control, exit loop now. */
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 */ /* Copy one byte to the head of the circular RX buffer */
recv->buffer[currhead] = *reqbuf++; recv->buffer[currhead] = *reqbuf++;
@ -631,10 +687,11 @@ static int cdcacm_release_rxpending(FAR struct cdcacm_dev_s *priv)
wd_cancel(priv->rxfailsafe); wd_cancel(priv->rxfailsafe);
/* If RX "interrupts" are enable and if input flow control is not in effect, /* If RX "interrupts" are enabled and if input flow control is not in
* then pass the packet at the head of the pending RX packet list to the to * effect, then pass the packet at the head of the pending RX packet list
* the upper serial layer. Otherwise, let the packet continue to pend the * to the upper serial layer. Otherwise, let the packet continue to pend
* priv->rxpending list until the upper serial layer is able to buffer it. * the priv->rxpending list until the upper serial layer is able to buffer
* it.
*/ */
#ifdef CONFIG_CDCACM_IFLOWCONTROL #ifdef CONFIG_CDCACM_IFLOWCONTROL
@ -2336,8 +2393,6 @@ static int cdcuart_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
if (!iflow) if (!iflow)
{ {
irqstate_t flags = enter_critical_section();
/* Flow control has been disabled. We need to make sure /* Flow control has been disabled. We need to make sure
* that DSR is set unconditionally. * that DSR is set unconditionally.
*/ */
@ -2348,6 +2403,11 @@ static int cdcuart_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
ret = cdcacm_serialstate(priv); ret = cdcacm_serialstate(priv);
} }
/* Save the new flow control setting. */
priv->iflow = false;
priv->iactive = false;
/* During the time that flow control was disabled, incoming /* During the time that flow control was disabled, incoming
* packets were queued in priv->rxpending. We must now * packets were queued in priv->rxpending. We must now
* process all of them (unless RX interrupts are also * process all of them (unless RX interrupts are also
@ -2356,11 +2416,6 @@ static int cdcuart_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
(void)cdcacm_release_rxpending(priv); (void)cdcacm_release_rxpending(priv);
/* Save the new flow control setting. */
priv->iflow = false;
priv->iactive = false;
leave_critical_section(flags);
} }
/* Flow control has been enabled. */ /* Flow control has been enabled. */
@ -2539,21 +2594,17 @@ static void cdcuart_rxint(FAR struct uart_dev_s *dev, bool enable)
{ {
/* RX "interrupts" are enabled. Is this a transition from disabled /* RX "interrupts" are enabled. Is this a transition from disabled
* to enabled state? * to enabled state?
*
* We also need to check if input control flow is in effect. If so,
* then we should not call uart_datareceived() until both
* priv->rxenabled is true and priv->iactive are false.
*/ */
if (!priv->rxenabled) if (!priv->rxenabled)
{ {
/* RX "interrupts are no longer disabled */ /* Yes.. RX "interrupts are no longer disabled */
priv->rxenabled = true; priv->rxenabled = true;
/* During the time that RX interrupts was disabled, incoming /* During the time that RX interrupts was disabled, incoming
* packets were queued in priv->rxpending. We must now process * packets were queued in priv->rxpending. We must now process
* all of them (unless flow control becomes enabled) * all of them (unless flow control is enabled)
* *
* NOTE: This action may cause this function to be re-entered * NOTE: This action may cause this function to be re-entered
* with enable == false. * with enable == false.
@ -2563,8 +2614,8 @@ static void cdcuart_rxint(FAR struct uart_dev_s *dev, bool enable)
} }
} }
/* RX "interrupts" are disabled. Is this a transition from enabled /* RX "interrupts" are disabled. Nothing special needs to be done on a
* to disabled state? * transition from the enabled to the disabled state.
*/ */
else else
@ -2604,7 +2655,6 @@ static bool cdcuart_rxflowcontrol(FAR struct uart_dev_s *dev,
{ {
#ifdef CONFIG_CDCACM_IFLOWCONTROL #ifdef CONFIG_CDCACM_IFLOWCONTROL
FAR struct cdcacm_dev_s *priv; FAR struct cdcacm_dev_s *priv;
int ret;
/* Sanity check */ /* Sanity check */
@ -2659,19 +2709,7 @@ static bool cdcuart_rxflowcontrol(FAR struct uart_dev_s *dev,
* a change in the setting of DSR. * a change in the setting of DSR.
*/ */
else if ((priv->serialstate & CDCACM_UART_DSR) == 0) else
{
/* Set DSR (set DCD in any case). */
priv->serialstate |= (CDCACM_UART_DSR | CDCACM_UART_DCD);
/* And send the SerialState message.
* REVISIT: Error return case. Would an error mean DSR is still
* clear?
*/
ret = cdcacm_serialstate(priv);
if (ret >= 0)
{ {
/* Flow control is not active (Needed before calling /* Flow control is not active (Needed before calling
* cdcacm_release_rxpending()) * cdcacm_release_rxpending())
@ -2679,19 +2717,31 @@ static bool cdcuart_rxflowcontrol(FAR struct uart_dev_s *dev,
priv->iactive = false; priv->iactive = false;
/* During the time that flow control ws disabled, /* Set DSR if it is not alredy set */
* incoming packets were queued in priv->rxpending. We
* must now process all of them (unless RX interrupts if ((priv->serialstate & CDCACM_UART_DSR) == 0)
* becomes enabled) {
priv->serialstate |= (CDCACM_UART_DSR | CDCACM_UART_DCD);
/* And send the SerialState message.
* REVISIT: Error return case. Would an error mean DSR is
* still clear?
*/
(void)cdcacm_serialstate(priv);
}
/* During the time that flow control ws disabled, incoming packets
* were queued in priv->rxpending. We must now process all of
* them (unless RX interrupts becomes enabled)
* *
* NOTE: This action may cause this function to be re * NOTE: This action may cause this function to be re-entered with
* entered with upper == false. * upper == false.
*/ */
(void)cdcacm_release_rxpending(priv); (void)cdcacm_release_rxpending(priv);
} }
} }
}
else else
{ {
/* Flow control is disabled ... DSR must be set */ /* Flow control is disabled ... DSR must be set */