32F4 USB CDC - any way to tell if Host has retrieved the data sent to it?

PHolt.1 · ‎2022-09-04

The Cube IDE code flow is

CDC_Transmit_FS

USBD_CDC_TransmitPacket

USBD_LL_Transmit

HAL_PCD_EP_Transmit

USB_WritePacket

and the last one does 16 32 bit word transfers to the USB FIFO

HAL_StatusTypeDef USB_WritePacket(USB_OTG_GlobalTypeDef *USBx, uint8_t *src, uint8_t ch_ep_num, uint16_t len, uint8_t dma)
{
  uint32_t USBx_BASE = (uint32_t)USBx;
  uint32_t *pSrc = (uint32_t *)src;
  uint32_t count32b, i;
 
  if (dma == 0U)
  {
    count32b = ((uint32_t)len + 3U) / 4U;
    for (i = 0U; i < count32b; i++)
    {
      USBx_DFIFO((uint32_t)ch_ep_num) = *((__packed uint32_t *)pSrc);
      pSrc++;
    }
  }
 
  return HAL_OK;
}

My Q is whether there is any way to tell when that data has been consumed.

I have been doing tests and found

there is a ~10kHz rate limit on calling CDC_Transmit_FS
the max CDC_Transmit_FS packet size is 800 bytes

I am sure these are both system dependent, so I am rate limiting to 1kHz and size limiting to 256 bytes.

There are various return codes from various functions but none of these appear to relate to whether you can shove more data out.

Is USB really "open loop" like that? I know it is all polled by the PC, seemingly at ~10kHz

waclawek.jan · ‎2022-09-04

> My Q is whether there is any way to tell when that data has been consumed.

Yes. And it's described exactly where you would expect it: in the manual.

It's used in multipacket transfers.

> Is USB really "open loop" like that? I know it is all polled by the PC, seemingly at ~10kHz

I don't know what is open loop in that. The USB standard clearly determines how host shall schedule transfers. Bulk endpoints are the lowest priority with no guaranteed bandwidth; OTOH if there is no other endpoint on the given host port tree to transfer from/to i.e. if there's free bandwidth, host is free to send IN tokens to given bulk endpoint at any pace it chooses.

JW

PHolt.1 · ‎2022-09-05

Thank you.

So there should be a "transfer complete" interrupt somewhere.

I don't think it has been implemented.

The ideal way to do this is to have an "all sent" flag, or some such.

People have been here before

https://community.st.com/s/question/0D53W00000hPoRCSA0/cdctransmitfs-is-giving-me-a-headache-usb-cdc-device-mode

It is possible that USBD_BUSY returned by the function below indicates that the data could not be sent. In that case should one re-send the same data?

/**
  * @brief  CDC_Transmit_FS
  *         Data to send over USB IN endpoint are sent over CDC interface
  *         through this function.
  *
  *
  * @param  Buf: Buffer of data to be sent
  * @param  Len: Number of data to be sent (in bytes)
  * @retval USBD_OK if all operations are OK else USBD_FAIL or USBD_BUSY
  *
  * This function has a call rate limit of about 10kHz, so the caller needs to
  * enforce that.
  *
  */
uint8_t CDC_Transmit_FS(uint8_t* Buf, uint16_t Len)
{
	uint8_t result = USBD_OK;
 
	// Prevent re-entrancy from multiple threads. This function is called from both
	// serial.c port 0 and from debugprint.c
	// This function should never block for any significant time
	osMutexAcquire(g_CDC_transmit_mutex, osWaitForever);
 
	USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)hUsbDeviceFS.pClassDataCDC;
 
	if (hcdc->TxState != 0){
		result =  USBD_BUSY;
	}
 
	if (result == USBD_OK) {
		USBD_CDC_SetTxBuffer(&hUsbDeviceFS, Buf, Len);
		result = USBD_CDC_TransmitPacket(&hUsbDeviceFS);
	}
 
	osMutexRelease(g_CDC_transmit_mutex);
 
	return result;
}

but when I look at what is controlling the Txstate flag, it doesn't seem to be anything to do with the Host.

Anyway, this seems to work (change to above function):

/**
  * @brief  CDC_Transmit_FS
  *         Data to send over USB IN endpoint are sent over CDC interface
  *         through this function.
  *
  *
  * @param  Buf: Buffer of data to be sent
  * @param  Len: Number of data to be sent (in bytes)
  * @retval USBD_OK if all operations are OK else USBD_FAIL or USBD_BUSY
  *
  * This function had a call rate limit of about 10kHz, until the mod below.
  * USBD_BUSY is never returned; the function waits instead.
  * 
  */
