Why are HAL Lookup tables located in RAM?

[richardst9]

Posted on June 14, 2016 at 11:10

I notice that the HAL locates some lookup tables in RAM. For example, in stm32f1xx_hal_rcc.c:

uint32_t HAL_RCC_GetSysClockFreq(void)

{ &sharpif   defined(RCC_CFGR2_PREDIV1SRC)   const uint8_t aPLLMULFactorTable[14] = {0, 0, 4,  5,  6,  7,  8,  9, 0, 0, 0, 0, 0, 13};   const uint8_t aPredivFactorTable[16] = { 1, 2,  3,  4,  5,  6,  7,  8, 9,10, 11, 12, 13, 14, 15, 16}; &sharpelse   const uint8_t aPLLMULFactorTable[16] = { 2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15, 16, 16}; &sharpif defined(RCC_CFGR2_PREDIV1)   const uint8_t aPredivFactorTable[16] = { 1, 2,  3,  4,  5,  6,  7,  8, 9,10, 11, 12, 13, 14, 15, 16}; &sharpelse   const uint8_t aPredivFactorTable[2] = { 1, 2}; &sharpendif /*RCC_CFGR2_PREDIV1*/ [note that this code extract includes a fix for another HAL bug.] There are lots of reasons locating lookup tables in RAM is a bad thing (e.g. performance, memory, robustness). Is there some reason why it was necessary in this case? If not, please could you remedy this in the next release of the Hal Libraries? One way is to add the ''static'' keyword. uint32_t HAL_RCC_GetSysClockFreq(void) { &sharpif   defined(RCC_CFGR2_PREDIV1SRC)   static const uint8_t aPLLMULFactorTable[14] = {0, 0, 4,  5,  6,  7,  8,  9, 0, 0, 0, 0, 0, 13};   static const uint8_t aPredivFactorTable[16] = { 1, 2,  3,  4,  5,  6,  7,  8, 9,10, 11, 12, 13, 14, 15, 16}; &sharpelse   static const uint8_t aPLLMULFactorTable[16] = { 2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15, 16, 16}; &sharpif defined(RCC_CFGR2_PREDIV1)   static const uint8_t aPredivFactorTable[16] = { 1, 2,  3,  4,  5,  6,  7,  8, 9,10, 11, 12, 13, 14, 15, 16}; &sharpelse   static const uint8_t aPredivFactorTable[2] = { 1, 2}; &sharpendif /*RCC_CFGR2_PREDIV1*/ Thanks, Richard #hal

[Tesla DeLorean]

Posted on June 15, 2016 at 01:53

I think it is just poor coding style, and lack of familiarity, the SPL is worse, they use a global that is volatile, and is only used in one function.

uint32_t SystemCoreClock = 168000000;
__I uint8_t AHBPrescTable[16] = {0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3, 4, 6, 7, 8, 9};

I usually move this inside the one function it is used in as a ''static const'', this way it is not copied to RAM or stack repeatedly, and doesn't pollute the name-space.

[richardst9]

Posted on June 15, 2016 at 09:43

agreed.

I feel that we are ''encouraged'' to base our code on ST libraries, but some of them don't meet our internal coding standards. I don't have a problem with that - none of us are perfect and the HAL code is still rather immature - but it leaves us in a bit of a quandary. Do we - waive some of the rules for these libraries - rewrite the sections of the HAL libraries that need improving - replace the sections of the HAL libraries that need improving My opinion is that the best course of action is to make ST aware of these issues and then hopefully they can work on improving their libraries. Then we all benefit. As you'll know, my biggest issues with the HAL libraries are not so much the ''immaturity'' as the lack of vital functionality and that its design didn't properly consider  existing codebases based on the SPL, but I risk becoming boring on the subject.

[AvaTar]

Posted on June 15, 2016 at 10:38

> My opinion is that the best course of action is to make ST aware of these issues and then hopefully they can work on improving their libraries. Then we all benefit.

I think ST is well aware of this issue. However, their idea of an ''all-embracing software, for free'' is not playing out as planned, and (IMHO) will not in the near future. Their hardware/silicon developers continue to produce new silicon considerably faster than their software department can support it, or even fix bugs. I think ST would be better off  leaving the software business to specialists. The original problem of this post is an indicator that ST employed mostly PC software developers, where no architectural difference between code and data space (i.e. Flash vs. RAM) exists.

[richardst9]

Posted on June 15, 2016 at 12:15

>__I uint8_t AHBPrescTable[16]

I believe that this is fixed in the latest STM32Cube_FW_F1_V1.4.0 HAL release, but I don't know if this fix has made it into the Standard Peripheral libraries yet. If not, then that might reveal something about ST intentions for the SPL.