Clean up some logic in the CDC/ACM host drivers. Test with some other host controller drivers. There are lots of problems.

This commit is contained in:
Gregory Nutt
2015-05-14 10:22:08 -06:00
parent 52720e9368
commit 2b3241d81f
2 changed files with 81 additions and 54 deletions
+10
View File
@@ -1126,6 +1126,16 @@ o USB (drivers/usbdev, drivers/usbhost)
the RX reception logic probably should be moved to its own the RX reception logic probably should be moved to its own
dedicated thread. dedicated thread.
- I get crashes when I run with the STM32 OTGHS host driver.
Apparently the host driver is trashing memory on receipt
of data.
- The SAMA5D EHCI and the LPC31 EHCI drivers both take semaphores
in the cancel method. The current CDC/ACM class driver calls
the cancel() method from an interrupt handler. This will
cause a crash. Those EHCI drivers should be redesigned to
permit cancellation from the interrupt level.
Most of these problems are unique to the Olimex LPC1766STK Most of these problems are unique to the Olimex LPC1766STK
DCD; some are probably design problems in the CDC/ACM host DCD; some are probably design problems in the CDC/ACM host
driver. The bottom line is that the host CDC/ACM driver is driver. The bottom line is that the host CDC/ACM driver is
+71 -54
View File
@@ -888,9 +888,9 @@ static void usbhost_txdata_work(FAR void *arg)
FAR struct uart_dev_s *uartdev; FAR struct uart_dev_s *uartdev;
FAR struct uart_buffer_s *txbuf; FAR struct uart_buffer_s *txbuf;
ssize_t nwritten; ssize_t nwritten;
int nxfrd;
int txndx; int txndx;
int txtail; int txtail;
int ret;
priv = (FAR struct usbhost_cdcacm_s *)arg; priv = (FAR struct usbhost_cdcacm_s *)arg;
DEBUGASSERT(priv); DEBUGASSERT(priv);
@@ -913,13 +913,12 @@ static void usbhost_txdata_work(FAR void *arg)
/* Loop until The UART TX buffer is empty (or we become disconnected) */ /* Loop until The UART TX buffer is empty (or we become disconnected) */
txtail = txbuf->tail; txtail = txbuf->tail;
nxfrd = 0;
txndx = 0; txndx = 0;
while (txtail != txbuf->head && !priv->disconnected) while (txtail != txbuf->head && priv->txena && !priv->disconnected)
{ {
/* Copy data from the UART TX buffer until either 1) the UART TX /* Copy data from the UART TX buffer until either 1) the UART TX
* buffer has been emptie, or 2) the Bulk OUT buffer is full. * buffer has been emptied, or 2) the Bulk OUT buffer is full.
*/ */
txndx = 0; txndx = 0;
@@ -931,7 +930,6 @@ static void usbhost_txdata_work(FAR void *arg)
/* Increment counters and indices */ /* Increment counters and indices */
nxfrd++;
txndx++; txndx++;
if (++txtail > txbuf->size) if (++txtail > txbuf->size)
{ {
@@ -939,6 +937,16 @@ static void usbhost_txdata_work(FAR void *arg)
} }
} }
/* Save the updated tail pointer so that it cannot be sent again */
txbuf->tail = txtail;
/* Bytes were removed from the TX buffer. Inform any waiters that
* there is space available in the TX buffer.
*/
uart_datasent(uartdev);
/* Send the filled TX buffer to the CDC/ACM device */ /* Send the filled TX buffer to the CDC/ACM device */
nwritten = DRVR_TRANSFER(hport->drvr, priv->bulkout, nwritten = DRVR_TRANSFER(hport->drvr, priv->bulkout,
@@ -946,29 +954,26 @@ static void usbhost_txdata_work(FAR void *arg)
if (nwritten < 0) if (nwritten < 0)
{ {
/* The most likely reason for a failure is that CDC/ACM device /* The most likely reason for a failure is that CDC/ACM device
* NAK'ed our packet * NAK'ed our packet OR that the device has been disconnected.
* *
* Just break out of the loop, rescheduling the work. * Just break out of the loop, rescheduling the work (unless
* the device is disconnected).
*/ */
udbg("ERROR: DRVR_TRANSFER for packet failed: %d\n", (int)nwritten); udbg("ERROR: DRVR_TRANSFER for packet failed: %d\n", (int)nwritten);
break; break;
} }
/* Save the updated tail pointer so that it cannot be send again */
txbuf->tail = txtail;
} }
/* We get here because either 1) the UART TX buffer is empty and there is /* We get here because: 1) the UART TX buffer is empty and there is
* nothing more to send, or 2) the CDC/ACM device was not ready to accept * nothing more to send, 2) the CDC/ACM device was not ready to accept our
* our data. * data, or the device is no longer available.
* *
* If the last packet sent was and even multiple of the packet size, then * If the last packet sent was and even multiple of the packet size, then
* we need to send a zero length packet (ZLP). * we need to send a zero length packet (ZLP).
*/ */
if (nxfrd > 0 && txndx == priv->pktsize) if (txndx == priv->pktsize && !priv->disconnected)
{ {
/* Send the ZLP to the CDC/ACM device */ /* Send the ZLP to the CDC/ACM device */
@@ -984,13 +989,19 @@ static void usbhost_txdata_work(FAR void *arg)
} }
} }
/* If any bytes were removed from the buffer, inform any waiters there there is /* Check again if TX reception is enabled and that the device is still
* space available. * connected. These states could have changed since we started the
* transfer.
*/ */
if (nxfrd) if (priv->txena && !priv->disconnected)
{ {
uart_datasent(uartdev); /* Schedule TX data work to occur after a delay. */
ret = work_queue(LPWORK, &priv->txwork, usbhost_txdata_work, priv,
USBHOST_CDCACM_TXDELAY);
DEBUGASSERT(ret >= 0);
UNUSED(ret);
} }
} }
@@ -1029,21 +1040,6 @@ static void usbhost_rxdata_work(FAR void *arg)
uartdev = &priv->uartdev; uartdev = &priv->uartdev;
rxbuf = &uartdev->recv; rxbuf = &uartdev->recv;
/* Do nothing if RX reception is disabled or if RX flow control is in
* effect.
*/
#ifdef CONFIG_SERIAL_IFLOWCONTROL
if (!priv->rxena || !priv->rts)
#else
if (!priv->rxena)
#endif
{
/* Terminate the work now *without* rescheduling */
return;
}
/* Get the index in the RX packet buffer where we will take the first /* Get the index in the RX packet buffer where we will take the first
* byte of data. * byte of data.
*/ */
@@ -1066,9 +1062,14 @@ static void usbhost_rxdata_work(FAR void *arg)
* *
* 1. The UART RX buffer is full * 1. The UART RX buffer is full
* 2. There is no more data available from the CDC/ACM device * 2. There is no more data available from the CDC/ACM device
* 3. RX rec
*/ */
for (;;) #ifdef CONFIG_SERIAL_IFLOWCONTROL
while (priv->rxena && priv->rts && !priv->disconnected)
#else
while (priv->rxena && !priv->disconnected)
#endif
{ {
/* Stop now if there is no room for another character in the RX buffer. */ /* Stop now if there is no room for another character in the RX buffer. */
@@ -1089,12 +1090,15 @@ static void usbhost_rxdata_work(FAR void *arg)
priv->inbuf, priv->pktsize); priv->inbuf, priv->pktsize);
if (nread < 0) if (nread < 0)
{ {
/* The most likely reason for a failure is that the /* The most likely reason for a failure is that the has no
* has no data available now and NAK'ed the IN token. * data available now and NAK'ed the IN token OR that the
* transfer was cancelled because the device was disconnected.
* *
* Just break out of the loop, rescheduling the work. * Just break out of the loop, rescheduling the work (if the
* device was not disconnected.
*/ */
udbg("ERROR: DRVR_TRANSFER for packet failed: %d\n", (int)nread);
break; break;
} }
@@ -1104,7 +1108,7 @@ static void usbhost_rxdata_work(FAR void *arg)
* no interest to us. * no interest to us.
*/ */
DEBUGASSERT(nread <= priv->pktsize); DEBUGASSERT(nread <= priv->pktsize);
priv->nrxbytes = (uint16_t)nread; priv->nrxbytes = (uint16_t)nread;
rxndx = 0; rxndx = 0;
@@ -1148,6 +1152,7 @@ static void usbhost_rxdata_work(FAR void *arg)
*/ */
priv->nrxbytes = 0; priv->nrxbytes = 0;
priv->rxndx = 0;
/* Inform any waiters there there is new incoming data available. */ /* Inform any waiters there there is new incoming data available. */
@@ -1156,24 +1161,25 @@ static void usbhost_rxdata_work(FAR void *arg)
} }
} }
/* We break out to here if either: 1) the UART RX buffer is full or, /* We break out to here: 1) the UART RX buffer is full, 2) the CDC/ACM
* 2) the CDC/ACM device is not ready to provide us with more serial * device is not ready to provide us with more serial data, or 3) the
* data. * device has been disconnected.
* *
* Check again if RX reception is enabled. This state could have * Check if the device is still available: RX enabled, no RX flow
* changed since we started the transfer. * control in effect, and that the device is not disconnected. These
* states could have changed since we started the transfer.
*/ */
#ifdef CONFIG_SERIAL_IFLOWCONTROL #ifdef CONFIG_SERIAL_IFLOWCONTROL
if (priv->rxena && priv->rts && work_available(&priv->rxwork)) if (priv->rxena && priv->rts && work_available(&priv->rxwork) && !priv->disconnected)
#else #else
if (priv->rxena && work_available(&priv->rxwork) && !priv->disconnected) if (priv->rxena && work_available(&priv->rxwork) && !priv->disconnected)
#endif #endif
{ {
/* Schedule RX data reception work flow to occur after a delay. /* Schedule RX data reception work to occur after a delay. This will
* This will effect our responsive in certain cased. The delayed * affect our responsive in certain cases. The delayed work, however,
* work, however, will be cancelled and replaced with immediate * will be cancelled and replaced with immediate work when the upper
* work when the upper layer demands more data. * layer demands more data.
*/ */
ret = work_queue(LPWORK, &priv->rxwork, usbhost_rxdata_work, priv, ret = work_queue(LPWORK, &priv->rxwork, usbhost_rxdata_work, priv,
@@ -1964,7 +1970,9 @@ static int usbhost_connect(FAR struct usbhost_class_s *usbclass,
FAR const uint8_t *configdesc, int desclen) FAR const uint8_t *configdesc, int desclen)
{ {
FAR struct usbhost_cdcacm_s *priv = (FAR struct usbhost_cdcacm_s *)usbclass; FAR struct usbhost_cdcacm_s *priv = (FAR struct usbhost_cdcacm_s *)usbclass;
#ifdef HAVE_INTIN_ENDPOINT
FAR struct usbhost_hubport_s *hport; FAR struct usbhost_hubport_s *hport;
#endif
char devname[DEV_NAMELEN]; char devname[DEV_NAMELEN];
int ret; int ret;
@@ -1972,8 +1980,10 @@ static int usbhost_connect(FAR struct usbhost_class_s *usbclass,
configdesc != NULL && configdesc != NULL &&
desclen >= sizeof(struct usb_cfgdesc_s)); desclen >= sizeof(struct usb_cfgdesc_s));
#ifdef HAVE_INTIN_ENDPOINT
hport = priv->usbclass.hport; hport = priv->usbclass.hport;
DEBUGASSERT(hport); DEBUGASSERT(hport);
#endif
/* Get exclusive access to the device structure */ /* Get exclusive access to the device structure */
@@ -2542,12 +2552,16 @@ static void usbhost_rxint(FAR struct uart_dev_s *uartdev, bool enable)
if (enable && !priv->rxena) if (enable && !priv->rxena)
{ {
/* Restart RX data reception work flow unless RX flow control is in /* Cancel any pending, delayed RX data reception work */
* effect.
(void)work_cancel(LPWORK, &priv->rxwork);
/* Restart immediate RX data reception work (unless RX flow control
* is in* effect.
*/ */
#ifdef CONFIG_SERIAL_IFLOWCONTROL #ifdef CONFIG_SERIAL_IFLOWCONTROL
if (priv->rts && work_available(&priv->rxwork)) if (priv->rts))
#endif #endif
{ {
ret = work_queue(LPWORK, &priv->rxwork, ret = work_queue(LPWORK, &priv->rxwork,
@@ -2692,9 +2706,12 @@ static void usbhost_txint(FAR struct uart_dev_s *uartdev, bool enable)
if (enable && !priv->txena) if (enable && !priv->txena)
{ {
/* Restart TX data transmission work flow */ /* Cancel any pending, delayed TX data transmission work */
(void)work_cancel(LPWORK, &priv->txwork);
/* Restart immediate TX data transmission work */
DEBUGASSERT(work_available(&priv->txwork));
ret = work_queue(LPWORK, &priv->txwork, ret = work_queue(LPWORK, &priv->txwork,
usbhost_txdata_work, priv, 0); usbhost_txdata_work, priv, 0);
DEBUGASSERT(ret >= 0); DEBUGASSERT(ret >= 0);