uint8_t CDC_Transmit_FS(uint8_t* Buf, uint16_t Len)
{
	uint8_t result = USBD_OK;
 
	// Prevent re-entrancy from multiple threads. This function is called from both
	// serial.c port 0 and from debugprint.c
	// This function should never block for any significant time
	osMutexAcquire(g_CDC_transmit_mutex, osWaitForever);
 
	USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)hUsbDeviceFS.pClassDataCDC;
 
	// Change 5/9/22 PH to wait until Host has collected the data
	do
	{
		USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)hUsbDeviceFS.pClassDataCDC;
 
/*
	if (hcdc->TxState != 0)
	{
		result =  USBD_BUSY;
	}
	*/
 
		if (hcdc->TxState != 0)	osDelay(1);
	}
	while (hcdc->TxState != 0);
 
	if (result == USBD_OK) {
		USBD_CDC_SetTxBuffer(&hUsbDeviceFS, Buf, Len);
		result = USBD_CDC_TransmitPacket(&hUsbDeviceFS);
	}
 
	osMutexRelease(g_CDC_transmit_mutex);
 
	return result;
}

waclawek.jan · ‎2022-09-05

> So there should be a "transfer complete" interrupt somewhere.

Yes. And the manual clearly says, where and how it is called.

> I don't think it has been implemented.

In hardware? Of course it is. In Cube? I don't care but yes, it is, in some way. Look it up, Cube is open source.

Have you read the Cube USB Device driver's manual?

> The ideal way to do this is to have an "all sent" flag, or some such.

Then implement it. I've just told you, how. It's your program.

JW

waclawek.jan · ‎2022-09-05

OK looked it up for you in a 'F4 Cube v1.21 CDC example.

The In XFRC interrupt calls HAL_PCD_DataInStageCallback() (or the same through the "registered" mechanism) which as with all callbacks is implemented as weak and is supposed to be implemented by user.

The CDC example implements it as a straighforward call to USBD_LL_DataInStage(), which for non-EP0 implements the multi-packet transfers and finally calls pdev->pClass->DataIn(), that in the CDC example points to USBD_CDC_DataIn() and that appears to signal through hcdc->TxState = 0;

JW

PHolt.1 · ‎2022-09-05

Thank you.

I am doing an osDelay(1) on TxState!=0 and that seems to be solid. It is obvious that something is toggling this flag, and it has to be an interrupt.

No amount of data seems to break it now.

However I still left the packet size limit in there, at 256 bytes. Maybe this is pointless and maybe one can just pass a 64k packet to CDC_Transmit_FS but there is quite a lot on google about people who had problems with this.

EDIT: Searching my project for HAL_PCD_DataInStageCallback() I can see many references but cannot work out how this stuff works. I found what looks like the actual function here

void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum)
{
  USBD_LL_DataInStage((USBD_HandleTypeDef*)hpcd->pData, epnum, hpcd->IN_ep[epnum].xfer_buff);
}

and that calls

/**
* @brief  USBD_DataInStage
*         Handle data in stage
* @param  pdev: device instance
* @param  epnum: endpoint index
* @retval status
*/
USBD_StatusTypeDef USBD_LL_DataInStage(USBD_HandleTypeDef *pdev, uint8_t epnum,
                                       uint8_t *pdata)
{
  USBD_EndpointTypeDef *pep;
 
  if(epnum == 0U)
  {
    pep = &pdev->ep_in[0];
 
    if ( pdev->ep0_state == USBD_EP0_DATA_IN)
    {
      if(pep->rem_length > pep->maxpacket)
      {
        pep->rem_length -= pep->maxpacket;
 
        USBD_CtlContinueSendData (pdev, pdata, (uint16_t)pep->rem_length);
 
        /* Prepare endpoint for premature end of transfer */
        USBD_LL_PrepareReceive (pdev, 0U, NULL, 0U);
      }
      else
      { /* last packet is MPS multiple, so send ZLP packet */
        if((pep->total_length % pep->maxpacket == 0U) &&
           (pep->total_length >= pep->maxpacket) &&
           (pep->total_length < pdev->ep0_data_len))
        {
          USBD_CtlContinueSendData(pdev, NULL, 0U);
          pdev->ep0_data_len = 0U;
 
          /* Prepare endpoint for premature end of transfer */
          USBD_LL_PrepareReceive (pdev, 0U, NULL, 0U);
        }
        else
        {
          if((pdev->pClass->EP0_TxSent != NULL)&&
             (pdev->dev_state == USBD_STATE_CONFIGURED))
          {
            pdev->pClass->EP0_TxSent(pdev);
          }
          USBD_LL_StallEP(pdev, 0x80U);
          USBD_CtlReceiveStatus(pdev);
        }
      }
    }
    else
    {
      if ((pdev->ep0_state == USBD_EP0_STATUS_IN) ||
          (pdev->ep0_state == USBD_EP0_IDLE))
      {
        USBD_LL_StallEP(pdev, 0x80U);
      }
    }
 
    if (pdev->dev_test_mode == 1U)
    {
      USBD_RunTestMode(pdev);
      pdev->dev_test_mode = 0U;
    }
  }
  else if((pdev->pClass->DataIn != NULL) &&
          (pdev->dev_state == USBD_STATE_CONFIGURED))
  {
    pdev->pClass->DataIn(pdev, epnum);
  }
  else
  {
    /* should never be in this condition */
    return USBD_FAIL;
  }
 
  return USBD_OK;
}

