MSP430G2432: Strange issue when changing duty cycle

In case it is helpful, here is my initialization code...

void tmra_init(void)
{
	TA0CTL = TASSEL_2 | ID_3 | MC_1;  // SMCLK, 1 MHz, Up Mode
	TACCR0 = 1000 - 1;			// 1 ms overflow -> 1 kHz Frequency
	TACCTL0 = CCIE;				// Enable Interrupt (Used for 100 us overflow)

	// PWM1 Setup
	TA0CCTL1 = OUTMOD_6;		// Port 2.6 (pin 19) -> PWM toggle/set mode
	TACCR1 = 0;					// Start at 0% duty cycle
	P2SEL |= BIT6;				// Port 2.6 (pin 19) -> CCR1 Output

	// PWM2 Setup
	TA0CCTL2 = OUTMOD_6;		// Port 1.4 (pin 6) -> PWM toggle/set mode
	TACCR2 = 0;					// Start at 0% duty cycle
	P1SEL |= BIT4;				// Port 1.4 (pin 6) -> CCR2 Output
	P1SEL2 |= BIT4;

	P2DIR |= BIT6;
	P2SEL |= BIT6;
	P2SEL2 &= ~BIT6;

	// Port 2.7 Setup
	P2SEL &= ~BIT7;
	P2SEL2 &= ~BIT7;

	TA0CTL = TASSEL_2 | ID_3 | MC_1;  // SMCLK, 1 MHz, Up Mode
}

And here is where I change the value of the duty cycle.

void set3000KdutyCycle(uint16_t duty)
{
	TACCR1 = duty;
}



void set5000KdutyCycle(uint16_t duty)
{
	TACCR2 = duty;
}

Hi Bruce,

Thank you for your reply.

I understand your thought process behind missing a compare due to the CCR value being less than the TAR value.  I added this line of code to prevent that from happening.

void set3000KdutyCycle(uint16_t duty)
{
    if (duty <= TAR) {}
    else
    {
        TACCR1 = duty;
    }

}

Unfortunately, it did not fix it.  It did make it happen less often and at different duty cycles.

I also tried changing to all the different output modes without success.  Changing it to mode 2 from mode 6 made the signal stay low during the messed up cycles instead of high.

Do you have any other ideas to try?

Thanks

Hi Bruce,

I think you could be on the right track here.  It makes a lot of sense.

“I suspect you're updating the duty cycle in your CCR0 interrupt. You may want to try enabling the CCR1/2 interrupts (TIMER0_A1_VECTOR) and updating the relevant CCR there. By updating right after the CCR match, you have a full cycle to get the update in. This isn't a general solution, but in your case, where your increments are small, it should work.”

I’m not actually updating the duty cycle in CCRO.  However, my code runs in a timed loop which starts when TAR matches TACCR0 (every 1 ms).  Therefore, the duty cycle is getting updated shortly after the CCR0 interrupt.

I tried to change per your suggestion.  However, apparently I have something set up wrong because I set breakpoints up for each case and the program only stops when the CCR0 overflows and not CCR1 and CCR2 matches.  Do you see anything wrong with this code?

Here is my initialization...

void tmra_init(void)
{
       TA0CTL = TASSEL_2 | ID_3 | MC_1;  // SMCLK, 1 MHz, Up Mode
       TACCR0 = 1000 - 1;                // 1 ms overflow -> 1 kHz Frequency

       // PWM1 Setup
       TA0CCTL1 = OUTMOD_6;       // Port 2.6 (pin 19) -> PWM toggle/set mode
       TACCR1 = 0;                              // Start at 0% duty cycle
       P2SEL |= BIT6;                           // Port 2.6 (pin 19) -> CCR1 Output

       // PWM2 Setup
       TA0CCTL2 = OUTMOD_6;       // Port 1.4 (pin 6) -> PWM toggle/set mode
       TACCR2 = 0;                              // Start at 0% duty cycle
       P1SEL |= BIT4;                           // Port 1.4 (pin 6) -> CCR2 Output
       P1SEL2 |= BIT4;

       P2DIR |= BIT6;
       P2SEL |= BIT6;
       P2SEL2 &= ~BIT6;

       // Port 2.7 Setup
       P2SEL &= ~BIT7;
       P2SEL2 &= ~BIT7;

       TA0CTL = TASSEL_2 + ID_3 + MC_1 + TAIE;  // SMCLK, 1 MHz, Up Mode, Enable TAIFG Interrupt
}

