Several bugs in function HAL_SPI_TransmitReceive from STM32Cube_FW_H7_V1.4.0

[Gpeti]

For the record the function HAL_SPI_TransmitReceive does not work for a data size of 32 bits.

There are at least 2 bugs:

HAL_StatusTypeDef HAL_SPI_TransmitReceive(SPI_HandleTypeDef *hspi, uint8_t *pTxData, uint8_t *pRxData, uint16_t Size,
                                          uint32_t Timeout)
{
  HAL_SPI_StateTypeDef tmp_state;
  HAL_StatusTypeDef errorcode = HAL_OK;
 
  uint32_t   tickstart;
  uint32_t   tmp_mode;
  uint16_t   initial_TxXferCount;
  uint16_t   initial_RxXferCount;
 
  /* Check Direction parameter */
  assert_param(IS_SPI_DIRECTION_2LINES(hspi->Init.Direction));
 
  /* Process Locked */
  __HAL_LOCK(hspi);
 
  /* Init tickstart for timeout management*/
  tickstart = HAL_GetTick();
 
  initial_TxXferCount = Size;
  initial_RxXferCount = Size;
  tmp_state           = hspi->State;
  tmp_mode            = hspi->Init.Mode;
 
  if (!((tmp_state == HAL_SPI_STATE_READY) || \
        ((tmp_mode == SPI_MODE_MASTER) && (hspi->Init.Direction == SPI_DIRECTION_2LINES) && (tmp_state == HAL_SPI_STATE_BUSY_RX))))
  {
    errorcode = HAL_BUSY;
    __HAL_UNLOCK(hspi);
    return errorcode;
  }
 
  if ((pTxData == NULL) || (pRxData == NULL) || (Size == 0UL))
  {
    errorcode = HAL_ERROR;
    __HAL_UNLOCK(hspi);
    return errorcode;
  }
 
  /* Don't overwrite in case of HAL_SPI_STATE_BUSY_RX */
  if (hspi->State != HAL_SPI_STATE_BUSY_RX)
  {
    hspi->State = HAL_SPI_STATE_BUSY_TX_RX;
  }
 
  /* Set the transaction information */
  hspi->ErrorCode   = HAL_SPI_ERROR_NONE;
  hspi->pRxBuffPtr  = (uint8_t *)pRxData;
  hspi->RxXferCount = Size;
  hspi->RxXferSize  = Size;
  hspi->pTxBuffPtr  = (uint8_t *)pTxData;
  hspi->TxXferCount = Size;
  hspi->TxXferSize  = Size;
 
  /*Init field not used in handle to zero */
  hspi->RxISR       = NULL;
  hspi->TxISR       = NULL;
 
  /* Set the number of data at current transfer */
 MODIFY_REG(hspi->Instance->CR2, SPI_CR2_TSIZE, Size);
 
 
  __HAL_SPI_ENABLE(hspi);
 
  if (hspi->Init.Mode == SPI_MODE_MASTER)
  {
    /* Master transfer start */
    SET_BIT(hspi->Instance->CR1, SPI_CR1_CSTART);
  }
 
  /* Transmit and Receive data in 32 Bit mode */
  if (hspi->Init.DataSize > SPI_DATASIZE_16BIT)
  {
    while ((initial_TxXferCount > 0UL) || (initial_RxXferCount > 0UL))
    {
      /* Check TXP flag */
      if ((__HAL_SPI_GET_FLAG(hspi, SPI_FLAG_TXP)) && (initial_TxXferCount > 0UL))
      {
        *((__IO uint32_t *)&hspi->Instance->TXDR) = *((uint32_t *)hspi->pTxBuffPtr);
        hspi->pTxBuffPtr += sizeof(uint32_t);
        hspi->TxXferCount --;
        initial_TxXferCount = hspi->TxXferCount;
      }
 
      /* Check RXWNE/EOT flag */
      if (((hspi->Instance->SR & (SPI_FLAG_RXWNE | SPI_FLAG_EOT)) != 0UL) && (initial_RxXferCount > 0UL))
      {
        *((uint32_t *)hspi->pRxBuffPtr) = *((__IO uint32_t *)&hspi->Instance->RXDR);
        hspi->pRxBuffPtr += sizeof(uint32_t);
        hspi->RxXferCount --;
        initial_RxXferCount = hspi->RxXferCount;
      }
    }

1) Line 61: TSIZE should be the number of transfer not the number of bytes.

2) lines 82 and 91: the counters should be decremented by 4 and not 1.

Apparently the function has never been tested with a data size of 32 bits, not even once, as it fails even in the nominal case.

[Gpeti]

There is also an endianness issue when reading data from SPI register

[Amel NASRI]

Hi @Gpeti​ ,

Thanks for bringing these issues to our attention. I'll check them and take needed actions to report them internally.

Could you please provide more details about what you call endianness issue?

Thanks,

Amel

[Gpeti]

Sure. The bytes must be inverted when read from or written to SPI registers.

Something like that:

*((uint32_t *)hspi->pRxBuffPtr) = __REV( *((__IO uint32_t *)&hspi->Instance->RXDR) );