I still have corruption on CDC input, at around 1% level, but I am OK with that for what I am doing. It won't be a documented feature on the finished product.

PHolt.1 · ‎2022-09-05

Updated code

/**
  * @brief  CDC_Transmit_FS
  *         Data to send over USB IN endpoint are sent over CDC interface
  *         through this function.
  *
  *
  * @param  Buf: Buffer of data to be sent
  * @param  Len: Number of data to be sent (in bytes)
  * @retval USBD_OK if all operations are OK else USBD_FAIL or USBD_BUSY
  *
  * This function had a call rate limit of about 10kHz, and a packet length limit
  * of about 800 bytes - until the osDelay mod below.
  * USBD_BUSY is never returned; the function waits instead.
  *
  */
uint8_t CDC_Transmit_FS(uint8_t* Buf, uint16_t Len)
{
	uint8_t result = USBD_OK;
 
	// This function is called from both serial.c port 0 and from debugprint.c.
	osMutexAcquire(g_CDC_transmit_mutex, osWaitForever);
 
	bool busy=false;
	do
	{
		USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)hUsbDeviceFS.pClassDataCDC;
 
/*
 *  old code
	if (hcdc->TxState != 0)
	{
		result =  USBD_BUSY;
	}
*/
		if (hcdc->TxState != 0)
		{
			busy=true;
			osDelay(1);
		}
		else
		{
			busy=false;
		}
	}
	while (busy);
 
	if (result == USBD_OK) {
		USBD_CDC_SetTxBuffer(&hUsbDeviceFS, Buf, Len);
		result = USBD_CDC_TransmitPacket(&hUsbDeviceFS);
	}
 
	osMutexRelease(g_CDC_transmit_mutex);
 
	return result;
}

Piranha · ‎2022-09-05

Rewrote that function for you...

uint8_t CDC_Transmit_FS(uint8_t* Buf, uint16_t Len)
{
	// This function is called from both serial.c port 0 and from debugprint.c.
	osMutexAcquire(g_CDC_transmit_mutex, osWaitForever);
 
	USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)hUsbDeviceFS.pClassDataCDC;
 
	while (hcdc->TxState) {
		osDelay(1);
	}
 
	USBD_CDC_SetTxBuffer(&hUsbDeviceFS, Buf, Len);
	uint8_t result = USBD_CDC_TransmitPacket(&hUsbDeviceFS);
 
	osMutexRelease(g_CDC_transmit_mutex);
 
	return result;
}

Cannot comment on the functionality apart from the fact that as always the delay tries to "fix" something broken.

PHolt.1 · ‎2022-09-05

Thank you : - ) Yes I realised my code had some junk in it.

My C is quite simple and to aid documentation I would write

while (hcdc->TxState != 0)

Also USBD_CDC_TransmitPacket gets called only if TxState=0 so it cannot ever return USBD_BUSY.

This all works great - thanks.

The osDelay(1) yields to other RTOS tasks. In reality that wait loop is very fast - empirically about 80us of wait, but yielding to RTOS (for 0 to 1000us in this case) is the right way to structure a quantity of tasks which don't have any special relative priorities. I have about 20 tasks which all run nicely, in IDLE priority. And a few which are higher.

If I wanted highest possible USB performance I would hang in that loop, with a counter which exits after say a few ms (some hardware failure). I use that method (without a counter) where the only possible failure would be defective silicon.

Piranha · ‎2022-09-06

> while (hcdc->TxState != 0)

That is necessary for people, who don't know the C language... ;)

> I have about 20 tasks which all run nicely, in IDLE priority. And a few which are higher.

Idle thread should run at the lowest possible priority and should be the only thread running at that priority, otherwise it's not really an idle thread. And typically it should put the CPU into sleep mode by just executing __DSB(); __WFI(); . Priorities should be ordered something like this:

Higher priority threads.
Normal priority threads.
Timer thread.
Idle thread.