CDC_ReceiveQueue fails after CPU halt

[gigaj0ule]

trafficstars

I have not pinned down the source of this bug yet, but it is reproducible on both windows and linux.

1. Initialize an STM32F1 (maybe others too) and enumerate as CDC device.
2. Halt core with GDB
3. Send stuff to device such that multiple USB packets are enqeued.
4. Resume core

CDC_ReceiveQueue_ReadSize and other CDC_ReceiveQueue functions will now be out-of-sync with received packets until the device undergoes a host-initiated USB reset or unplug event.

[fpistm]

Hi @adammunich Not an expert about that but in that case I don't think this could be possible to keep it sync.

[gigaj0ule]

My understanding is that the host will receive NACKs when the 64 byte endpoint buffer is full, but send them after core resumes and empties the buffer. So, something must becoming out-of-sync on resume. I have not figured out what yet. But the behavior i encountered, is that the rxqueue size may be zero, but there are still "ghost" packet in the EP rx buffer which is not emptied until you send another packet from the host.

[gigaj0ule]

I think I fixed it after a solid 10 hours of late night troubleshooting.

We have to change the following line in usbd_ep_conf.c

{CDC_OUT_EP, PMA_CDC_OUT_ADDR, PCD_DBL_BUF},

To use single buffer mode.

{CDC_OUT_EP, PMA_CDC_OUT_ADDR, PCD_SNG_BUF},

The (ST hal?) software is otherwise unable to keep track of the ping-pong state of the double buffer when debugging with GDB (or if the core is halted some other way).

If I halt the STM32 and let the computer send two packets and timeout, when it resumes the USB is always one packet behind unless it is hardware reset by kernel. This ghost in the shell that doesn't make it out until you send another packet, which then takes place of the ghost packet.

Note: this is a really serious error that makes the microcontroller unrecoverable even in soft reset. It must be physically disconnected from the computer which is really bad news for reliability, so I suggest we push this fix to the arduino core.

[fpistm]

@adammunich thanks for feedback, anyway, I guess this should not be the good fix. For F1, double buffering is used so this seems not a good idea to change this. @makarenya made a lot of work around this and I know I've not backport all their fixes when I've updated the STM32 USB device middleware because it was hard to maintain them.

[gigaj0ule]

This problem has re appeared as someone had over-written the fix https://github.com/fpistm/Arduino_Core_STM32/commit/90b75ccc3ccdb4242e0092cffef6b443bb034331 had committed

It was very troublesome to figure out the source of garbage serial data again.

[gigaj0ule]

To fix it we must update const ep_desc_t ep_def[] to:

/* Includes ------------------------------------------------------------------*/
#include "usbd_ep_conf.h"

#ifdef USBD_USE_CDC
const ep_desc_t ep_def[] = {
#ifdef USE_USB_HS
  {0x00,       CDC_DATA_HS_MAX_PACKET_SIZE},
  {0x80,       CDC_DATA_HS_MAX_PACKET_SIZE},
  {CDC_OUT_EP, CDC_DATA_HS_MAX_PACKET_SIZE},
  {CDC_IN_EP,  CDC_DATA_HS_MAX_PACKET_SIZE},
  {CDC_CMD_EP, CDC_CMD_PACKET_SIZE}
#else /* USE_USB_FS */
#ifdef USB_OTG_FS
  {0x00,       CDC_DATA_FS_MAX_PACKET_SIZE},
  {0x80,       CDC_DATA_FS_MAX_PACKET_SIZE},
  {CDC_OUT_EP, CDC_DATA_FS_MAX_PACKET_SIZE},
  {CDC_IN_EP,  CDC_DATA_FS_MAX_PACKET_SIZE},
  {CDC_CMD_EP, CDC_CMD_PACKET_SIZE}
#else
  {0x00,       PMA_EP0_OUT_ADDR, PCD_SNG_BUF},
  {0x80,       PMA_EP0_IN_ADDR,  PCD_SNG_BUF},
//  {CDC_OUT_EP, PMA_CDC_OUT_ADDR, PCD_DBL_BUF},
#ifndef USBD_CDC_USE_SINGLE_BUFFER
  {CDC_OUT_EP, PMA_CDC_OUT_ADDR, PCD_DBL_BUF},
#else
  {CDC_OUT_EP, PMA_CDC_OUT_ADDR, PCD_SNG_BUF},
#endif
  {CDC_IN_EP,  PMA_CDC_IN_ADDR,  PCD_SNG_BUF},
  {CDC_CMD_EP, PMA_CDC_CMD_ADDR, PCD_SNG_BUF}
#endif
#endif
};
#endif /* USBD_USE_CDC */

[fpistm]

igure out the source of garbage serial data again

As stated, this commit is part of my fork and was never merged in the official repo.