And here is the interrupt vector...

#pragma vector=TIMER0_A1_VECTOR
__interrupt void timer0_A1_interrupt(void)
{
    switch (TAIV)
    {
    case TA0IV_TACCR1:
        if (prev3000KDuty != currDutyCycle3000K)
        {
            TACCR1 = currDutyCycle3000K;
            prev3000KDuty = currDutyCycle3000K;
        }
        else {}
        break;
    case TA0IV_TACCR2:
        if (prev5000KDuty != currDutyCycle5000K)
        {
            TACCR2 = currDutyCycle5000K;
            prev5000KDuty = currDutyCycle5000K;
        }
        else {}
        break;
    case TA0IV_TAIFG:
        one_ms_elapsed = TRUE;
        break;
    default:
        break;
    }

}

I have breakpoints set on lines 7, 15, and 23 of the interrupt vector.  It is only stopping on 23.

Thank you very much for your help.

John

Updating CCR1 in CCR1 match ISR will cause problems when CCR1 value is incremented - you will get double CCR match during single timer cycle which in case of toggle PWM mode obviously will cause glitch. 

Better put CCR1/CCR2 update in timer overflow interrupt, not "in a timed loop which starts when TAR matches TACCR0 (every 1 ms).".

Hi llmars,

Thanks for your reply.  I think I didn't explain what I'm doing well.  When the TAR matches TACCRO, I set a flag.  Then, my main loop only consists of checking and clearing the flag and running my routines.

 while (1u)
    {
		if (one_ms_elapsed)
		{
			one_ms_elapsed = FALSE;

			light_task();
			timer_task();

			CLR_WDT_M();

		}
    	else {}

    }

I set this flag in the TIMER0_A0_VECTOR at first, but I changed to the TIMER0_A1_VECTOR per Bruce's suggestion.   

Don't worry, I got what you are doing. Thing is that your polling cycle approach is much more CPU-intensive than what I offer. Timer clock is 1MHz, CPU max freq is 16MHz meaning just 16 CPU cycles per timer tick. If you want glitchless CCR updates at low CCR values, you shall not be wasting CPU time like you do. Even ISR approach could be too late when CCR1/CCR2 is equal 1 and going to be 2 after update, but I can't tell - you shall try (obviously if you choose to).

Hi Caleb,

Thanks for your reply.  That fixed that issue.  Unfortunately, I still have the weird duty cycle issue after trying this suggestion.  Any other ideas?

Thanks,

John

I should probably be more specific about what I tried :)

Bruce suggested updating the duty cycles when each of the compares happen.  So, I changed my code in such a way that instead of actually updating the TACCRx register, it sets a global variable for the duty cycle and a flag to change it during the next compare interrupt.  

....

currDutyCycle5000K++;
update5000KPWM = TRUE;

....

#pragma vector=TIMER0_A1_VECTOR
__interrupt void timer0_A1_interrupt(void)
{
    switch (TAIV)
    {
    case TA0IV_TACCR1:
        if (update5000KPWM)
        {
            TACCR2 = currDutyCycle5000K;
            update5000KPWM = FALSE;
        }
        else {}
        break;
    case TA0IV_TACCR2:
        if (update3000KPWM)
        {
            TACCR1 = currDutyCycle3000K;
            update3000KPWM = FALSE;
        }
        else {}
        break;
    case TA0IV_TAIFG:
        one_ms_elapsed = TRUE;
        break;
    default:
        break;
    }

}

