From 9388a90842080217f054ac3655a94b0a28375c96 Mon Sep 17 00:00:00 2001 From: Damien George Date: Wed, 16 Apr 2014 15:51:27 +0100 Subject: [PATCH] stmhal: Fix USB CDC buffer overrun error. Need to wait for the low-level USB driver to send the data over the USB in-endpoint before the buffer can be used again. This patch adds a check for this. --- stmhal/usbd_cdc_interface.c | 119 +++++++++++++++++++++++++----------- 1 file changed, 84 insertions(+), 35 deletions(-) diff --git a/stmhal/usbd_cdc_interface.c b/stmhal/usbd_cdc_interface.c index c6b6cf23f6..552f7cc9ec 100644 --- a/stmhal/usbd_cdc_interface.c +++ b/stmhal/usbd_cdc_interface.c @@ -61,6 +61,8 @@ static uint16_t UserRxBufLen = 0; // counts number of valid characters in UserRx static uint8_t UserTxBuffer[APP_TX_DATA_SIZE]; // data for USB IN endpoind is stored in this buffer static uint16_t UserTxBufPtrIn = 0; // increment this pointer modulo APP_TX_DATA_SIZE when new data is available static __IO uint16_t UserTxBufPtrOut = 0; // increment this pointer modulo APP_TX_DATA_SIZE when data is drained +static uint16_t UserTxBufPtrOutShadow = 0; // shadow of above +static uint8_t UserTxBufPtrWaitCount = 0; // used to implement a timeout waiting for low-level USB driver static int user_interrupt_char = VCP_CHAR_NONE; static void *user_interrupt_data = NULL; @@ -250,41 +252,61 @@ static int8_t CDC_Itf_Control(uint8_t cmd, uint8_t* pbuf, uint16_t length) { * @retval None */ void USBD_CDC_HAL_TIM_PeriodElapsedCallback(void) { - uint32_t buffptr; - uint32_t buffsize; - - if(UserTxBufPtrOut != UserTxBufPtrIn) - { - if(UserTxBufPtrOut > UserTxBufPtrIn) /* rollback */ - { - buffsize = APP_TX_DATA_SIZE - UserTxBufPtrOut; + if (!dev_is_connected) { + // CDC device is not connected to a host, so we are unable to send any data + return; } - else - { - buffsize = UserTxBufPtrIn - UserTxBufPtrOut; - } - - buffptr = UserTxBufPtrOut; - // dpgeorge: For some reason that I don't understand, a packet size of 64 bytes - // (CDC_DATA_FS_MAX_PACKET_SIZE) does not get through to the USB host until the - // next packet is sent. To work around this, we just make sure that we never - // send a packet 64 bytes in length. - if (buffsize == CDC_DATA_FS_MAX_PACKET_SIZE) { - buffsize -= 1; + if (UserTxBufPtrOut == UserTxBufPtrIn) { + // No outstanding data to send + return; } - - USBD_CDC_SetTxBuffer(&hUSBDDevice, (uint8_t*)&UserTxBuffer[buffptr], buffsize); - - if(USBD_CDC_TransmitPacket(&hUSBDDevice) == USBD_OK) - { - UserTxBufPtrOut += buffsize; - if (UserTxBufPtrOut == APP_TX_DATA_SIZE) - { - UserTxBufPtrOut = 0; - } + + if (UserTxBufPtrOut != UserTxBufPtrOutShadow) { + // We have sent data and are waiting for the low-level USB driver to + // finish sending it over the USB in-endpoint. + if (UserTxBufPtrWaitCount < 10) { + PCD_HandleTypeDef *hpcd = hUSBDDevice.pData; + USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; + if (USBx_INEP(3)->DIEPTSIZ & USB_OTG_DIEPTSIZ_XFRSIZ) { + // USB in-endpoint is still reading the data + UserTxBufPtrWaitCount++; + return; + } + } + UserTxBufPtrOut = UserTxBufPtrOutShadow; + } + + if (UserTxBufPtrOutShadow != UserTxBufPtrIn) { + uint32_t buffptr; + uint32_t buffsize; + + if (UserTxBufPtrOutShadow > UserTxBufPtrIn) { // rollback + buffsize = APP_TX_DATA_SIZE - UserTxBufPtrOutShadow; + } else { + buffsize = UserTxBufPtrIn - UserTxBufPtrOutShadow; + } + + buffptr = UserTxBufPtrOutShadow; + + // dpgeorge: For some reason that I don't understand, a packet size of 64 bytes + // (CDC_DATA_FS_MAX_PACKET_SIZE) does not get through to the USB host until the + // next packet is sent. To work around this, we just make sure that we never + // send a packet 64 bytes in length. + if (buffsize == CDC_DATA_FS_MAX_PACKET_SIZE) { + buffsize -= 1; + } + + USBD_CDC_SetTxBuffer(&hUSBDDevice, (uint8_t*)&UserTxBuffer[buffptr], buffsize); + + if (USBD_CDC_TransmitPacket(&hUSBDDevice) == USBD_OK) { + UserTxBufPtrOutShadow += buffsize; + if (UserTxBufPtrOutShadow == APP_TX_DATA_SIZE) { + UserTxBufPtrOutShadow = 0; + } + UserTxBufPtrWaitCount = 0; + } } - } } /** @@ -370,11 +392,38 @@ void USBD_CDC_SetInterrupt(int chr, void *data) { void USBD_CDC_Tx(const char *str, uint32_t len) { for (int i = 0; i < len; i++) { - // if the buffer is full, wait until it gets drained, with a timeout of 1000ms (wraparound of tick is taken care of by 2's complement arithmetic) - uint32_t start = HAL_GetTick(); - while (((UserTxBufPtrIn + 1) & (APP_TX_DATA_SIZE - 1)) == UserTxBufPtrOut && HAL_GetTick() - start <= 1000) { - __WFI(); // enter sleep mode, waiting for interrupt + // If the CDC device is not connected to the host then we don't have anyone to receive our data. + // The device may become connected in the future, so we should at least try to fill the buffer + // and hope that it doesn't overflow by the time the device connects. + // If the device is not connected then we should go ahead and fill the buffer straight away, + // ignoring overflow. Otherwise, we should make sure that we have enough room in the buffer. + if (dev_is_connected) { + // If the buffer is full, wait until it gets drained, with a timeout of 500ms + // (wraparound of tick is taken care of by 2's complement arithmetic). + uint32_t start = HAL_GetTick(); + while (((UserTxBufPtrIn + 1) & (APP_TX_DATA_SIZE - 1)) == UserTxBufPtrOut && HAL_GetTick() - start <= 500) { + __WFI(); // enter sleep mode, waiting for interrupt + } + + // Some unused code that makes sure the low-level USB buffer is drained. + // Waiting for low-level is handled in USBD_CDC_HAL_TIM_PeriodElapsedCallback. + /* + start = HAL_GetTick(); + PCD_HandleTypeDef *hpcd = hUSBDDevice.pData; + if (hpcd->IN_ep[0x83 & 0x7f].is_in) { + //volatile uint32_t *xfer_count = &hpcd->IN_ep[0x83 & 0x7f].xfer_count; + //volatile uint32_t *xfer_len = &hpcd->IN_ep[0x83 & 0x7f].xfer_len; + USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; + while ( + // *xfer_count < *xfer_len // using this works + // (USBx_INEP(3)->DIEPTSIZ & USB_OTG_DIEPTSIZ_XFRSIZ) // using this works + && HAL_GetTick() - start <= 2000) { + __WFI(); // enter sleep mode, waiting for interrupt + } + } + */ } + UserTxBuffer[UserTxBufPtrIn] = str[i]; UserTxBufPtrIn = (UserTxBufPtrIn + 1) & (APP_TX_DATA_SIZE - 1); }