From af204f9567f71983c7fe7c3817f2ada212113b5f Mon Sep 17 00:00:00 2001 From: Makuna Date: Mon, 3 Aug 2015 19:35:17 -0700 Subject: [PATCH 1/3] Interrupt cleanup Fixes issue of reentrant calls to nointerrupts() exposed functional replacements to cli sei and SREG when dealing with interrupts InterruptLock class to auto stop and restore interrupt level Fix user ISR calls to be like Arduino with interrupts disabled fully. --- .../esp8266/cores/esp8266/Arduino.h | 49 ++++++++++++++++--- .../cores/esp8266/core_esp8266_timer.c | 12 ++++- .../esp8266/core_esp8266_wiring_digital.c | 7 +-- .../libraries/Servo/src/esp8266/Servo.cpp | 4 -- .../esp8266/tools/sdk/include/ets_sys.h | 4 +- 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/hardware/esp8266com/esp8266/cores/esp8266/Arduino.h b/hardware/esp8266com/esp8266/cores/esp8266/Arduino.h index 6b379d5b1e..b293bb633e 100644 --- a/hardware/esp8266com/esp8266/cores/esp8266/Arduino.h +++ b/hardware/esp8266com/esp8266/cores/esp8266/Arduino.h @@ -137,17 +137,50 @@ void timer0_detachInterrupt(void); void ets_intr_lock(); void ets_intr_unlock(); -// level (0-15), -// level 15 will disable ALL interrupts, -// level 0 will disable most software interrupts +#ifndef __STRINGIFY +#define __STRINGIFY(a) #a +#endif + +// these low level routines provide a replacement for SREG interrupt save that AVR uses +// but are esp8266 specific. A normal use pattern is like +// +//{ +// uint32_t savedPS = xt_rsil(1); // this routine will allow level 2 and above +// // do work here +// xt_wsr_ps(savedPS); // restore the state +//} // -#define xt_disable_interrupts(state, level) __asm__ __volatile__("rsil %0," __STRINGIFY(level) : "=a" (state)) -#define xt_enable_interrupts(state) __asm__ __volatile__("wsr %0,ps; isync" :: "a" (state) : "memory") +// level (0-15), interrupts of the given level and above will be active +// level 15 will disable ALL interrupts, +// level 0 will enable ALL interrupts, +// +#define xt_rsil(level) (__extension__({uint32_t state; __asm__ __volatile__("rsil %0," __STRINGIFY(level) : "=a" (state)); state;})) +#define xt_wsr_ps(state) __asm__ __volatile__("wsr %0,ps; isync" :: "a" (state) : "memory") + +#define interrupts() xt_rsil(0) +#define noInterrupts() xt_rsil(15) -extern uint32_t interruptsState; +// this auto class wraps up xt_rsil so your code can be simplier, but can only be +// used in an ino or cpp files. A normal use pattern is like +// +//{ +// { +// InterruptLock(1); // this routine will allow level 2 and above +// // do work within interrupt lock here +// } +// do work outside of interrupt lock here outside its scope +//} +// +#define InterruptLock(intrLevel) \ +class _AutoDisableIntr { \ +public: \ + _AutoDisableIntr() { _savedPS = xt_rsil(intrLevel); } \ + ~_AutoDisableIntr() { xt_wsr_ps(_savedPS); } \ +private: \ + uint32_t _savedPS; \ + }; \ +_AutoDisableIntr _autoDisableIntr -#define interrupts() xt_enable_interrupts(interruptsState) -#define noInterrupts() __asm__ __volatile__("rsil %0,15" : "=a" (interruptsState)) #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) diff --git a/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_timer.c b/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_timer.c index 819c5f9901..b130d3c939 100644 --- a/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_timer.c +++ b/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_timer.c @@ -32,7 +32,13 @@ static volatile timercallback timer1_user_cb = NULL; void timer1_isr_handler(void *para){ if ((T1C & ((1 << TCAR) | (1 << TCIT))) == 0) TEIE &= ~TEIE1;//edge int disable T1I = 0; - if (timer1_user_cb) timer1_user_cb(); + if (timer1_user_cb) { + // to make ISR compatible to Arduino AVR model where interrupts are disabled + // we disable them before we call the client ISR + uint32_t savedPS = xt_rsil(15); // stop other interrupts + timer1_user_cb(); + xt_wsr_ps(savedPS); + } } void timer1_isr_init(){ @@ -72,7 +78,11 @@ static volatile timercallback timer0_user_cb = NULL; void timer0_isr_handler(void* para){ if (timer0_user_cb) { + // to make ISR compatible to Arduino AVR model where interrupts are disabled + // we disable them before we call the client ISR + uint32_t savedPS = xt_rsil(15); // stop other interrupts timer0_user_cb(); + xt_wsr_ps(savedPS); } } diff --git a/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring_digital.c b/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring_digital.c index 5be065af51..c64961f03b 100644 --- a/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring_digital.c +++ b/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring_digital.c @@ -123,7 +123,11 @@ void interrupt_handler(void *arg) { if (handler->fn && (handler->mode == CHANGE || (handler->mode & 1) == digitalRead(i))) { + // to make ISR compatible to Arduino AVR model where interrupts are disabled + // we disable them before we call the client ISR + uint32_t savedPS = xt_rsil(15); // stop other interrupts handler->fn(); + xt_wsr_ps(savedPS); } } ETS_GPIO_INTR_ENABLE(); @@ -152,9 +156,6 @@ extern void __detachInterrupt(uint8_t pin) { } } -// stored state for the noInterrupts/interrupts methods -uint32_t interruptsState = 0; - void initPins() { //Disable UART interrupts system_set_os_print(0); diff --git a/hardware/esp8266com/esp8266/libraries/Servo/src/esp8266/Servo.cpp b/hardware/esp8266com/esp8266/libraries/Servo/src/esp8266/Servo.cpp index e9fce77f16..af8bf549e7 100644 --- a/hardware/esp8266com/esp8266/libraries/Servo/src/esp8266/Servo.cpp +++ b/hardware/esp8266com/esp8266/libraries/Servo/src/esp8266/Servo.cpp @@ -59,8 +59,6 @@ static uint8_t s_servoCount = 0; // the total number of attached s_se //------------------------------------------------------------------------------ template void Servo_Handler(T* timer) { - noInterrupts(); - uint8_t servoIndex; // clear interrupt @@ -101,8 +99,6 @@ template void Servo_Handler(T* timer) timer->setEndOfCycle(); } - - interrupts(); } static void initISR(ServoTimerSequence timerId) diff --git a/hardware/esp8266com/esp8266/tools/sdk/include/ets_sys.h b/hardware/esp8266com/esp8266/tools/sdk/include/ets_sys.h index 607d601f18..526a24d40e 100644 --- a/hardware/esp8266com/esp8266/tools/sdk/include/ets_sys.h +++ b/hardware/esp8266com/esp8266/tools/sdk/include/ets_sys.h @@ -65,8 +65,8 @@ inline bool ETS_INTR_WITHINISR() { uint32_t ps; __asm__ __volatile__("rsr %0,ps":"=a" (ps)); - // PS.EXCM bit check - return ((ps & (1 << 4)) != 0); + // PS.INTLEVEL check + return ((ps & 0x0f) != 0); } inline uint32_t ETS_INTR_ENABLED(void) From 5c580a4aad7c3fc80ec98134fdf9a67283f7f5f2 Mon Sep 17 00:00:00 2001 From: Makuna Date: Mon, 3 Aug 2015 19:55:56 -0700 Subject: [PATCH 2/3] make compatible with existing interrupt lock class Support both the normal auto lock at all levels, and the lock at a specific level requiring different syntax --- .../esp8266/cores/esp8266/Arduino.h | 21 ---------- .../esp8266/cores/esp8266/interrupts.h | 38 ++++++++++++++++--- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/hardware/esp8266com/esp8266/cores/esp8266/Arduino.h b/hardware/esp8266com/esp8266/cores/esp8266/Arduino.h index b293bb633e..fa65a2d02e 100644 --- a/hardware/esp8266com/esp8266/cores/esp8266/Arduino.h +++ b/hardware/esp8266com/esp8266/cores/esp8266/Arduino.h @@ -160,27 +160,6 @@ void ets_intr_unlock(); #define interrupts() xt_rsil(0) #define noInterrupts() xt_rsil(15) -// this auto class wraps up xt_rsil so your code can be simplier, but can only be -// used in an ino or cpp files. A normal use pattern is like -// -//{ -// { -// InterruptLock(1); // this routine will allow level 2 and above -// // do work within interrupt lock here -// } -// do work outside of interrupt lock here outside its scope -//} -// -#define InterruptLock(intrLevel) \ -class _AutoDisableIntr { \ -public: \ - _AutoDisableIntr() { _savedPS = xt_rsil(intrLevel); } \ - ~_AutoDisableIntr() { xt_wsr_ps(_savedPS); } \ -private: \ - uint32_t _savedPS; \ - }; \ -_AutoDisableIntr _autoDisableIntr - #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) diff --git a/hardware/esp8266com/esp8266/cores/esp8266/interrupts.h b/hardware/esp8266com/esp8266/cores/esp8266/interrupts.h index d86f89b1f1..78982fe457 100644 --- a/hardware/esp8266com/esp8266/cores/esp8266/interrupts.h +++ b/hardware/esp8266com/esp8266/cores/esp8266/interrupts.h @@ -8,23 +8,51 @@ extern "C" { #include "ets_sys.h" } - -#define xt_disable_interrupts(state, level) __asm__ __volatile__("rsil %0," __STRINGIFY(level) : "=a" (state)) -#define xt_enable_interrupts(state) __asm__ __volatile__("wsr %0,ps; isync" :: "a" (state) : "memory") +// these auto classes wrap up xt_rsil so your code can be simplier, but can only be +// used in an ino or cpp files. + +// InterruptLock is used when you want to completely disable locks +//{ +// { +// InterruptLock lock; +// // do work within interrupt lock here +// } +// do work outside of interrupt lock here outside its scope +//} +// class InterruptLock { public: InterruptLock() { - xt_disable_interrupts(_state, 15); + _state = = xt_rsil(15); } ~InterruptLock() { - xt_enable_interrupts(_state); + xt_wsr_ps(_state); } protected: uint32_t _state; }; +// AutoInterruptLock is when you need to set a specific level, A normal use pattern is like +// +//{ +// { +// AutoInterruptLock(1); // this routine will allow level 2 and above +// // do work within interrupt lock here +// } +// do work outside of interrupt lock here outside its scope +//} +// +#define AutoInterruptLock(intrLevel) \ +class _AutoDisableIntr { \ +public: \ + _AutoDisableIntr() { _savedPS = xt_rsil(intrLevel); } \ + ~_AutoDisableIntr() { xt_wsr_ps(_savedPS); } \ +private: \ + uint32_t _savedPS; \ + }; \ +_AutoDisableIntr _autoDisableIntr #endif //INTERRUPTS_H From 7db662d610093c5c75b350e4d4475ae0133154c9 Mon Sep 17 00:00:00 2001 From: Makuna Date: Mon, 3 Aug 2015 19:58:42 -0700 Subject: [PATCH 3/3] copy paste error fi --- hardware/esp8266com/esp8266/cores/esp8266/interrupts.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hardware/esp8266com/esp8266/cores/esp8266/interrupts.h b/hardware/esp8266com/esp8266/cores/esp8266/interrupts.h index 78982fe457..8139974474 100644 --- a/hardware/esp8266com/esp8266/cores/esp8266/interrupts.h +++ b/hardware/esp8266com/esp8266/cores/esp8266/interrupts.h @@ -24,7 +24,7 @@ extern "C" { class InterruptLock { public: InterruptLock() { - _state = = xt_rsil(15); + _state = xt_rsil(15); } ~InterruptLock() {