Hi John,

Thanks for the clarification. I think this approach is a little flawed as Ilmars pointed out:

"Updating CCR1 in CCR1 match ISR will cause problems when CCR1 value is incremented - you will get double CCR match during single timer cycle which in case of toggle PWM mode obviously will cause glitch. 

Better put CCR1/CCR2 update in timer overflow interrupt, not "in a timed loop which starts when TAR matches TACCR0 (every 1 ms)."

Can you try a similar approach to what you've done above, but instead of changing the CCRx value in the CCRx interrupt, change it in the CCR0 interrupt when the respective updatexxxxKPWM flag has been set?

Best regards, 
Caleb Overbay

Hi Caleb,

I changed my interrupt routine to this.

#pragma vector=TIMER0_A1_VECTOR
__interrupt void timer0_A1_interrupt(void)
{
    switch (TAIV)
    {
    case TA0IV_TACCR1:
        break;
    case TA0IV_TACCR2:
        break;
    case TA0IV_TAIFG:
        one_ms_elapsed = TRUE;

        if (update5000KPWM)
        {
            TACCR2 = currDutyCycle5000K;
            update5000KPWM = FALSE;
        }
        else {}

        if (update3000KPWM)
        {
            TACCR1 = currDutyCycle3000K;
            update3000KPWM = FALSE;
        }
        else {}

        break;
    default:
        break;
    }

}

When I ran it that way, it did something really strange.  The output would go high for 2.5 us and back low.  

Then, I remove the CCIE enable from each TA0CCTLx register to see if that would work and since they are irrelevant now anyway.

// PWM1 Setup
//TA0CCTL1 = OUTMOD_6 | CCIE;		// Port 2.6 (pin 19) -> PWM toggle/set mode
TA0CCTL1 = OUTMOD_6;
TACCR1 = 0;					// Start at 0% duty cycle
P2SEL |= BIT6;				// Port 2.6 (pin 19) -> CCR1 Output

// PWM2 Setup
//TA0CCTL2 = OUTMOD_6 | CCIE;		// Port 1.4 (pin 6) -> PWM toggle/set mode
TA0CCTL2 = OUTMOD_6;
TACCR2 = 0;					// Start at 0% duty cycle
P1SEL |= BIT4;				// Port 1.4 (pin 6) -> CCR2 Output
P1SEL2 |= BIT4;

It worked after that, but the duty cycle issue is still happening.

Caleb Overbay said:

I've been able to test out your code and replicate the issues you've been experiencing.

Just curious - at which CCR1/CCR2 range update glitch problems occur? Is it low range <= 2?

If it is so, then I would implement CCR1 update following way which wait for timer to run away "at safe distance" from CCR1, then update it:

If (TACCR1 < 3)
  __delay_cycles(16*50); // assuming timerfreq == 1MHz and CPUfreq == 16MHz
TACCR1 = currDutyCycle3000K;
update3000KPWM = FALSE;

Caleb Overbay said:

Hi John,

I've been able to test out your code and replicate the issues you've been experiencing. I've also tried all the workarounds we've suggested throughout this thread and reached the same results as you. The good thing here is that we can replicate it and we're both on the same page.

Unfortunately, I still don't have an explanation for this issue. As you've described, even stopping the timer and then updating the CCRx values doesn't solve the issue. This is something that concerns me because it is the "correct" way to modify these values without seeing race conditions.

I need to do some more testing and I'll work with our quality team to find out what is really going on here. I'll keep you updated throughout the process.

Best regards,
Caleb Overbay

Hi Caleb,

Thanks for the update.  It's good that you can repeat the issue.  Let me know if you want me to try anything on my end.

Thanks,

John

Hi Ilmars, 

After looking at where the glitch occurs, I've calculated that it's somewhere in the range of when CCR1 transitions from a value of 6 to a value of 7. There should be plenty of time for the CCR0 interrupt to update the register. Even when I stopped the timer and then updated the value in CCR1, the issue was still occurring.

