search

rocketscream / Low-Power

Low Power Library for Arduino
www.rocketscream.com
1.26k stars 345 forks source link

Timer 2 is not saved/restored properly #94

Closed rwared11 closed 4 years ago

rwared11 commented 4 years ago

I noticed that after returning from LowPower.idle my atmega328p's PWM was no longer working.

There seem to be a few bugs in the code, and it seems to be repeated in several places in LowPower.cpp. Sorry I'm not in a position to submit a pull request, but I thought I would raise the issues.

For instance:
if (TCCR2B & CS22) ... TCCR2B &= ~(1 << CS22) the handling of CS22, CS21, CS20 are inconsistent, sometimes used as a bit-position and sometimes as a raw value, leading to incorrect manipulation of the TCCR2B register.

Secondly, the same register is restored before calling power_timer2_enable, however it seems in my tests that power_timer2_enable changes this register as well, so the restoration needs to come after it, not before.

Instead of fixing these issues, I instead changed my version to save and restore the entire 8-bit TCCR2B register, and that has fixed my PWM issue.

rocketscream commented 4 years ago

I think there's a bug when restoring the original TCCR2B content. Bit manipulation of the CS20, CS21, CS22 is correct but nothing was saved on the rest of the bits (FOC2A, FOC2B and WGM22). So, I will patch this by copying the entire TCCR2B content onto the clockSource variable and restoring it before powering it back.

Secondly, the same register is restored before calling power_timer2_enable, however it seems in my tests that power_timer2_enable changes this register as well, so the restoration needs to come after it, not before.

The power_timer2_enable() does this only, so it is not the issue here: #define power_timer2_enable() (PRR &= (uint8_t)~(1 << PRTIM2))

Thanks for reporting.

rocketscream commented 4 years ago

Fixed with 2f9b2e5.