HAL_SAI_Transmit() does not stop the I2S clocks when complete

[DerekR]

Hello,

Some background:

STM32CubeIDE 1.6.1

MCU: STM32L4R9ZGJ6

SDK: STM32Cube_FW_L4_V1.14.0 (Yes I know it's old ~ 2019) Project is too large to update at the moment.

I am currently debugging external DAC audio issues. I reverted to using simple SAI blocking functions for testing purposes. In doing so, I have discovered that HAL_SAI_Transmit() keeps all I2S clocks running after the specified amount of data is transmitted.

Question 1: Are the I2S clocks supposed to stop after HAL_SAI_Transmit() returns, or is it up to the user to call __HAL_SAI_DISABLE() to stop the clocks manually?

Looking at the STM32L4R9 reference manual (RM0432 Rev 5), I found the following on pg 1678:

"In master TX mode, enabling the audio block immediately generates the bit clock for the

external slaves even if there is no data in the FIFO, However FS signal generation is

conditioned by the presence of data in the FIFO. After the FIFO receives the first data to

transmit, this data is output to external slaves. If there is no data to transmit in the FIFO, 0

values are then sent in the audio frame with an underrun flag generation."

My concerns is with the statement "However FS signal generation is

conditioned by the presence of data in the FIFO". Looking at a logic analyzer, I see the FS signal (or WCLK) always toggling even after the FIFO is flushed onto the I2S bus. See screenshot below:

 

Question 2: Is this a bug in the STM MCU / driver, or is the reference manual incorrect?

I am attempting to transmit a small portion of an audio file as a test. The audio file is 44.1 KHz, stereo (2 channels), 16-bits per channel. Since the clocks keep running after HAL_SAI_Transmit() returns, garbage is sent to the DAC on the data line beyond the end of the specified buffer length as seen below:

 

Question 3: If the clocks do keep running after HAL_SAI_Transmit() returns, why am I seeing garbage output on the data line instead of 0x00 as defined in the reference manual? Is the only explanation that it could be coupling to the other clock lines and picking up noise?

Edit: I tried calling __HAL_SAI_DISABLE() after HAL_SAI_Transmit() returns and some garbage data is still output before the SAI clocks actually stop as seen below:

 

Interestingly the garbage data looks the same across multiple captures so this makes me think the SAI driver / hardware is incrementing beyond the end of the transmit buffer and outputting random data from RAM. I have verified the 'Size' value passed into HAL_SAI_Transmit() is the length of my buffer so I don't see why data beyond this would be output.

Thanks,

Derek

[waclawek.jan]

> FS signal generation is conditioned by the presence of data in the FIFO

This wording is cumbersome but IMO it means that FS won't start until first data are written, but then remains toggling indefinitely.

I don't know what are the data you see, I wouldn't expect SAI to release the data line until disabled.

If you are uncertain what Cube/HAL functions do, simply read them, Cube is open source.

JW

[DerekR]

Hello @Community member​ ,

According to the HAL API document:

 

Based on this description, HAL_SAI_Transmit() will "Transmit an amount of data in blocking mode" where Size: Amount of data to be sent.

Looking at the logic analyzer, more than "Size" bytes are output on the bus. According to the STM32L4R9 reference manual: "If there is no data to transmit in the FIFO, 0 values are then sent in the audio frame with an underrun flag generation."

So I would expect the data line to be held low after HAL_SAI_Transmit() transmits all bytes. Instead, garbage (random data) is being output. I guess I will start debugging the HAL SAI driver...

Thanks,

Derek

[DerekR · Accepted solution]

Edit 2: Putting this at the top to save others time of reading this whole post. After investigating a bit more, this turned out to be a lack of documentation. The SDK examples pass a buffer of type uint16_t into HAL_SAI_Transmit_DMA() which accounts for the 16-bit data size. For example:

uint16_t PlayBuff[PLAY_BUFF_SIZE];

HAL_SAI_Transmit_DMA(&SaiHandle, (uint8_t *)PlayBuff, PLAY_BUFF_SIZE);

In the above example, PLAY_BUFF_SIZE is the number is 16-bit data elements which matches the configuration of SAI_DATASIZE_16, not the number of bytes. I made the assumption that the "Size" parameter in HAL_SAI_Transmit() was in units of bytes like most of the other STM32 HAL functions. I think the HAL documentation and function header should be changed from "Size Amount of data to be sent" to "Size Amount of data to be sent in units of hsai->Init.DataSize" or similar.

This particular issue has been resolved.

Original reply follows:

I think I found the issue. I have been debugging numerous issues for days now so a second set of eyes to confirm my findings would be much appreciated. It appears there is a bug in stm32l4xx_hal_sai.c, HAL_SAI_Transmit(), where the counter for the number of bytes transmitted is not updated based on the data size. Ie, the number of bytes transmitted is always decremented by 1 byte even if your data size is 16 bits (in my case) or 32 bits. See below code snippet line 61:

HAL_StatusTypeDef HAL_SAI_Transmit(SAI_HandleTypeDef *hsai, uint8_t *pData, uint16_t Size, uint32_t Timeout)
{
  uint32_t tickstart = HAL_GetTick();
  uint32_t temp;
 
  if ((pData == NULL) || (Size == 0U))
  {
    return  HAL_ERROR;
  }
 
  if (hsai->State == HAL_SAI_STATE_READY)
  {
    /* Process Locked */
    __HAL_LOCK(hsai);
 
    hsai->XferSize = Size;
    hsai->XferCount = Size;
    hsai->pBuffPtr = pData;
    hsai->State = HAL_SAI_STATE_BUSY_TX;
    hsai->ErrorCode = HAL_SAI_ERROR_NONE;
 
    /* Check if the SAI is already enabled */
    if ((hsai->Instance->CR1 & SAI_xCR1_SAIEN) == 0U)
    {
      /* fill the fifo with data before to enabled the SAI */
      SAI_FillFifo(hsai);
      /* Enable SAI peripheral */
      __HAL_SAI_ENABLE(hsai);
    }
 
    while (hsai->XferCount > 0U)
    {
      /* Write data if the FIFO is not full */
      if ((hsai->Instance->SR & SAI_xSR_FLVL) != SAI_FIFOSTATUS_FULL)
      {
        if ((hsai->Init.DataSize == SAI_DATASIZE_8) && (hsai->Init.CompandingMode == SAI_NOCOMPANDING))
        {
          hsai->Instance->DR = *hsai->pBuffPtr;
          hsai->pBuffPtr++;
        }
        else if (hsai->Init.DataSize <= SAI_DATASIZE_16)
        {
          temp = (uint32_t)(*hsai->pBuffPtr);
          hsai->pBuffPtr++;
          temp |= ((uint32_t)(*hsai->pBuffPtr) << 8);
          hsai->pBuffPtr++;
          hsai->Instance->DR = temp;
        }
        else
        {
          temp = (uint32_t)(*hsai->pBuffPtr);
          hsai->pBuffPtr++;
          temp |= ((uint32_t)(*hsai->pBuffPtr) << 8);
          hsai->pBuffPtr++;
          temp |= ((uint32_t)(*hsai->pBuffPtr) << 16);
          hsai->pBuffPtr++;
          temp |= ((uint32_t)(*hsai->pBuffPtr) << 24);
          hsai->pBuffPtr++;
          hsai->Instance->DR = temp;
        }
        hsai->XferCount--;       // <------ Issue is here
      }
      else
      {
        /* Check for the Timeout */
        if ((((HAL_GetTick() - tickstart) > Timeout) || (Timeout == 0U)) && (Timeout != HAL_MAX_DELAY))
        {
          /* Update error code */
          hsai->ErrorCode |= HAL_SAI_ERROR_TIMEOUT;
 
          /* Clear all the flags */
          hsai->Instance->CLRFR = 0xFFFFFFFFU;
 
          /* Disable SAI peripheral */
          /* No need to check return value because state update, unlock and error return will be performed later */
          (void) SAI_Disable(hsai);
 
          /* Flush the fifo */
          SET_BIT(hsai->Instance->CR2, SAI_xCR2_FFLUSH);
 
          /* Change the SAI state */
          hsai->State = HAL_SAI_STATE_READY;
 
          /* Process Unlocked */
          __HAL_UNLOCK(hsai);
 
          return HAL_ERROR;
        }
      }
    }
 
    hsai->State = HAL_SAI_STATE_READY;
 
    /* Process Unlocked */
    __HAL_UNLOCK(hsai);
 
    return HAL_OK;
  }
  else
  {
    return HAL_BUSY;
  }
}

I believe the code should be updated to the following:

hsai->XferCount--; // For SAI_DATASIZE_8

hsai->XferCount-=2; // For SAI_DATASIZE_16

hsai->XferCount-=4; // For SAI_DATASIZE_32

etc...

Otherwise for data size >= SAI_DATASIZE_10, HAL_SAI_Transmit() will index beyond the end of the buffer and output random data in RAM. I believe this affects HAL_SAI_Transmit_IT() as well since it calls SAI_Transmit_IT16Bit() and SAI_Transmit_IT32Bit() which both of these functions only decrement the byte counter by 1 byte (hsai->XferCount--).

The only reason I could see this not being a bug is that if the "Size" parameter in HAL_SAI_Transmit() represents the number of data elements (multiplies of the data size) instead of the number of bytes in the buffer.

Can someone from ST review this to determine if this is a bug or lack of documentation and make note to fix for future SDK releases / clarify documentation?

Edit: If this is a bug it appears to exist in the latest STM32L4 SDK as of 11/3/21 (STM32Cube_FW_L4_V1.17.0)

Thanks,

Derek

[Piranha]

        else if (hsai->Init.DataSize <= SAI_DATASIZE_16)
        {
          temp = (uint32_t)(*hsai->pBuffPtr);
          hsai->pBuffPtr++;
          temp |= ((uint32_t)(*hsai->pBuffPtr) << 8);
          hsai->pBuffPtr++;
          hsai->Instance->DR = temp;
        }
        else
        {
          temp = (uint32_t)(*hsai->pBuffPtr);
          hsai->pBuffPtr++;
          temp |= ((uint32_t)(*hsai->pBuffPtr) << 8);
          hsai->pBuffPtr++;
          temp |= ((uint32_t)(*hsai->pBuffPtr) << 16);
          hsai->pBuffPtr++;
          temp |= ((uint32_t)(*hsai->pBuffPtr) << 24);
          hsai->pBuffPtr++;
          hsai->Instance->DR = temp;
        }

Do the HAL developers know that Cortex-M3 and higher support unaligned memory accesses?

[DerekR]

At first glance, it appears this code is just byte swapping the values based on the data size.