0
\$\begingroup\$ 
#include "stm32f4xx.h" #define SR_CC1IF (1U<<1) #define SR_CC2IF (1U<<2) #define SR_CC3IF (1U<<3) #define SR_CC4IF (1U<<4) int reg_a; int reg_b; int reg_c; int reg_d; int x; int main(void) { tim1_all_channels_input_capture(); while(1) { while( (TIM1->SR & SR_CC1IF)==1 && (TIM1->SR & SR_CC2IF)==1 && (TIM1->SR & SR_CC3IF)==1 && (TIM1->SR & SR_CC4IF)==1 ) { x=x+1; reg_a =TIM1->CCR1; reg_b =TIM1->CCR2; reg_c =TIM1->CCR3; reg_d =TIM1->CCR4; } } }

Above is my code. I am doing input capture on TIM1 ch1,2,3,4 (all channels). Second while loop checks if an input capture occurred or not by checking the Status register(SR) bits 1,2,3,4 respectively for ch1,ch2,ch3, and ch4. If 1 that means capture occurred.

When all of these channels have been captured then I enter the while loop and start storing the CCRx value in variables. Whenever I read a value from CCR1 register the SR registers bit 1 has to reset to 0. For CCRx, SR register bit x should reset according to the reference manual by ST. I am using a STM32F401RE.

enter image description here

enter image description here

The problem is when I am reading CCR1(ch1), CCIF1(SR bit 1) is set to '0' but also rest of the SR bits are set to '0' (bit 2,3,4). Even though I have not read from their respective registers.

Is there any way to avoid this?

edit::

#include "stm32f4xx.h" #include <stdint.h> #include <stdio.h> #include "math.h" #include<stdlib.h> #define SR_CC1IF (1U<<1) #define SR_CC2IF (1U<<2) #define SR_CC3IF (1U<<3) #define SR_CC4IF (1U<<4) #define SR_CC_ALL (SR_CC1IF | SR_CC2IF | SR_CC3IF | SR_CC4IF) int reg_a=0; int reg_b=0; int reg_c=0; int reg_d=0; void tim2_all_channels_input_capture(void); int x= 0; void main(void) { tim2_all_channels_input_capture(); while(1) { volatile uint32_t sr = TIM1->SR; if((sr & SR_CC_ALL) == SR_CC_ALL) { x=x+1; reg_a =TIM1->CCR1; reg_b =TIM1->CCR2; reg_c =TIM1->CCR3; reg_d =TIM1->CCR4; } } }
\$\endgroup\$

1 Answer 1

Reset to default
3
\$\begingroup\$

There's just too many bugs and problems... Some remarks/general code review:

  • (TIM1->SR & SR_CC2IF)==1 etc -> (TIM1->SR & (1U<<2))==1 is plain wrong since CC2IF results in 0x4 not 0x1. Same bug with all of those masks. Instead you should check (TIM1->SR & SR_CC2IF)!=0.

  • Most MCUs do not have bit-level instructions but operate at least on byte level. That means that each of your TIM1->SR & SR_CC1IF checks result in a register read - you keep reading the register over and over. That's wrong and inefficient.

  • reg_a =TIM1->CCR1; reads the register and as the friendly manual points out, doing so will clear the SR register.

  • While single stepping and having a raw memory map and/or hardware peripheral view of the register open in the debugger, the debugger is usually not smart enough to pay special respect to "clear by reading" registers. So it's a known problem with debuggers that single stepping through such code might cause the debugger to clear registers.

  • Are you sure that you should loop and read while the flags are set? That doesn't make much sense. Seems that the while should be swapped for an if.

  • Never use signed int when dealing with raw binary data. There are very few situations where you should ever use it at all. Stick to the stdint.h types instead.

  • The correct format of main() for "freestanding" microcontroller systems and the gcc compiler is void main (void), since freestanding systems never return from main. Make sure to compile with -ffreestanding.

  • Global variables should be avoided.

A first start towards a working program might look like this:

#include "stm32f4xx.h" #include <stdint.h> #define SR_CC1IF (1U<<1) #define SR_CC2IF (1U<<2) #define SR_CC3IF (1U<<3) #define SR_CC4IF (1U<<4) #define SR_CC_ALL (SR_CC1IF | SR_CC2IF | SR_CC3IF | SR_CC4IF) void main (void) { uint32_t x=0; uint32_t reg_a=0; uint32_t reg_b=0; uint32_t reg_c=0; uint32_t reg_d=0; tim1_all_channels_input_capture(); while(1) { volatile uint32_t sr = TIM1->SR; // only read the actual register once if((sr & SR_CC_ALL) == SR_CC_ALL) { x=x+1; reg_a = TIM1->CCR1; reg_b = TIM1->CCR2; reg_c = TIM1->CCR3; reg_d = TIM1->CCR4; } } }
\$\endgroup\$
9
  • \$\begingroup\$ The OP seems to want to check if all the CCxIF flags are set, while your suggestion only checks for any one of them. \$\endgroup\$ Commented Oct 31, 2023 at 15:12
  • \$\begingroup\$ @brhans Ah, fair enough. Will fix, thanks. \$\endgroup\$ Commented Oct 31, 2023 at 15:17
  • \$\begingroup\$ @Lundin Using global variable because if I make it local it doesn't work with the debugger. if((sr & SR_CC_ALL) == SR_CC_ALL) I think this wont work because SR register has more bits further up(bit 6,7,8...16) which are set by the system. I am using 'while' because i continuously want to monitor SR ,as soon as it is true it should be executed. I have made some changes to code in the answer ..can you have a look. \$\endgroup\$ Commented Oct 31, 2023 at 15:45
  • 1
    \$\begingroup\$ @rachitjuthani Bad debugger or maybe you are debugging with optimizations enabled. You could declare them static to move them off the stack, while keeping them in local scope. \$\endgroup\$ Commented Oct 31, 2023 at 15:48
  • 1
    \$\begingroup\$ @rachitjuthani "I think this wont work because SR register has more bits further up" It will work just fine, that's what bit masking is for in the first place: to mask out the relevant parts. \$\endgroup\$ Commented Oct 31, 2023 at 15:48