cancel
Showing results for 
Search instead for 
Did you mean: 

HAL USART serial buffer pointers not declared volatile?

Johi
Senior III

One of the practices of software design is to use volatile buffers when ISR and main loop access the data in the same buffers. This makes sure certain compiler register optimisations do not create unreliable code.

If I look at the HAL code for the USART for example: 

 

 

/**
  * @brief TX interrupt handler for 7 or 8 bits data word length .
  * @note   Function is called under interruption only, once
  *         interruptions have been enabled by HAL_UART_Transmit_IT().
  *  huart UART handle.
  * @retval None
  */
static void UART_TxISR_8BIT(UART_HandleTypeDef *huart)
{
  /* Check that a Tx process is ongoing */
  if (huart->gState == HAL_UART_STATE_BUSY_TX)
  {
    if (huart->TxXferCount == 0U)
    {
      /* Disable the UART Transmit Data Register Empty Interrupt */
      ATOMIC_CLEAR_BIT(huart->Instance->CR1, USART_CR1_TXEIE);

      /* Enable the UART Transmit Complete Interrupt */
      ATOMIC_SET_BIT(huart->Instance->CR1, USART_CR1_TCIE);
    }
    else
    {
      huart->Instance->TDR = (uint8_t)(*huart->pTxBuffPtr & (uint8_t)0xFF);
      huart->pTxBuffPtr++;
      huart->TxXferCount--;
    }
  }
}

 

 

I observe that the *huart->pTxBufferPtr is not declared volatile.

Nor is the type of const uint8_t pData in the signature of HAL_UART_Transmit_IT:

 

 

HAL_StatusTypeDef HAL_UART_Transmit_IT(UART_HandleTypeDef *huart, const uint8_t *pData, uint16_t Size);

 

 

Hence my question: how should I relate the concept of volatile to the HAL code sample above? 

One could imagine (maybe too far - fetched): fill the buffer, call HAL_UART_Transmit_IT and later on the buffer is written due to some register optimisation? 

   

 

1 ACCEPTED SOLUTION

Accepted Solutions
Pavel A.
Evangelist III

This is complicated. Short answer - 'volatile' should not be (mis)used to synchronize access to data in normal memory among several software threads, memory barriers and 'atomic' types exist for that. Purpose of volatile is more limited to "hardware registers", in case of Cortex-M this is memory of type "device" or "strongly ordered". The HAL "handle" structures and data buffers sit in 100% normal memory. Except of cached buffers.... which is yet another story. 

Aliasing (when something is accessed thru a pointer and directly or thru another pointer, with possibly different attributes) is yet another story.

Bottom line of it - in absence of either hint (volatile or memory barrier) the compiler tends to optimize more aggressively, which can result in wrong behavior. So it's better to provide it at least the volatile hint.

But compilers usually do not try to optimize aggressively across boundaries of compilation units  (files) exactly because there's too much code like that. If it breaks users will come to the compiler folks with fire and pitchforks ))

Rewriting everything nicely using atomic.h types or std::atomic is too much impact. Also, some compilers are better and smarter that others (better detect concurrency, interrupt handlers). Users of these compilers just happily live with legacy code without worry. TL;DR. 

Happy new year!

 

View solution in original post

3 REPLIES 3
gbm
Lead II

volatile specifier in this case should be used only if an object is used for communication between two code fragments executed at different processor priorities (written by ISR, read by thread). I don't use HAL but I assume this pointer is not examined by the HAL routine called from a thread. It that's the case there is no need for it being declared as volatile.

AScha.3
Chief

The "volatile" is needed, if in INT some variable is changed - but the "main" process doesn't know about it.

But here , in transmit-this-buffer , you write some data to a buffer and give it (by pointer) to the HAL_UART_Transmit_IT , but there is nothing changed in this data, what is later used in "main".

And if you use receive in INT, obviously its up to you, to set the used variables or arrays as volatile , that you give to the HAL call :

HAL_UART_Receive_IT(&huart7, &ESP_RX[ESP_RX_COUNT], 1);
If you feel a post has answered your question, please click "Accept as Solution".
Pavel A.
Evangelist III

This is complicated. Short answer - 'volatile' should not be (mis)used to synchronize access to data in normal memory among several software threads, memory barriers and 'atomic' types exist for that. Purpose of volatile is more limited to "hardware registers", in case of Cortex-M this is memory of type "device" or "strongly ordered". The HAL "handle" structures and data buffers sit in 100% normal memory. Except of cached buffers.... which is yet another story. 

Aliasing (when something is accessed thru a pointer and directly or thru another pointer, with possibly different attributes) is yet another story.

Bottom line of it - in absence of either hint (volatile or memory barrier) the compiler tends to optimize more aggressively, which can result in wrong behavior. So it's better to provide it at least the volatile hint.

But compilers usually do not try to optimize aggressively across boundaries of compilation units  (files) exactly because there's too much code like that. If it breaks users will come to the compiler folks with fire and pitchforks ))

Rewriting everything nicely using atomic.h types or std::atomic is too much impact. Also, some compilers are better and smarter that others (better detect concurrency, interrupt handlers). Users of these compilers just happily live with legacy code without worry. TL;DR. 

Happy new year!