STMicroelectronics / stm32g0xx_hal_driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32G0 series.
BSD 3-Clause "New" or "Revised" License
13 stars 4 forks source link

USB OUT stall's on high bus load #2

Open lichtfeind opened 1 year ago

lichtfeind commented 1 year ago

I have a firmware running on the NUCLEO-G0B1RE that receives bulk out transfers over USB and echos them back. The endpoint is setup for double buffering. At high bus load some transfers are aborted with a STALL.

The firmware is ported by @marckleinebudde can be found here https://github.com/marckleinebudde/candleLight_fw/tree/stm32g0

I suspect the cause to be in Inc/stm32g0xx_ll_usb.h#L659. It does two writes to the buffer descriptor table. One to clear BLSIZE, NUM_BLOCK and COUNTn_RX and one the set BLSIZE and NUM_BLOCK. This results in a small window in which NUM_BLOCK is 0 while the endpoint is valid.

As stated in RM0444 37.5.2

Reception memory buffer locations are written starting from the address contained in the ADDRn_RX for a number of bytes corresponding to the received data packet length, or up to the last allocated memory location, as defined by BLSIZE and NUM_BLOCK, whichever comes first. In this way, the USB peripheral never writes beyond the end of the allocated reception memory buffer area. If the length of the data packet payload (actual number of bytes used by the application) is greater than the allocated buffer, the USB peripheral detects a buffer overrun condition. In this case, a STALL handshake is sent instead of the usual ACK to notify the problem to the host, no interrupt is generated and the transaction is considered failed.

So either make sure COUNTn_RX is cleared without clearing NUM_BLOCK or setting STATRX to NAK while changing the entry.

ALABSTM commented 5 months ago

Hi @lichtfeind,

Please excuse this delayed reply. Your point looks relevant. It has been forwarded to our development teams for further analysis. I will keep you informed.

To be sure I correctly understood, you suspect the below code block to cause the issue, mainly the two successive accesses to the buffer descriptor table via the pdwReg parameter:

  1. to clear both BLSIZE and NBLK,
  2. to set them back.
#define USB_DRD_SET_CHEP_CNT_RX_REG(pdwReg, wCount) \
  do { \
    uint32_t wNBlocks; \
    \
    (pdwReg) &= ~(USB_CNTRX_BLSIZE | USB_CNTRX_NBLK_MSK); \
    \
    if ((wCount) > 62U) \
    { \
      USB_DRD_CALC_BLK32((pdwReg), (wCount), wNBlocks); \
    } \
    else \
    { \
      if ((wCount) == 0U) \
      { \
        (pdwReg) |= USB_CNTRX_BLSIZE; \
      } \
      else \
      { \
        USB_DRD_CALC_BLK2((pdwReg), (wCount), wNBlocks); \
      } \
    } \
  } while(0) /* USB_DRD_SET_CHEP_CNT_RX_REG */

With regards,

ALABSTM commented 5 months ago

ST Internal Reference: 173021

ALABSTM commented 4 months ago

Hi @lichtfeind,

Just wondering. Did you give it another try with the latest HAL-LL version available on GitHub to see whether the problem still persists? A number of fixes have been pushed since you first created this issue, some related to the LL USB driver.

Please let me know and thank you in advance.

With regards,

marckleinebudde commented 4 months ago

Hi @ALABSTM,

To be sure I correctly understood, you suspect the below code block to cause the issue, mainly the two successive accesses to the buffer descriptor table via the pdwReg parameter:

The issue it that there is a small window, where both COUNTn_RX and NUM_BLOCK are cleared. A received USB frame results in a stall, see patch description of https://github.com/candle-usb/candleLight_fw/pull/156/commits/dffacdeafa9cfe196d76ec8f3b90839f043fcea2

marckleinebudde commented 4 months ago

Just wondering. Did you give it another try with the latest HAL-LL version available on GitHub to see whether the problem still persists? A number of fixes have been pushed since you first created this issue, some related to the LL USB driver.

The problematic code is still in place, maybe this version fixes other problems, but not this one.