🐛 Fix AVR 644/1284 Timer / PWM conflicts (#23629)

Author: descipher

Marlin/src/HAL/AVR/HAL_SPI.cpp

@@ -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)

Marlin/src/HAL/AVR/fast_pwm.cpp

@@ -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
@@ -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;
@@ -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);
   }
 }
 

Marlin/src/HAL/AVR/timers.h

@@ -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) {
@@ -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;
   }
 }
@@ -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)) \
     : \
   ); \
@@ -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 */ \
@@ -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")                        \
@@ -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