0

I am working on changing LED brightness via ADC value.Actually I did with standard periph library and it works preatty well but when ı want to write my code in register layer.I cant find any documents except ST cookbook document.So I write my code referenced by cookbook.It works but not as ı want,I can change brightness of LEDs one time.What can I do for change value of LEDs continously? My code shown in below

while (1)
  {     
    value=ADC_Read();
    //PD12,PD13,PD14,PD15 selected for pwm control
     pulse_value=map(value,0,4095,0,9999);
    
     TIM4->CCR1|=pulse_value;
     TIM4->CCR2|=pulse_value;
  
     TIM4->CCMR1|=0;
     TIM4->CCMR1|=0x60;
     TIM4->CCMR1|=0x6000;
     
      TIM4->CCER|= 1<<0 | 1<<4;
      
      
      delay(1680000);
  
     TIM4->CCR3|=pulse_value;
     TIM4->CCR4|=pulse_value;
  
     TIM4->CCMR2|=0;
     TIM4->CCMR2|=0x60;
     TIM4->CCMR2|=0x6000;
     
      TIM4->CCER|= 1<<8 | 1<<12;
      TIM4->CR1|=0x1;
      
      delay(1680000);
   TIM4->CCER|= 1<<0 | 0<<4| 0<<8 | 0<<12;

1
  • you need to learn bitwise operations. Without it you will not succeed in the uC programming. The whole code is full of bugs. BTW yu cant change CCRx values when you want. The best time is the overflow or CCRx event Commented Jun 27, 2020 at 10:21

1 Answer 1

2

So when I messed with a PWM on three LEDs I assume this was on an STM32 chip (STM32F4 is too vague in the future more of the part number is needed)(this was an STM32F031F4P6), looks like the timers are similar not uncommon to re-use IP across products.

PUT32(TIM2_CR1,0x00000000);
PUT32(TIM2_CCR2,200-1);
PUT32(TIM2_CCR3,400-1);
PUT32(TIM2_CCR4,600-1);
PUT32(TIM2_ARR, 800-1);
PUT32(TIM2_CNT,0x00000000);
PUT32(TIM2_PSC,0x00000000);
PUT32(TIM2_CCMR1,(6<<12));
PUT32(TIM2_CCMR2,(6<<12)|(7<<4));
PUT32(TIM2_CCER,(1<<4)|(1<<8)|(1<<12));
PUT32(TIM2_CR1,0x00000001);

while(1)
{
    for(ra=0;ra<800;ra++)
    {
        PUT32(TIM2_CCR2,ra);
        PUT32(TIM2_CCR3,ra);
        PUT32(TIM2_CCR4,ra);
        for(rb=0;rb<2000;rb++) dummy(rb);
    }
    for(ra--;ra;ra--)
    {
        PUT32(TIM2_CCR2,ra);
        PUT32(TIM2_CCR3,ra);
        PUT32(TIM2_CCR4,ra);
        for(rb=0;rb<2000;rb++) dummy(rb);
    }
}

And this works, as a reference (yes technically some of those are 16 bit registers but st is flexible on this allowing 32 bit accesses, and notice they spaced the registers out on word boundaries anyway.

So this is a problem

 TIM4->CCR1|=pulse_value;
 TIM4->CCR2|=pulse_value;

you want to replace the bits not ORR them with what was there, eventually they will be all ones

TIM4->CCR1 = (TIM4->CCR1)&(~0xFFFF)|pulse_value;

But really since the register is not partitioned it is one field/value

TIM4->CCR1 = pulse_value;

Would be the right thing to do.

This does nothing of course:

TIM4->CCMR1|=0;

Did you mean

TIM4->CCMR1=0;

? Why do the fields separately (as a habit and have the peripheral change for each of your read-modify-writes, just set the register bits you want in one shot, be it ONE read-modify-write or one write). As a general rule not safe to do the fields in separate operations like this against a peripheral. And you can't just or equals a register without anding the field to clear it.

Bits 6:4 OC1M: Output compare 1 mode
110: PWM mode 1

TIM4->CCMR1|=0x60;

Instead something like this is needed for that one field

TIM4->CCMR1 = (TIM4->CCMR1&(~(7<<4))) | (6<<4);

I find it more readable as

x = TIM4->CCMR1;
x&=(~7)<<4;
x|=   6<<4;
TIM4->CCMR1 = x;

Or even better

Bits 14:12 OC2M[2:0]: Output compare 2 mode
Bits 6:4 OC1M: Output compare 1 mode

x = TIM4->CCMR1;
x&=(~7)<<12;
x|=   6<<12;
x&=(~7)<<4;
x|=   6<<4;
TIM4->CCMR1 = x;

I suspect what you were really wanting to do was:

 TIM4->CCMR1=0;
 TIM4->CCMR1|=0x6060;

The register resets to 0x0000 so

 TIM4->CCMR1=0x6060;

would have been fine. Or for readability

 TIM4->CCMR1=(6<<12)|(6<<4);

And you can see the 12 and 4 straight from the reference manual in the code as well as the 6 (110 binary).

And further I believe (you can read the docs as well as I can/have) that you only need to set up the pwm one time so all the registers only do one time at init, then for every new value you read from the adc, you only change the compare register value. And on that period or the next the pwm will change state relative to the new register value (depending on when you wrote it relative to the current period).

You are pretty close I think you get the idea, just work on your AND, OR, XOR, and NOT skills (bitwise logical operations).

When dealing with say a gpio register where pin A3 may be some control signal for widget B and A5 may be some control signal for widget X, and you want to have separate software for each widget (some external peripheral or led or button or whatever), then read-modify-writes (done correctly) make a lot of sense depending on how the logic for that gpio peripheral for that chip work. But if you are configuring a uart, a timer, a spi controller, etc. Where you need to control all the bits in a register in one shot, you own the whole thing with one driver, then read-modify-writes don't necessarily make sense. Just write the whole register with all the bits you want for that one configuration as far as the register access goes. For readability (relative term, beauty is in the eye of the beholder, to each his(/her) own, etc) of the code you may choose to just write the hex value of the whole thing, do some shifting so that you can easily translate the values to/from the reference manual to help reduce errors. Make a number of macros that do the bit manipulation on a variable so that you can really plug in values from the reference manual into the code (SET_BITS(x,16,14,6); SET_BITS(x,6,4,6);) etc.

Your main loop could end up looking like

while (1)
{     
  value=ADC_Read();
  //PD12,PD13,PD14,PD15 selected for pwm control
   pulse_value=map(value,0,4095,0,9999);

   TIM4->CCR1=pulse_value;
   TIM4->CCR2=pulse_value;
   TIM4->CCR3=pulse_value;
   TIM4->CCR4=pulse_value;
   delay(1680000);
}

with the setup up front being the rest of the registers in the TIM that need to be written to make it work. possibly using a small value for the CCR1 registers or some sort of starting point, or do an adc read then do the setup with that adc reads pulse_value and then go into the while loop.

Sign up to request clarification or add additional context in comments.

1 Comment

Thank you so much for your comment,so usefull for me.I am still Electronics Engineering student so actually beginner for this work.I aware my faults now,I will work to improve my skills.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.