Skip to content

Commit

Permalink
🐛 Fix AVR 644/1284 Timer / PWM conflicts (MarlinFirmware#23629)
Browse files Browse the repository at this point in the history
  • Loading branch information
descipher authored and LCh-77 committed Feb 1, 2022
1 parent d32fa4b commit 1ea6791
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 26 deletions.
20 changes: 9 additions & 11 deletions Marlin/src/HAL/AVR/HAL_SPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,20 @@

void spiBegin() {
#if PIN_EXISTS(SD_SS)
OUT_WRITE(SD_SS_PIN, HIGH);
// Do not init HIGH for boards with pin 4 used as Fans or Heaters or otherwise, not likely to have multiple SPI devices anyway.
#if defined(__AVR_ATmega644__) || defined(__AVR_ATmega644P__) || defined(__AVR_ATmega644PA__) || defined(__AVR_ATmega1284P__)
// SS must be in output mode even it is not chip select
SET_OUTPUT(SD_SS_PIN);
#else
// set SS high - may be chip select for another SPI device
OUT_WRITE(SD_SS_PIN, HIGH);
#endif
#endif
SET_OUTPUT(SD_SCK_PIN);
SET_INPUT(SD_MISO_PIN);
SET_OUTPUT(SD_MOSI_PIN);

#if DISABLED(SOFTWARE_SPI)
// SS must be in output mode even it is not chip select
//SET_OUTPUT(SD_SS_PIN);
// set SS high - may be chip select for another SPI device
//#if SET_SPI_SS_HIGH
//WRITE(SD_SS_PIN, HIGH);
//#endif
// set a default rate
spiInit(1);
#endif
IF_DISABLED(SOFTWARE_SPI, spiInit(SPI_HALF_SPEED));
}

#if NONE(SOFTWARE_SPI, FORCE_SOFT_SPI)
Expand Down
8 changes: 4 additions & 4 deletions Marlin/src/HAL/AVR/fast_pwm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const Timer get_pwm_timer(const pin_t pin) {

#ifdef TCCR0A
case TIMER0B: // Protected timer, but allow setting the duty cycle on OCR0B for pin D4 only
return Timer({ { &TCCR0A, nullptr, nullptr }, { (uint16_t*)&OCR0B, nullptr, nullptr }, nullptr, 0, 0, true, true });
return Timer({ { &TCCR0A, nullptr, nullptr }, { (uint16_t*)&OCR0A, (uint16_t*)&OCR0B, nullptr }, nullptr, 0, 1, true, true });
#endif

#if HAS_TCCR2
Expand Down Expand Up @@ -187,8 +187,8 @@ void set_pwm_duty(const pin_t pin, const uint16_t v, const uint16_t v_size/*=255
const Timer timer = get_pwm_timer(pin);
if (timer.isPWM) {
if (timer.n == 0) {
TCCR0A |= _BV(COM0B1); // Only allow a TIMER0B select and OCR0B duty update for pin D4 outputs no frequency changes are permited.
OCR0B = v;
_SET_COMnQ(timer, timer.q, COM_CLEAR_SET); // Only allow a TIMER0B select...
_SET_OCRnQ(timer, timer.q, v); // ...and OCR0B duty update. For output pin D4 no frequency changes are permitted.
}
else if (!timer.isProtected) {
const uint16_t top = timer.n == 2 ? TERN(USE_OCR2A_AS_TOP, *timer.OCRnQ[0], 255) : *timer.ICRn;
Expand All @@ -197,7 +197,7 @@ void set_pwm_duty(const pin_t pin, const uint16_t v, const uint16_t v_size/*=255
}
}
else
digitalWrite(pin, v < 128 ? LOW : HIGH);
digitalWrite(pin, v < v_size / 2 ? LOW : HIGH);
}
}

Expand Down
22 changes: 11 additions & 11 deletions Marlin/src/HAL/AVR/timers.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ typedef uint16_t hal_timer_t;
#define DISABLE_STEPPER_DRIVER_INTERRUPT() CBI(TIMSK1, OCIE1A)
#define STEPPER_ISR_ENABLED() TEST(TIMSK1, OCIE1A)

#define ENABLE_TEMPERATURE_INTERRUPT() SBI(TIMSK0, OCIE0B)
#define DISABLE_TEMPERATURE_INTERRUPT() CBI(TIMSK0, OCIE0B)
#define TEMPERATURE_ISR_ENABLED() TEST(TIMSK0, OCIE0B)
#define ENABLE_TEMPERATURE_INTERRUPT() SBI(TIMSK0, OCIE0A)
#define DISABLE_TEMPERATURE_INTERRUPT() CBI(TIMSK0, OCIE0A)
#define TEMPERATURE_ISR_ENABLED() TEST(TIMSK0, OCIE0A)

FORCE_INLINE void HAL_timer_start(const uint8_t timer_num, const uint32_t) {
switch (timer_num) {
Expand All @@ -87,7 +87,7 @@ FORCE_INLINE void HAL_timer_start(const uint8_t timer_num, const uint32_t) {
case MF_TIMER_TEMP:
// Use timer0 for temperature measurement
// Interleave temperature interrupt with millies interrupt
OCR0B = 128;
OCR0A = 128;
break;
}
}
Expand Down Expand Up @@ -180,7 +180,7 @@ void TIMER1_COMPA_vect() { \
: \
: [timsk0] "i" ((uint16_t)&TIMSK0), \
[timsk1] "i" ((uint16_t)&TIMSK1), \
[msk0] "M" ((uint8_t)(1<<OCIE0B)),\
[msk0] "M" ((uint8_t)(1<<OCIE0A)),\
[msk1] "M" ((uint8_t)(1<<OCIE1A)) \
: \
); \
Expand All @@ -193,9 +193,9 @@ void TIMER1_COMPA_vect_bottom()

/* 14 cycles maximum latency */
#define HAL_TEMP_TIMER_ISR() \
extern "C" void TIMER0_COMPB_vect() __attribute__ ((signal, naked, used, externally_visible)); \
extern "C" void TIMER0_COMPB_vect_bottom() asm ("TIMER0_COMPB_vect_bottom") __attribute__ ((used, externally_visible, noinline)); \
void TIMER0_COMPB_vect() { \
extern "C" void TIMER0_COMPA_vect() __attribute__ ((signal, naked, used, externally_visible)); \
extern "C" void TIMER0_COMPA_vect_bottom() asm ("TIMER0_COMPA_vect_bottom") __attribute__ ((used, externally_visible, noinline)); \
void TIMER0_COMPA_vect() { \
__asm__ __volatile__ ( \
A("push r16") /* 2 Save R16 */ \
A("in r16, __SREG__") /* 1 Get SREG */ \
Expand Down Expand Up @@ -223,7 +223,7 @@ void TIMER0_COMPB_vect() { \
A("push r30") \
A("push r31") \
A("clr r1") /* C runtime expects this register to be 0 */ \
A("call TIMER0_COMPB_vect_bottom") /* Call the bottom handler - No inlining allowed, otherwise registers used are not saved */ \
A("call TIMER0_COMPA_vect_bottom") /* Call the bottom handler - No inlining allowed, otherwise registers used are not saved */ \
A("pop r31") \
A("pop r30") \
A("pop r27") \
Expand Down Expand Up @@ -251,10 +251,10 @@ void TIMER0_COMPB_vect() { \
A("reti") /* 4 Return from interrupt */ \
: \
: [timsk0] "i"((uint16_t)&TIMSK0), \
[msk0] "M" ((uint8_t)(1<<OCIE0B)) \
[msk0] "M" ((uint8_t)(1<<OCIE0A)) \
: \
); \
} \
void TIMER0_COMPB_vect_bottom()
void TIMER0_COMPA_vect_bottom()

#endif // HAL_TEMP_TIMER_ISR

0 comments on commit 1ea6791

Please sign in to comment.