Using structs to centralize the hardware abstraction layers make the code execution slow

126 views Asked by At

I'm trying to use the structure for centralizing my hardware configuration. However, this makes my codes slow later while the struct has been defined as a global variable in the RAM.

For example, I have defined.

typedef struct
{
    PeriphBus       TIM_Bus;
    uint32_t        TIM_Clk;
    uint16_t        TIM_Prescaler;
    uint32_t        TIM_CounterMode;
    uint32_t        TIM_Autoreload;
    uint32_t        TIM_ClockDivision;
    uint32_t        TIM_RepetitionCounter;
    TIM_TypeDef     *TIM_Peripheral;
    IRQn_Type       TIM_Interrupt;
    uint32_t        TIM_IPeriority;
} TIMHandler;

TIMHandler  TIMCCS = {
    .TIM_Bus                        = APB1,
    .TIM_Clk                        = LL_APB1_GRP1_PERIPH_TIM2,
    .TIM_Prescaler                  = (10000 - 1),
    .TIM_CounterMode                = LL_TIM_COUNTERMODE_UP,
    .TIM_Autoreload                 = (1000 - 1),
    .TIM_ClockDivision              = LL_TIM_CLOCKDIVISION_DIV1,
    .TIM_RepetitionCounter          = 0,
    .TIM_Peripheral                 = TIM2,
    .TIM_Interrupt                  = TIM2_IRQn,
    .TIM_IPeriority                 = 2,
};

And later in the code, I'm trying to reset the interrupt flag with this code.

void TIMCCS_IRQHandler (void)
{
    ... // some codes here are deleted to keep it simpler.
    LL_TIM_ClearFlag_UPDATE (TIMCCS->TIM_Peripheral);
}

Unfortunately, this last function to reset the interrupt flag is prolonged, while if I replace it with

LL_TIM_ClearFlag_UPDATE (TIM2);

It gets back to normal.

I'm wondering where I'm making a mistake. I'm using ARM GCC as the compiler for STM32F7 microcontrollers.

2

There are 2 answers

3
Michael Becker On BEST ANSWER

My first bit of advice is to review the assembly code produced between the two versions of code. Is it doing what you think it is?

Next question, do you have the optimizer turned on?

To me, at first glance it makes sense that one runs slower than the other. If you look at your first version, the ISR needs to load the address of the global struct. Then it needs to dereference that address to get the value stored in the TIM_Peripheral variable. Then it passes that value to the ClearFlag function. Whereas the second version just directly passes that value in, skipping all the memory accesses. But again, the assembly code is your friend.

1
Craig Estey On

This is prefaced by the top comments ...

I think it's not a type issue, it's how the memory mapping and addressing works.

No, it is a type issue. If the type passed to the function had been correct, the issue would have been resolved. Read on for the solution ...

Precisely the "LL_TIM_ClearFlag_UPDATE(&TIMCCS->TIM_Peripheral)" works but with an excessive delay which is not acceptable to my case.

Then, in other words, it does not work. See below for the full analysis why.

I mean "LL_TIM_ClearFlag_UPDATE(TIM2)" is faster and I was expecting to find a way to generate the same assembly code for both cases.

That's because this code is correct. Your code using TIMCCS is not equivalent [and is wrong].

Here is the function definition:

/**
  * @brief  Clear the update interrupt flag (UIF).
  * @rmtoll SR           UIF           LL_TIM_ClearFlag_UPDATE
  * @param  TIMx Timer instance
  * @retval None
  */
__STATIC_INLINE void LL_TIM_ClearFlag_UPDATE(TIM_TypeDef * TIMx)
{
    WRITE_REG(TIMx->SR, ~(TIM_SR_UIF));
}

The argument must be a pointer to a TIM_TypeDef struct. Obviously, TIM2 is such a pointer [because it is compiling and is working].

Within that struct, the SR element is a uint16_t that is a port address. The code clears the TIM_SR_UIF bit in the register that SR designates.

When you pass in as the argument: &TIMCCS->TIM_Peripheral you are not passing a TIM_TypeDef * pointer.

You are passing the address of your TIMCSS struct itself and not what TIM_Peripheral points to.

So, you're passing a garbage pointer value to the function. So, the function will index past the end of your struct and pick up whatever 16 bit value happens to be at the offset for SR.

This has two [bad] effects:

(1) The function will clear a bit at some random location in the register port space. This could cause some serious damage, depending on what port address is actually used. Since the processor didn't croak, it has some subtler effect.

(2) Because the port address is wrong, it will not actually clear the interrupt flag in the correct interrupt controller port!

So, when you return from the ISR, the interrupt (having not been cleared), will immediately reinterrupt the processor.

You will [again] enter the ISR and [again] not clear the correct interrupt bit. And, when the ISR returns, the processor will be immediately reinterrupted.

This cycle will continue to repeat.

So, you are being hammered with [infinitely] many spurious interrupts!

The base level code [that is desperately trying to make progress] might get a single instruction executed before the interrupt occurs again. So, it will make some [grindingly slow] progress.

So, for every base level instruction that executes, there are [probably] 10 [or more] instructions that need to be executed to enter/exit the ISR.

So, the result is that the process will seem slow (10x to 100x slow).

Once again, that's because your call to the function is incorrect.

As I said, what you want to pass to the function is the contents of the TIM_Peripheral member of your TIMCSS struct. Remember this important point: You set the value of TIM2 into that member as an initializer. And, we know that TIM2 is the correct value.

To use your struct instance correctly, you want:

LL_TIM_ClearFlag_UPDATE(TIMCCS.TIM_Peripheral);

Side notes:

This code snippet should seem familiar because I gave it in my top comments, based on what made correct c code.

Instead of answering the questions I posed [which would have helped you resolve the issue], you chose to "fixate" on your analysis, which I mentioned in the top comments had to be wrong [and gave the reason why].

In fact, my very first comment gave the correct solution.