missing volatile type for _IT functions

[atsju]

Hello, this is quite a broad question about code quality.

Shouldn't the buffers used in _IT functions (like HAL_SPI_TransmitReceive_IT) be declared volatile ? Example here : https://github.com/STMicroelectronics/STM32CubeL4/blob/d023c0d560ace11509f9b761c8913a9e48fcf194/Projects/NUCLEO-L496ZG/Examples/SPI/SPI_FullDuplex_ComIT/Src/main.c#L51 Because technically the buffer are modified during HAL_SPI_IRQHandler which is called from an interrupt handler.

I have seen the casts to __IO in functions like SPI_TxISR_8BIT and I never experienced any issue so I suppose it is "working" in most cases but I'm worried this is not completely C compliant and relies on compiler implementation. Do you have an analysis of this coding practice ?

[ASELSTM]

Hi @atsju,

Thank you for your contribution.

Regarding the first question :

Shouldn't the buffers used in _IT functions (like HAL_SPI_TransmitReceive_IT) be declared volatile ?

The buffers are addressed by reference (pointer) and can neither be changed spuriously nor change address on the fly. Moreover, the buffers are coming from upper layers, it can be from middleware or user applicative code, so they cannot be forced to volatile.

Concerning the second question :

Do you have an analysis of this coding practice ?

Our code is actually based on CMSIS layer and the __IO is properly defined there as volatile for all supported compilers and cores.

https://github.com/STMicroelectronics/STM32CubeL4/blob/5e1553e07706491bd11f4edd304e093b6e4b83a4/Drivers/CMSIS/Core/Include/core_cm4.h#L222

I hope you find this answer helpful.

With regards,

[atsju]

Hi @ALABSTM

The buffers are addressed by reference (pointer) and can neither be changed spuriously nor change address on the fly.

I agree the pointers are not volatile but the buffer pointed IS volatile. It IS accessed spuriously in interrupt. I agree you use a pointer to access the buffer but as the pointer is not declared "to volatile" you cannot guarantee something will not be optimized away.

Moreover, the buffers are coming from upper layers, it can be from middleware or user applicative code, so they cannot be forced to volatile.

This is what you explained in "const correctness" tickets also ;) (https://github.com/STMicroelectronics/STM32CubeF4/issues/10) The fact that it is difficult to correct doesn't make the original code correct anyway. As a user I'm would not be chocked to need to declare a buffer volatile when I use function HAL_SPI_TransmitReceive_IT because I know it relies on ISR.

Our code is actually based on CMSIS layer and the __IO is properly defined there as volatile for all supported compilers and cores.

Even with __IO properly defined, can you guarantee that casting the volatile only on some points is ok ?

If you can give me a clear explication with references to C standard about how casting to volatile will work for every access from ISR and from outside then it's OK for me. But I honestly doubt it is possible.

Best regards,

[ASELSTM]

Hi @atsju,

I agree the pointers are not volatile but the buffer pointed IS volatile. It IS accessed spuriously in interrupt. I agree you use a pointer to access the buffer but as the pointer is not declared "to volatile" you cannot guarantee something will not be optimized away.

As a user I'm would not be chocked to need to declare a buffer volatile when I use function HAL_SPI_TransmitReceive_IT because I know it relies on ISR.

Actually, it’s a matter of design. There is no need to protect the user part from optimization of the compiler if the design is well done. The volatile are mostly reserved for registers managed by the driver layer and sometimes for global variables shared between different threads.

Regarding the second question:

If you can give me a clear explication with references to C standard about how casting to volatile will work for every access from ISR and from outside then it's OK for me. But I honestly doubt it is possible.

There is actually no reference to C standards as it is rather a matter of design than C standard rules.

Please allow me to close this issue, then. Thank you for your comprehension and thank you again for this report.