2023-11-21 05:34 PM
Controller: STM32G431VBT6
Compiler: arm-none-eabi-gcc version 12.2.1
Libraries: CMSIS
I use STM32G431VBT6 for a sensorless BLDC motor control with zero-cross detection.
TIM1 is used for PWM outputs. See below part of the code.
RCC->APB2ENR |= RCC_APB2ENR_TIM1EN; // enable clock
TIM1->PSC = 0; // no prescaling
TIM1->ARR = SYSTEM_CLK / MOTOR_PWM_FREQ / 2; // auto reload register
TIM1->CR1 |= TIM_CR1_CMS_0; // center aligned mode 1, int. at rising edge
TIM1->CR1 |= TIM_CR1_ARPE; // ARR register is buffered
TIM1->CCR1 = TIM1->ARR; // trigger in the mid of PWM off
// channel 2, phase W
TIM1->CCMR1 |= TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2M_2; // PWM mode 1
TIM1->CCMR1 |= TIM_CCMR1_OC2PE; // output compare preload enable
// channel 3, phase V
TIM1->CCMR2 |= TIM_CCMR2_OC3M_1 | TIM_CCMR2_OC3M_2; // PWM mode 1
TIM1->CCMR2 |= TIM_CCMR2_OC3PE; // output compare preload enable
// channel 4, phase U
TIM1->CCMR2 |= TIM_CCMR2_OC4M_1 | TIM_CCMR2_OC4M_2; // PWM mode 1
TIM1->CCMR2 |= TIM_CCMR2_OC4PE; // output compare preload enable
// 1 µs dead time for both edges -- 22R, CSD18510KTT
TIM1->BDTR |= (MOTOR_DT << TIM_BDTR_DTG_Pos) | TIM_BDTR_OSSI | TIM_BDTR_OSSR;
TIM1->CR2 |= TIM_CR2_CCPC; // CCxE, CCxNE and OCxM bits are preloaded
TIM1->EGR |= TIM_EGR_UG;
NVIC_EnableIRQ(TIM1_CC_IRQn);
NVIC_EnableIRQ(TIM1_UP_TIM16_IRQn);
TIM1->CR1 |= TIM_CR1_CEN; // enable timer
Zero-Cross is sampled using internal comparators every PWM cycle within CCR1 interrupt routine. The corresponding interrupt enable flag is set like: TIM1->DIER |= TIM_DIER_CC1IE
The problem:
Sometimes TIM1_DIER_CC1IE gets reset unexpectedly. I checked all the code. There is no software action that does reset TIM1_DIER_CC1IE.
How is it possible that TIM_DIER_CC1IE of TIM1->DIER gets reset without any software action?
Is there any hardware condition (except device reset) that can change TIM1->DIER register?
How to debug?
I've been struggling for days now. Can anybody help? I appreciate every helping comment.
Solved! Go to Solution.
2023-11-27 10:14 PM - edited 2023-11-27 10:19 PM
> I haven't considered that TIM1->DIER |= ... is not an atomic operation. [...] If you know a better solution than mine, please let me know.
Use bit-banding.
#define PERIPH_BB(address, bit) (*(volatile uint32_t *)(PERIPH_BB_BASE + (((uint32_t)(&(address)) - PERIPH_BASE) * 32) + ((bit) * 4)))
PERIPH_BB(TIM1->DIER, TIM_DIER_UIE_Pos) = 1;
Note, that in programming - and twice so in microcontrollers programming - there is no universal "better", and you can't just slap on some solution universally; so let's see if this solution can be used here.
Bit-banding is available only on Cortex-M3 and Cortex-M4 - but 'G4 uses the latter, so you are OK.
Whether TIM1 falls into the *peripheral* bit-banding area on 'G4, I don't know, but I'd be surprised if it wouldn't - I leave that to you to check.
Also, bit-banding is still RMW at the hardware level (the key difference for this issue is, that the hardware RMW is uninterruptible by the processor or any other busmaster) and that may bring problems with registers which are not just "storage-type" but which are changed also by hardware. However, TIMx_DIER is not that case, and so using bit-banding to change individual bits in TIMx_DIER is OK.
JW
2023-11-22 01:18 AM
> Zero-Cross is sampled using internal comparators every PWM cycle within CCR1 interrupt routine. The corresponding interrupt enable flag is set like: TIM1->DIER |= TIM_DIER_CC1IE
Why is it not in the initialization portion shown above?
> Is there any hardware condition (except device reset) that can change TIM1->DIER register?
Timer reset through its reset bit in RCC_APBxRSTRx. Also, through the DMAR/DCR mechanism, but it would need to be set so that the registers range include DIER.
> How to debug?
Use a data breakpoint (in some IDEs called watchpoint) on TIM1_DIER, but if you set it repeatedly, it will be hard to capture an unwanted access.
JW
2023-11-25 10:41 AM - edited 2023-11-25 10:43 AM
Thank you for your answear.
@waclawek.jan wrote:> Zero-Cross is sampled using internal comparators every PWM cycle within CCR1 interrupt routine. The corresponding interrupt enable flag is set like: TIM1->DIER |= TIM_DIER_CC1IE
Why is it not in the initialization portion shown above?
I thought it isn't relevant. Here it is:
// TIM8 -- commutation time and stall detection
RCC->APB2ENR |= RCC_APB2ENR_TIM8EN; // enable clock
TIM8->PSC = 340 - 1; // prescaler 170 MHz / 340 -> one step = 2 µs
TIM8->ARR = 0xFFFF; // auto reload register
// As the preload registers are transferred to the shadow registers only when an update event
// occurs, before starting the counter, all registers must be initialized by setting the UG bit in
// the TIMx_EGR register.
TIM8->EGR |= TIM_EGR_UG;
NVIC_EnableIRQ(TIM8_CC_IRQn); // enable capture compare interrupt
TIM8->CR1 |= TIM_CR1_CEN; // enable counter
// voltage at M_SHUNT_X+
// Vadc = (3.3V/R7 + Vs/R6) / (1/R6 + 1/R7 + 1/R8)
// Vs = 0.005R * Is
// Is_max = 12 A -> Vadc = 0.1629 V -> GAIN = 8 -> Vadc = 1.3029 V
RCC->APB2ENR |= RCC_APB2ENR_SYSCFGEN; // enable SYSCFG clock
// OPAMP1
OPAMP1->CSR |= (0b01010 << OPAMP_CSR_PGGAIN_Pos); // non inverting gain = 8 with VINM0 pin for input or bias
OPAMP1->CSR |= OPAMP_CSR_OPAMPINTEN; // redirect output to ADC
OPAMP1->CSR |= (0b10 << OPAMP_CSR_VMSEL_Pos); // VINM0 pin connected to OPAMP VINM input
OPAMP1->CSR |= (0b00 << OPAMP_CSR_VPSEL_Pos); // VINP0 pin connected to OPAMP VINP input
OPAMP1->CSR |= OPAMP_CSR_OPAMPxEN; // enable OPAMP
// OPAMP2
OPAMP2->CSR |= (0b01010 << OPAMP_CSR_PGGAIN_Pos); // non inverting gain = 8 with VINM0 pin for input or bias
OPAMP2->CSR |= OPAMP_CSR_OPAMPINTEN; // redirect output to ADC
OPAMP2->CSR |= (0b10 << OPAMP_CSR_VMSEL_Pos); // VINM0 pin connected to OPAMP VINM input
OPAMP2->CSR |= (0b00 << OPAMP_CSR_VPSEL_Pos); // VINP0 pin connected to OPAMP VINP input
OPAMP2->CSR |= OPAMP_CSR_OPAMPxEN; // enable OPAMP
// OPAMP3
OPAMP3->CSR |= (0b01010 << OPAMP_CSR_PGGAIN_Pos); // non inverting gain = 8 with VINM0 pin for input or bias
OPAMP3->CSR |= OPAMP_CSR_OPAMPINTEN; // redirect output to ADC
OPAMP3->CSR |= (0b10 << OPAMP_CSR_VMSEL_Pos); // VINM0 pin connected to OPAMP VINM input
OPAMP3->CSR |= (0b00 << OPAMP_CSR_VPSEL_Pos); // VINP0 pin connected to OPAMP VINP input
OPAMP3->CSR |= OPAMP_CSR_OPAMPxEN; // enable OPAMP
// analog comparator COMP1 -- MOTOR_V
COMP1->CSR |= (0b010 << COMP_CSR_HYST_Pos); // hysteresis of 20 mV
COMP1->CSR |= COMP_CSR_INPSEL; // select PB1 as positive input
COMP1->CSR |= (0b110 << COMP_CSR_INMSEL_Pos); // select PA4 as negative input
// analog comparator COMP3 -- MOTOR_W
COMP3->CSR |= (0b010 << COMP_CSR_HYST_Pos); // hysteresis of 20 mV
COMP3->CSR |= COMP_CSR_INPSEL; // select PC1 as positive input
COMP3->CSR |= (0b111 << COMP_CSR_INMSEL_Pos); // select PC0 as negative input
// analog comparator COMP4 -- MOTOR_U
COMP4->CSR |= (0b010 << COMP_CSR_HYST_Pos); // hysteresis of 20 mV
COMP4->CSR |= COMP_CSR_INPSEL; // select PE7 as positive input
COMP4->CSR |= (0b110 << COMP_CSR_INMSEL_Pos); // select PE8 as negative input
// configuration COMP1_2_3 interrupt
RCC->APB2ENR |= RCC_APB2ENR_SYSCFGEN; // enable alternate function IO clock
NVIC_EnableIRQ(COMP1_2_3_IRQn); // enable interrupt
NVIC_EnableIRQ(COMP4_IRQn); // enable interrupt
COMP1->CSR |= COMP_CSR_EN; // enable comparator
COMP3->CSR |= COMP_CSR_EN; // enable comparator
COMP4->CSR |= COMP_CSR_EN; // enable comparator
TIM1->DIER |= TIM_DIER_CC1IE is done during program loop.
TIM1->DIER &= ~TIM_DIER_CC1IE is done during program loop.
TIM1->DIER |= TIM_DIER_UIE is done during program loop.
TIM1->DIER &= ~TIM_DIER_UIE is done during program loop.
@waclawek.jan wrote:> Is there any hardware condition (except device reset) that can change TIM1->DIER register?
Timer reset through its reset bit in RCC_APBxRSTRx. Also, through the DMAR/DCR mechanism, but it would need to be set so that the registers range include DIER.
I don't touch any of those registers.
@waclawek.jan wrote:> How to debug?
Use a data breakpoint (in some IDEs called watchpoint) on TIM1_DIER, but if you set it repeatedly, it will be hard to capture an unwanted access.
In dead, TIM1->DIER is written several times during program loop.
So for debugging I set TIM1->DIER to a constant value: TIM1->DIER = TIM_DIER_CC1IE | TIM_DIER_UIE;
And I used boolean flags instead: f_TIM1_DIER_CC1IE_enable and f_TIM1_DIER_UIE_enable.
So I could set a watchpoint on TIM1->DIER. But in this configuration the error doesn't occur any more. The error only occurs if I write to TIM1->DIER during program loop.
For further debugging I changed the software back to normal mode.
TIM1_DIER_CC1IE and TIM1_DIER_UIE are enabled an disabled during program loop.
To figure out where TIM1_DIER_CC1IE changes unexpectedly, I introduced a boolean variable f_TIM1_DIER_CC1IE as a copy of TIM1_DIER_CC1IE. Every time I change TIM1_DIER_CC1IE, I also change f_TIM1_DIER_CC1IE. So register flag TIM1_DIER_CC1IE and software variable f_TIM1_DIER_CC1IE must always contain the same value.
volatile bool f_TIM1_DIER_CC1IE = false;
volatile uint16_t motor_fail = 0;
// everytime I enable TIM1_DIER_CC1IE, I also set the flag f_TIM1_DIER_CC1IE = true
TIM1->DIER |= TIM_DIER_CC1IE;
f_TIM1_DIER_CC1IE = true;
// everytime I disable TIM1_DIER_CC1IE, I also reset the flag f_TIM1_DIER_CC1IE = false
f_TIM1_DIER_CC1IE = false;
TIM1->DIER &= ~TIM_DIER_CC1IE;
// during programm loop I check at several points if software flag f_TIM1_DIER_CC1IE and register flag TIM1_DIER_CC1IE differ
__disable_irq();
if (f_TIM1_DIER_CC1IE && ((TIM1->DIER & TIM_DIER_CC1IE) == 0) && (motor_fail == 0)) { motor_fail = 650; }
__enable_irq();
TIM1->DIER |= TIM_DIER_UIE;
__disable_irq();
if (f_TIM1_DIER_CC1IE && ((TIM1->DIER & TIM_DIER_CC1IE) == 0) && (motor_fail == 0)) { motor_fail = 651; }
__enable_irq();
Result: motor_fail = 651
I figured out that at one point in my software, sometimes then I set TIM1->DIER |= TIM_DIER_UIE, at the same time TIM1_DIER_CC1IE changes. How is this going on? Changing a bit within a register shouldn't affect the other bits.
Could this be a software problem? A compiler problem? A hardware problem?
2023-11-25 11:16 PM
Do you change TIM1->DIER in some interrupt?
Do you use RTOS?
JW
2023-11-26 05:14 PM
As Jan already noted, most likely you are changing that register in an interrupt or RTOS thread with a code, which is not interrupt/thread safe.
Also take a note that every register access ends up in a separate load/store instruction. And each RMW operation ends up in a read-modify-write sequence of 3 instructions. It's much more efficient to write a code, for example, like this:
// OPAMP1
OPAMP1->CSR =
(0b01010 << OPAMP_CSR_PGGAIN_Pos) | // non inverting gain = 8 with VINM0 pin for input or bias
OPAMP_CSR_OPAMPINTEN | // redirect output to ADC
(0b10 << OPAMP_CSR_VMSEL_Pos) | // VINM0 pin connected to OPAMP VINM input
(0b00 << OPAMP_CSR_VPSEL_Pos) | // VINP0 pin connected to OPAMP VINP input
OPAMP_CSR_OPAMPxEN; // enable OPAMP
2023-11-27 02:59 PM
Thank you for the hint. Indeed I change TIM1->DIER within some interrupt.
void XXX_IRQHandler(void)
{
...
TIM1->DIER |= TIM_DIER_CC1IE;
...
}
// main loop
...
TIM1->DIER |= TIM1_DIER_UIE;
...
And now I understand what's happening. I haven't considered that TIM1->DIER |= ... is not an atomic operation.
My new solution is as follows:
__disable_irq();
TIM1->DIER |= TIM_DIER_UIE;
__enable_irq();
The solution of Piranha works if you want to define the whole register. But as I want to change only one bit and keep the other bits (RMW), this doesn't work for me. If you know a better solution than mine, pleas let me know. Thank you.
2023-11-27 10:14 PM - edited 2023-11-27 10:19 PM
> I haven't considered that TIM1->DIER |= ... is not an atomic operation. [...] If you know a better solution than mine, please let me know.
Use bit-banding.
#define PERIPH_BB(address, bit) (*(volatile uint32_t *)(PERIPH_BB_BASE + (((uint32_t)(&(address)) - PERIPH_BASE) * 32) + ((bit) * 4)))
PERIPH_BB(TIM1->DIER, TIM_DIER_UIE_Pos) = 1;
Note, that in programming - and twice so in microcontrollers programming - there is no universal "better", and you can't just slap on some solution universally; so let's see if this solution can be used here.
Bit-banding is available only on Cortex-M3 and Cortex-M4 - but 'G4 uses the latter, so you are OK.
Whether TIM1 falls into the *peripheral* bit-banding area on 'G4, I don't know, but I'd be surprised if it wouldn't - I leave that to you to check.
Also, bit-banding is still RMW at the hardware level (the key difference for this issue is, that the hardware RMW is uninterruptible by the processor or any other busmaster) and that may bring problems with registers which are not just "storage-type" but which are changed also by hardware. However, TIMx_DIER is not that case, and so using bit-banding to change individual bits in TIMx_DIER is OK.
JW
2023-11-28 09:55 AM
Thank you. This is what I was looking for.
2023-11-28 04:32 PM
Always enabling the interrupts unconditionally makes that code impossible to use in nested function calls. Better do it like this:
uint32_t rPRIMASK = __get_PRIMASK();
__disable_irq();
TIM1->DIER |= TIM_DIER_UIE;
__set_PRIMASK(rPRIMASK);
This version restores the previous state and therefore can be used anywhere.
About my previous example - one can still use "=", "|=" or other assignment operator with that code. The main point there was not to do the assignment to the same register several times in a row. It's even better to introduce a temporary variable for different manipulations and then just do a single assignment to a register.