Browse Source

Add memory barrier, optimal interrupt on-off

Disabling an ISR on ARM has 3 instructions of latency. A Memory barrier is REQUIRED to ensure proper and predictable disabling. Memory barriers are expensive, so avoid disabling if already disabled (See https://mcuoneclipse.com/2015/10/16/nvic-disabling-interrupts-on-arm-cortex-m-and-the-need-for-a-memory-barrier-instruction/)
etagle 6 years ago
parent
commit
0566badcef

+ 5
- 0
Marlin/src/HAL/HAL_DUE/DebugMonitor_Due.cpp View File

@@ -46,6 +46,11 @@ static void TXBegin(void) {
46 46
   // Disable UART interrupt in NVIC
47 47
   NVIC_DisableIRQ( UART_IRQn );
48 48
 
49
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
50
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
51
+  __DSB();
52
+  __ISB();
53
+
49 54
   // Disable clock
50 55
   pmc_disable_periph_clk( ID_UART );
51 56
 

+ 10
- 0
Marlin/src/HAL/HAL_DUE/HAL_timers_Due.cpp View File

@@ -99,6 +99,11 @@ void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) {
99 99
   // Disable interrupt, just in case it was already enabled
100 100
   NVIC_DisableIRQ(irq);
101 101
 
102
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
103
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
104
+  __DSB();
105
+  __ISB();
106
+
102 107
   // Disable timer interrupt
103 108
   tc->TC_CHANNEL[channel].TC_IDR = TC_IDR_CPCS;
104 109
 
@@ -133,6 +138,11 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num) {
133 138
 void HAL_timer_disable_interrupt(const uint8_t timer_num) {
134 139
   IRQn_Type irq = TimerConfig[timer_num].IRQ_Id;
135 140
   NVIC_DisableIRQ(irq);
141
+
142
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
143
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
144
+  __DSB();
145
+  __ISB();
136 146
 }
137 147
 
138 148
 // missing from CMSIS: Check if interrupt is enabled or not

+ 10
- 0
Marlin/src/HAL/HAL_DUE/MarlinSerial_Due.cpp View File

@@ -245,6 +245,11 @@
245 245
     // Disable UART interrupt in NVIC
246 246
     NVIC_DisableIRQ( HWUART_IRQ );
247 247
 
248
+    // We NEED memory barriers to ensure Interrupts are actually disabled!
249
+    // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
250
+    __DSB();
251
+    __ISB();
252
+
248 253
     // Disable clock
249 254
     pmc_disable_periph_clk( HWUART_IRQ_ID );
250 255
 
@@ -290,6 +295,11 @@
290 295
     // Disable UART interrupt in NVIC
291 296
     NVIC_DisableIRQ( HWUART_IRQ );
292 297
 
298
+    // We NEED memory barriers to ensure Interrupts are actually disabled!
299
+    // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
300
+    __DSB();
301
+    __ISB();
302
+
293 303
     pmc_disable_periph_clk( HWUART_IRQ_ID );
294 304
   }
295 305
 

+ 5
- 0
Marlin/src/HAL/HAL_DUE/watchdog_Due.cpp View File

@@ -68,6 +68,11 @@ void watchdogSetup(void) {
68 68
       // Disable WDT interrupt (just in case, to avoid triggering it!)
69 69
       NVIC_DisableIRQ(WDT_IRQn);
70 70
 
71
+      // We NEED memory barriers to ensure Interrupts are actually disabled!
72
+      // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
73
+      __DSB();
74
+      __ISB();
75
+
71 76
       // Initialize WDT with the given parameters
72 77
       WDT_Enable(WDT, value);
73 78
 

+ 5
- 0
Marlin/src/HAL/HAL_LPC1768/HAL_timers.h View File

@@ -143,6 +143,11 @@ FORCE_INLINE static void HAL_timer_disable_interrupt(const uint8_t timer_num) {
143 143
     case 0: NVIC_DisableIRQ(TIMER0_IRQn); // Disable interrupt handler
144 144
     case 1: NVIC_DisableIRQ(TIMER1_IRQn); // Disable interrupt handler
145 145
   }
146
+
147
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
148
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
149
+  __DSB();
150
+  __ISB();
146 151
 }
147 152
 
148 153
 // This function is missing from CMSIS

+ 21
- 0
Marlin/src/HAL/HAL_LPC1768/LPC1768_PWM.cpp View File

@@ -258,6 +258,11 @@ bool LPC1768_PWM_attach_pin(pin_t pin, uint32_t min /* = 1 */, uint32_t max /* =
258 258
                                  // OK to update the active table because the
259 259
                                  // ISR doesn't use any of the changed items
260 260
 
261
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
262
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
263
+  __DSB();
264
+  __ISB();
265
+
261 266
   if (ISR_table_update) //use work table if that's the newest
262 267
     temp_table = work_table;
263 268
   else
@@ -342,6 +347,11 @@ bool LPC1768_PWM_detach_pin(pin_t pin) {
342 347
 ////  interrupt controlled PWM code
343 348
   NVIC_DisableIRQ(HAL_PWM_TIMER_IRQn);
344 349
 
350
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
351
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
352
+  __DSB();
353
+  __ISB();
354
+
345 355
   if (ISR_table_update) {
346 356
     ISR_table_update = false;    // don't update yet - have another update to do
347 357
     NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);  // re-enable PWM interrupts
@@ -428,6 +438,12 @@ bool LPC1768_PWM_write(pin_t pin, uint32_t value) {
428 438
 
429 439
 ////  interrupt controlled PWM code
430 440
   NVIC_DisableIRQ(HAL_PWM_TIMER_IRQn);
441
+
442
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
443
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
444
+  __DSB();
445
+  __ISB();
446
+
431 447
   if (!ISR_table_update)   // use the most up to date table
432 448
     COPY_ACTIVE_TABLE;  // copy active table into work table
433 449
 
@@ -456,6 +472,11 @@ bool useable_hardware_PWM(pin_t pin) {
456 472
 
457 473
   NVIC_DisableIRQ(HAL_PWM_TIMER_IRQn);
458 474
 
475
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
476
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
477
+  __DSB();
478
+  __ISB();
479
+
459 480
   bool return_flag = false;
460 481
   for (uint8_t i = 0; i < NUM_ISR_PWMS; i++)         // see if it's already setup
461 482
     if (active_table[i].pin == pin) return_flag = true;

+ 5
- 0
Marlin/src/HAL/HAL_STM32F4/HAL_timers_STM32F4.cpp View File

@@ -123,6 +123,11 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num) {
123 123
 
124 124
 void HAL_timer_disable_interrupt(const uint8_t timer_num) {
125 125
   HAL_NVIC_DisableIRQ(timerConfig[timer_num].IRQ_Id);
126
+
127
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
128
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
129
+  __DSB();
130
+  __ISB();
126 131
 }
127 132
 
128 133
 hal_timer_t HAL_timer_get_compare(const uint8_t timer_num) {

+ 5
- 0
Marlin/src/HAL/HAL_STM32F7/HAL_timers_STM32F7.cpp View File

@@ -127,6 +127,11 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num) {
127 127
 
128 128
 void HAL_timer_disable_interrupt(const uint8_t timer_num) {
129 129
   HAL_NVIC_DisableIRQ(timerConfig[timer_num].IRQ_Id);
130
+
131
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
132
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
133
+  __DSB();
134
+  __ISB();
130 135
 }
131 136
 
132 137
 hal_timer_t HAL_timer_get_compare(const uint8_t timer_num) {

+ 21
- 0
Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.cpp View File

@@ -29,6 +29,22 @@
29 29
 #include "HAL.h"
30 30
 #include "HAL_timers_Teensy.h"
31 31
 
32
+/** \brief Instruction Synchronization Barrier
33
+  Instruction Synchronization Barrier flushes the pipeline in the processor,
34
+  so that all instructions following the ISB are fetched from cache or
35
+  memory, after the instruction has been completed.
36
+*/
37
+FORCE_INLINE static void __ISB(void) {
38
+  __asm__ __volatile__("isb 0xF":::"memory");
39
+}
40
+
41
+/** \brief Data Synchronization Barrier
42
+  This function acts as a special kind of Data Memory Barrier.
43
+  It completes when all explicit memory accesses before this instruction complete.
44
+*/
45
+FORCE_INLINE static void __DSB(void) {
46
+  __asm__ __volatile__("dsb 0xF":::"memory");
47
+}
32 48
 
33 49
 void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) {
34 50
   switch (timer_num) {
@@ -65,6 +81,11 @@ void HAL_timer_disable_interrupt(const uint8_t timer_num) {
65 81
     case 0: NVIC_DISABLE_IRQ(IRQ_FTM0); break;
66 82
     case 1: NVIC_DISABLE_IRQ(IRQ_FTM1); break;
67 83
   }
84
+
85
+  // We NEED memory barriers to ensure Interrupts are actually disabled!
86
+  // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
87
+  __DSB();
88
+  __ISB();
68 89
 }
69 90
 
70 91
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num) {

+ 0
- 4
Marlin/src/feature/Max7219_Debug_LEDs.cpp View File

@@ -73,7 +73,6 @@ static uint8_t LEDs[8] = { 0 };
73 73
 #endif
74 74
 
75 75
 void Max7219_PutByte(uint8_t data) {
76
-  CRITICAL_SECTION_START;
77 76
   for (uint8_t i = 8; i--;) {
78 77
     SIG_DELAY();
79 78
     WRITE(MAX7219_CLK_PIN, LOW);       // tick
@@ -84,12 +83,10 @@ void Max7219_PutByte(uint8_t data) {
84 83
     SIG_DELAY();
85 84
     data <<= 1;
86 85
   }
87
-  CRITICAL_SECTION_END;
88 86
 }
89 87
 
90 88
 void Max7219(const uint8_t reg, const uint8_t data) {
91 89
   SIG_DELAY();
92
-  CRITICAL_SECTION_START;
93 90
   WRITE(MAX7219_LOAD_PIN, LOW);  // begin
94 91
   SIG_DELAY();
95 92
   Max7219_PutByte(reg);          // specify register
@@ -99,7 +96,6 @@ void Max7219(const uint8_t reg, const uint8_t data) {
99 96
   WRITE(MAX7219_LOAD_PIN, LOW);  // and tell the chip to load the data
100 97
   SIG_DELAY();
101 98
   WRITE(MAX7219_LOAD_PIN, HIGH);
102
-  CRITICAL_SECTION_END;
103 99
   SIG_DELAY();
104 100
 }
105 101
 

+ 0
- 2
Marlin/src/module/temperature.cpp View File

@@ -1085,9 +1085,7 @@ void Temperature::updateTemperaturesFromRawValues() {
1085 1085
     watchdog_reset();
1086 1086
   #endif
1087 1087
 
1088
-  CRITICAL_SECTION_START;
1089 1088
   temp_meas_ready = false;
1090
-  CRITICAL_SECTION_END;
1091 1089
 }
1092 1090
 
1093 1091
 

Loading…
Cancel
Save