Modify of HAL library?

[Jang Wonyong]

Posted on April 09, 2018 at 08:33

HI,

I use SPI of HAL Library in STM32F0.

However, due to unaligned memory access, HardFault occurs in the HAL_SPI_TransmitReceive () function.So I modified the HAL library as follows.

uint8_t data_lo, data_hi;

uint16_t data16;

//hspi->Instance->DR = *((uint16_t*)hspi->pTxBuffPtr); <-original

data_lo = *hspi->pTxBuffPtr;

data_hi = *(hspi->pTxBuffPtr + 1);data16 = (uint16_t)data_lo | ((uint16_t)data_hi << 8); hspi->Instance->DR = data16;

//*((uint16_t*)hspi->pRxBuffPtr) = hspi->Instance->DR; 

<-

original

data16 = hspi->Instance->DR;

*hspi->pRxBuffPtr = (uint8_t)data16; *(hspi->pRxBuffPtr + 1) = (uint8_t)(data16 >> 8);

However, if the HAL library is updated, it needs to be modified again. How can I use it without modification?

Translated into Google Translator.

[Greg Fundyler]

Posted on April 09, 2018 at 16:16

What you did made everything work, but it did not solve the underlying problem. You have your SPI peripheral (and HAL) configured for 16-bit SPI data. The right way to resolve this issue is to A) pass an even address to 

HAL_SPI_TransmitReceive (i.e. 2-byte-aligned address), or B) configure for 8-bit data.

Switching to 8-bit data mode may swap the order of the bytes transferred, so keep that in mind. Look at the Data Size option in the SPI parameter settings in STM32CubeMX, if you are using that tool. Or make the init code look something like this: hspi1.Init.DataSize = SPI_DATASIZE_8BIT;

And definitely revert the change you made to the HAL library.

[Tesla DeLorean]

Posted on April 09, 2018 at 16:24

This defect in the HAL has been reported multiple times, the CM0 can't tolerate this level of sloppiness in the software. ST needs to get some rigour in their test and release program.

[Greg Fundyler]

Posted on April 09, 2018 at 17:41

Why is this a defect? If you configure for 8-bit operation, then it will not perform unaligned memory operations. If you configure for 16-bit operation, you better be sure to provide an aligned address. Am I missing something? Is that not how it actually behaves?

[Tesla DeLorean]

Posted on April 09, 2018 at 18:47

hspi->Instance->DR = *((uint16_t*)hspi->pTxBuffPtr); <-original

This code where a pointer of no guaranteed alignment is randomly cast as a 16-bit pointer. The CM0 does not permit unaligned read/write, ie spanning multiple word boundaries. Works fine on CM3 and CM4 builds.

>>If you configure for 16-bit operation, you better be sure to provide an aligned address.

You can't always guarantee that, and the compiler has no way of flagging or tracking it across arbitrary casts. When unpacking data from files or byte streams there is frequently not any alignment. On all CMx parts this is a hazard for pulling 64-bit int/doubles using LDRD/STRD from byte packed structures. ie double foo = *((double*)hspi->pTxBuffPtr); 

A lot of people use to x86 compilers don't really understand the ramifications of memory alignment, so there is plenty of code that isn't portable to ARM7, ARM9 and CM cores without modification. The technique there is to memcpy() the variable(s) to a buffer with suitable alignment.

The hazard is that code that runs fine for 99.9999% of cases will Hard Fault on the 0.0001% of cases where the data stream adjusts in a way you didn't handle properly, or the compiler packed a variable/structure that looked to be a safe optimization, or was directed via ♯ pragma pack(1). This is a particular problem if the physical implementation is several layers abstracted from Joe Random Coder whose depth of knowledge extends to cut-n-pasting code he found on the internets.

[Greg Fundyler]

Posted on April 09, 2018 at 19:21

Fair enough. What would the correct solution be then? You could keep the OP's modification, with a speed penalty. But that wouldn't work for HAL_SPI_TransmitReceive_DMA. You could introduce another set of functions with uint16_t* parameters that would avoid typecasting within the HAL code and explicitly place the responsibility of memory alignment on the user. What would you suggest?

[waclawek.jan]

Posted on April 09, 2018 at 19:45

While modifying/improving the library or even discussing it is a worthy topic, the OP does not request that, but he asks, how to prevent the Cube/CubeMX contraption to overwrite his modifications.

(I don't know, I don't Cube).

JW

[Andrew Neil]

Posted on April 09, 2018 at 21:32

waclawek.jan wrote:

the OP ... asks, how to prevent the Cube/CubeMX contraption to overwrite his modifications.

Which is also a frequently-recurring question. 

I also don't Cube, so don't know if it has such a facility - but I don't recall seeing one posted in response to such questions.

The obvious solution would seem to be to always take a copy of your project before doing any updates in Cube.

A change-tracking system - eg, SVN or Git - would help with that ...

[Andrew Neil]

Posted on April 09, 2018 at 21:40

Turvey.Clive.002

wrote:

This defect in the HAL has been reported multiple times, the CM0 can't tolerate this level of sloppiness in the software.

Turvey.Clive.002

further

wrote:

hspi->Instance->DR = *((uint16_t*)hspi->pTxBuffPtr); <-original

This code where a pointer of no guaranteed alignment is randomly cast as a 16-bit pointer.

Hence the compiler warns about 'type punning' ?

https://community.st.com/0D50X00009XkY9USAV

https://community.st.com/0D50X00009XkXPESA3

https://community.st.com/0D50X00009XkWkESAV

https://community.st.com/0D50X00009XkdjRSAR

etc, etc, ...

[Ben K]

Posted on April 10, 2018 at 00:35

You should modify the source code in the Cube's repository location, as this file is copied during the generation of every project. Hopefully ST will move the code base to git soon, as having to download and store pretty much the same middlewares and examples for each line is completely unnecessary.