I'll test out your above theory and let you all know the results. 

Best regards, 
Caleb Overbay

Caleb Overbay said:

After looking at where the glitch occurs, I've calculated that it's somewhere in the range of when CCR1 transitions from a value of 6 to a value of 7.

CPU @8MHz have just 8 clock cycles to execute during tick of timer which is running at 1MHz. During 6 timer ticks CPU possibly could not even finish to handle whole ISR and is right at CCR update instruction while timer already reached value 6 & CCR match occurred, PWM output toggled. Then CPU changes CCR value to 7 and at next timer tick next match occur (during same PWM period), output toggle again, you see glitch on scope. In set/reset or reset/set mode you would not see the glitch.

I would suggest to reconsider PWM mode, change it from toggle to set/reset and/or reset/set and close this thread of timer/CPU race condition fishing :)

Hi Ilmars,

This was what I was expecting as well, but changing the output mode had no effect. I do agree that a toggle output mode shouldn't be used for this application.

Some things I'm going to try today:

• Changing the 10ms updated period to 20ms
• Updating in the CCRx interrupt after some delay
• Trying on another part with TimerA module

Best regards, 
Caleb Overbay

Hi John,

I have some good news. It appears that there is a very unique race condition going on here, I still haven't found the exact root cause. However, I am able to alleviate the issue by using Ilmar's suggestion of adding a delay before updating CCR1 if it's value is less than 7:

        if (update3000KPWM)
        {
            if(TACCR1 < 7)
              __delay_cycles(8*50);         //Assuming timerfreq == 1MHz and CPUfreq == 8MHz
            TA0CTL &= ~MC_1;                //Stop Timer
            TACCR1 = currDutyCycle3000K;
            TA0CTL |= MC_1;                 //Start Timer
            update3000KPWM = FALSE;
        }
        else {}

By adding this delay we are ensuring the value in TAR is at least 50 before updating CCR1 to 7. This avoids the race condition that was occurring because of the update from 6 to 7.  Also, I highly recommend changing your OUTMOD to Reset/Set. This way we know that the PWM falling edge is always when CCRx is reached and the rising edge is always when CCR0 is reached.

I'll do a little more digging into this to discover a root cause but can you try this out and let me know your results.

Best regards, 

Caleb Overbay

Great!  I'll test it out Monday morning and let you know the results.  Thanks for your input, Ilmars.

Caleb Overbay said:

By adding this delay we are ensuring the value in TAR is at least 50 before updating CCR1 to 7. This avoids the race condition that was occurring because of the update from 6 to 7.

Glad you are making progress, working on it so hard. IMHO timer restart is superflous, it also does not help PWM frequency to be constant. BTW good idea in such cases as this is to use debug pin - set it before CCR update and reset after - to see when exactly CCR update respectively to PWM cycle state, occur.

Hi Caleb Overbay and Ilmars.

That fixed the issue with the code that Caleb has.  Unfortunately, when I put the fix back in to my production code, the very last cycle on the TACCR1 line stays high (when the TACCR2 line is off).  

Caleb,

I will send you my code in a PM.

Thanks,

John

John Deaton said:

I will send you my code in a PM.

Did you try to understand why this happens? Or just "I will send you my code, fix it".

What gave you the impression that I don't want to understand the issue?

John Deaton said:

the very last cycle on the TACCR1 line stays high (when the TACCR2 line is off).  

Instead of trying to stop timer at right time and w/o glitch, just switch off PWM outputs, then disable timer: 1) set desired values for PWM out pins in PxOUT 2) change PW output pin function to GPIO output 3) disable timer.

Hi Caleb,

Thanks for checking in.  Yes, it seems to be working.  Do you mean for this application I should have chosen a MCU with Timer B?  Unfortunately, I had to pick the cheapest option that would work as often happens :). 

Regards,

John