LPC1347 USB VCOM Loopback problem

[lpcware]

Content originally posted in LPCWare by dtesystems on Mon Jul 13 02:10:15 MST 2015
Hi,

I've recently switched from using microchip microcontrollers to NXP Cortex-M3 microcontrollers. One of the advantages seemed the build in USB.

While converting my communications library I noticed a problem with the USB communication, Trying the example code on the LPC Expresso 1347 devboard confirmed this error, so I'll explain it applied to these example projects.

I'm testing the robustness of the USB vcom interface by dumping 60 characters long data on the VCOM port in loopback mode with

1) usbd_rom_cdc_uart (with rx and tx of the USART bridged of course) -> No problem what so ever
2) usbd_rom_cdc (which is an example of a vcom loopback program) -> characters are lost during transmission

I haven't made any changes to the code.

I'm using LPCXpresso v7.8.0 with all the latest versions of the examples.
I'm using this terminal-app sites.google.com/site/terminalbpp and sending this string once every second.

[color=#900]012345678901234567890123456789012345678901234567890123456789 with +CR checked[/color]

[img]https://googledrive.com/host/0B3SVyKJMOKikfllOUkpVVHVIM0lyeElvSWNSMnhpbjFFSExrX2tXbjA4NTdvLUlUdnM1U2...

As you can see, characters are lost during transmission. I'm sending a character length below the RX and TX buffer length. When example (1) handles this string, there is a double interrupt used. One for the USB and one for the USART, without any problems.

I think it has something to do with the critical sections in the cdc_vcom.c code, where the USB interrupt gets disabled during a short time.

vcom_bread:

/* Virtual com port buffered read routine */
uint32_t vcom_bread(uint8_t *pBuf, uint32_t buf_len)
{
VCOM_DATA_T *pVcom = &g_vCOM;
uint16_t cnt = 0;
/* read from the default buffer if any data present */
if (pVcom->rx_count) {
cnt = (pVcom->rx_count < buf_len) ? pVcom->rx_count : buf_len;
memcpy(pBuf, pVcom->rx_buff, cnt);
pVcom->rx_rd_count += cnt;

/* enter critical section */
NVIC_DisableIRQ(USB0_IRQn);
if (pVcom->rx_rd_count >= pVcom->rx_count) {
pVcom->rx_flags &= ~VCOM_RX_BUF_FULL;
pVcom->rx_rd_count = pVcom->rx_count = 0;
}
/* exit critical section */
NVIC_EnableIRQ(USB0_IRQn);
}
return cnt;

}

vcom_read_req:

/* Virtual com port read routine */
ErrorCode_t vcom_read_req(uint8_t *pBuf, uint32_t len)
{
VCOM_DATA_T *pVcom = &g_vCOM;

/* check if we queued Rx buffer */
if (pVcom->rx_flags & (VCOM_RX_BUF_QUEUED | VCOM_RX_DB_QUEUED)) {
return ERR_BUSY;
}
/* enter critical section */
NVIC_DisableIRQ(USB0_IRQn);
/* if not queue the request and return 0 bytes */
USBD_API->hw->ReadReqEP(pVcom->hUsb, USB_CDC_OUT_EP, pBuf, len);
/* exit critical section */
NVIC_EnableIRQ(USB0_IRQn);
pVcom->rx_flags |= VCOM_RX_DB_QUEUED;

return LPC_OK;
}

vcom_read_cnt:

/* Gets current read count. */
uint32_t vcom_read_cnt(void)
{
VCOM_DATA_T *pVcom = &g_vCOM;
uint32_t ret = 0;

if (pVcom->rx_flags & VCOM_RX_DONE) {
ret = pVcom->rx_count;
pVcom->rx_count = 0;
}

return ret;
}

/* Virtual com port write routine*/
uint32_t vcom_write(uint8_t *pBuf, uint32_t len)
{
VCOM_DATA_T *pVcom = &g_vCOM;
uint32_t ret = 0;

if ( (pVcom->tx_flags & VCOM_TX_CONNECTED) && ((pVcom->tx_flags & VCOM_TX_BUSY) == 0) ) {
pVcom->tx_flags |= VCOM_TX_BUSY;

/* enter critical section */
NVIC_DisableIRQ(USB0_IRQn);
ret = USBD_API->hw->WriteEP(pVcom->hUsb, USB_CDC_IN_EP, pBuf, len);
/* exit critical section */
NVIC_EnableIRQ(USB0_IRQn);
}

return ret;
}

I can't seem to figure out why this goes wrong and how I can solve this. Since the USB ROM driver takes care of the filling of the IN_EP and the reading of the OUT_EP and the vom read/write functions of the opposite, I would expect the they didn't conflict with one and other.

By changing the main loop to following code, I can reduce the failure rate from 1/120 to 1/30000 on a 5 minute test. Which is already better but still not what I get with example 1 (0 failures on a 30min test at 100ms send interval).

changed main loop:

while (1) {
/* If VCOM port is opened echo whatever we receive back to host. */
if (vcom_connected() && g_vCOM.rx_count ) {
rdCnt = vcom_bread(&g_rxBuff[0], 256);
if (rdCnt) {
vcom_write(&g_rxBuff[0], rdCnt);
}
}
/* Sleep until next IRQ happens */
__WFI();
}

Can anybody help me with this?

[lpcware]

Content originally posted in LPCWare by SeleneSW on Wed Dec 30 09:27:11 MST 2015
Hi dtesystems,

While implementing USB CDC communication in one of our projects, I had your problem too.
After some investigation, I've found that the VCOM_bulk_out_hdlr function is broken in the sense that overrides the pVcom->rx_buff buffer with new data every time the interrupt is called.
In this way if the vcom_bread function is not called fast enought we lose some packets.
I've managed to fix this, implementing a check inside the VCOM_bulk_out_hdlr function, and if there is no space in the buffer for the new packet, I just don't do the USBD_API->hw->ReadEP call (with the side effect that the interrupt will not be called any time soon, because the USB peripheral has no space), and later in the vcom_bread when I make room reading data, I call the USBD_API->hw->ReadEP enabling the interrupt again.

[lpcware]

Content originally posted in LPCWare by dtesystems on Thu Jul 16 01:04:22 MST 2015
Thanks MH, Seems possible. I'll check it ASAP.

[lpcware]

Content originally posted in LPCWare by historysmistory on Tue Jul 14 06:23:55 MST 2015
I used this code a while ago on a different MCU and found I needed to make modifications to get these routines working correctly.

It seems the original buffered read routine is broken if the amount of data exceeds the supplied buffer size. Two things are wrong:

1 - the first call to vcom_bread sees that the buffer is smaller than the waiting data and correctly returns buf_len, does a memcpy() and updates its internal pointer and avoids saying the read is complete. However it all goes wrong after this... If the next call will fetch the last of the data, it returns buf_len again, and incorrectly increments its internal pointer by this amount - all of this is moot though because of the second problem.

2 - the memcpy always copies (returns) data from the start of the internal buffer i.e. it will always send back the same data each time

I believe the version below fixes the problem.

/* Virtual com port buffered read routine FIXED */

uint32_t vcom_bread_mod(uint8_t *pBuf, uint32_t buf_len)
{
VCOM_DATA_T *pVcom = &g_vCOM;
uint16_t cnt = 0;
/* read from the default buffer if any data present */
if (pVcom->rx_count) {
if ((pVcom->rx_count - pVcom->rx_rd_count) < buf_len) {
cnt = (pVcom->rx_count - pVcom->rx_rd_count);
}
else {
cnt = buf_len;
}
memcpy(pBuf, (pVcom->rx_buff + pVcom->rx_rd_count) , cnt);
pVcom->rx_rd_count += cnt;


/* enter critical section */
NVIC_DisableIRQ(LPC_USB_IRQ);
if (pVcom->rx_rd_count >= pVcom->rx_count) {
pVcom->rx_flags &= ~VCOM_RX_BUF_FULL;
pVcom->rx_rd_count = pVcom->rx_count = 0;
}
/* exit critical section */
NVIC_EnableIRQ(LPC_USB_IRQ);
}
return cnt;
}

Also the write routine needs to wait until not busy - so a fixed routine becomes:

/* Virtual com port write routine FIXED
 * Without this, written characters can be lost
 */

uint32_t vcom_write_mod(uint8_t *pBuf, uint32_t len) {
VCOM_DATA_T *pVcom = &g_vCOM;
uint32_t ret = 0;
// loop until ready to call function
while ((pVcom->tx_flags & VCOM_TX_CONNECTED)
&& ((pVcom->tx_flags & VCOM_TX_BUSY))) {
}

if ((pVcom->tx_flags & VCOM_TX_CONNECTED)
&& ((pVcom->tx_flags & VCOM_TX_BUSY) == 0)) {
pVcom->tx_flags |= VCOM_TX_BUSY;

/* enter critical section */
NVIC_DisableIRQ(LPC_USB_IRQ);
ret = USBD_API->hw->WriteEP(pVcom->hUsb, USB_CDC_IN_EP, pBuf, len);
/* exit critical section */
NVIC_EnableIRQ(LPC_USB_IRQ);
}

return ret;
}

[lpcware]

Content originally posted in LPCWare by dtesystems on Tue Jul 14 00:37:50 MST 2015
Addendum:

Until now I've been compiling the code as debug code, which resulted in a high error rate. Now I've switched to release mode and the error rate dropped to 1/250000 chars.

This is somewhat better, but still not acceptable.

Any ideas? Anybody?