Merge pull request #50 from SuperHouse/fix/interrupts

Fix/interrupts

.dir-locals.el

@@ -6,4 +6,20 @@
   (c-file-style . "bsd")
   (c-basic-offset . 4)
   )
+ (asm-mode
+  (indent-tabs-mode . nil)
+  ; this is basically a hack so asm-mode indents with spaces not tabs
+  ; taken from http://stackoverflow.com/questions/2668563/emacs-indentation-in-asm-mode
+  ; (moving to gas-mode may be a better choice)
+  (tab-stop-list (quote (4 8 12 16 20 24 28 32 36 40 44 48 52 56 60 64 68 72 76 80 84 88 92 96 100 104 108 112 116 120)))
+  (asm-comment-char . "#")
+  )
  )
+
+; IMPORTANT: If you want to write assembly and have indenting to not be infuriating,
+; you probably also want this in your .emacs file:
+;
+; (add-hook 'asm-mode-hook '(lambda () (setq indent-line-function 'indent-relative)))
+;
+; This is not safe to set as a local variable.
+

FreeRTOS/Source/portable/esp8266/port.c

@@ -198,6 +198,8 @@ portBASE_TYPE xPortStartScheduler( void )
     }
 
     /* Initialize system tick timer interrupt and schedule the first tick. */
+    _xt_isr_attach(INUM_TICK, sdk__xt_timer_int);
+    _xt_isr_unmask(BIT(INUM_TICK));
     sdk__xt_tick_timer_init();
 
     vTaskSwitchContext();
@@ -265,60 +267,3 @@ void IRAM vPortExitCritical( void )
 	portENABLE_INTERRUPTS();
 }
 
-/*-----------------------------------------------------------*/
-
-/* Main ISR handler for FreeRTOS side of the ESP libs?
-
-   As far as I can tell, the "real" Xtensa ISRs ("Exceptions") are
-   handled in libmain.a (xtensa_vectors.o) which then can call into here
-   passing an interrupt mask.
-*/
-
-_xt_isr isr[16];
-
-void IRAM _xt_isr_attach(uint8_t i, _xt_isr func)
-{
-    isr[i] = func;
-}
-
-uint16_t IRAM _xt_isr_handler(uint16_t i)
-{
-    uint8_t index;
-
-    /* I think this is implementing some kind of interrupt priority or
-       short-circuiting an expensive ffs for most common interrupts - ie
-       WDT And GPIO are common or high priority, then remaining flags.
-    */
-    if (i & (1 << INUM_WDT)) {
-	index = INUM_WDT;
-    }
-    else if (i & (1 << INUM_GPIO)) {
-	index = INUM_GPIO;
-    }else {
-	index = __builtin_ffs(i) - 1;
-
-	if (index == INUM_MAX) {
-	    /* I don't understand what happens here. INUM_MAX is not
-	       the highest interrupt number listed (and the isr array
-	       has 16 entries).
-
-	       Clearing that flag and then setting index to
-	       __builtin_ffs(i)-1 may result in index == 255 if no
-	       higher flags are set, unless this is guarded against
-	       somehow by the caller?
-
-	       I also don't understand why the code is written like
-	       this in esp_iot_rtos_sdk instead of just putting the i
-	       &= line near the top... Probably no good reason?
-	    */
-	    i &= ~(1 << INUM_MAX);
-	    index = __builtin_ffs(i) - 1;
-	}
-    }
-
-    _xt_clear_ints(1<<index);
-
-    isr[index]();
-
-    return i & ~(1 << index);
-}

FreeRTOS/Source/portable/esp8266/portmacro.h

@@ -73,8 +73,8 @@ extern "C" {
 #include "esp8266.h"
 #include "espressif/esp8266/ets_sys.h"
 #include <stdint.h>
-#include    "xtensa_rtos.h"
+#include "xtensa_rtos.h"
-#include "xtensa_interrupts.h"
+#include <esp/interrupts.h>
 
 /*-----------------------------------------------------------
  * Port specific definitions for ESP8266

README.md

@@ -134,7 +134,17 @@ The best way to write suitable code is to first add documentation somewhere like
 
 ## Coding Style
 
-For new contributions, please use BSD style and indent using 4 spaces. If you're an emacs user then there is a .dir-locals.el file in the root which configures cc-mode.
+For new contributions in C, please use BSD style and indent using 4 spaces.
+
+For assembly, please use the following:
+* Instructions indented using 8 spaces.
+* Inline comments use `#` as a comment delimiter.
+* Comments on their own line(s) use `/*`..`*/`.
+* First operand of each instruction should be vertically aligned where possible.
+* For xtensa special registers, prefer `wsr aX, SR` over `wsr.SR aX`
+
+If you're an emacs user then there is a .dir-locals.el file in the root which configures cc-mode and asm-mode (you will need to approve some variable values as safe). See also
+the additional comments in .dir-locals.el, if you're editing assembly code.
 
 Upstream code is left with the indentation and style of the upstream project.
 

core/esp_interrupts.c

@@ -0,0 +1,39 @@
+/* ESP8266 Xtensa interrupt management functions
+ *
+ *
+ * Part of esp-open-rtos
+ * Copyright (C) 2015 Angus Gratton
+ * BSD Licensed as described in the file LICENSE
+ */
+#include <esp/interrupts.h>
+
+_xt_isr isr[16];
+
+void IRAM _xt_isr_attach(uint8_t i, _xt_isr func)
+{
+    isr[i] = func;
+}
+
+/* Generic ISR handler.
+
+   Handles all flags set for interrupts in 'intset'.
+*/
+uint16_t IRAM _xt_isr_handler(uint16_t intset)
+{
+    /* WDT has highest priority (occasional WDT resets otherwise) */
+    if(intset & BIT(INUM_WDT)) {
+        _xt_clear_ints(BIT(INUM_WDT));
+        isr[INUM_WDT]();
+        intset -= BIT(INUM_WDT);
+    }
+
+    while(intset) {
+        uint8_t index = __builtin_ffs(intset) - 1;
+        uint16_t mask = BIT(index);
+        _xt_clear_ints(mask);
+        isr[index]();
+        intset -= mask;
+    }
+
+    return 0;
+}

core/exception_vectors.S

@@ -25,12 +25,19 @@
 
         .section .bss
 
-NMIHandlerStack:                # stack space for NMI handler
+/* Stack space for NMI handler
-        .skip   4*0x100
-.LNMIHandlerStackTop:
-NMIRegisterSaved:               # register space for saving NMI registers
-        .skip   4*(16 + 6)
 
+   NMI handler stack high water mark measured at 0x134 bytes. Any use
+   of the NMI timer callback will add stack overhead as well.
+
+   The NMI handler does a basic check for stack overflow
+*/
+        .balign 16
+NMIHandlerStack:
+        .skip 0x200
+.NMIHandlerStackTop:
+
+        .balign 16
 LoadStoreErrorHandlerStack:
         .word   0       # a0
         .word   0       # (unused)
@@ -67,10 +74,7 @@ DebugExceptionVector:
         .org    VecBase + 0x20
 NMIExceptionVector:
         .type   NMIExceptionVector, @function
-
+        j   NMIExceptionHandler
-        wsr     a0, excsave3
-        call0   CallNMIExceptionHandler
-        rfi     3  # Should never be reached
 
         .org    VecBase + 0x30
 KernelExceptionVector:
@@ -371,96 +375,103 @@ call_user_start:
 
 /*************************** NMI Exception Handler ***************************/
 
+#define NMI_STACK_CANARY 0xABBABABA
+
         .section .vecbase.text, "x"
 
-/* Save register relative to a0 */
-.macro SAVE_REG register, regnum
-        s32i \register, a0, (4 * (\regnum + 6))
-.endm
-
-/* Load register relative to sp */
-.macro LOAD_REG register, regnum
-        l32i \register, sp, (4 * (\regnum + 6))
-.endm
-
         .literal_position
-
         .balign  16
-CallNMIExceptionHandler:
+NMIExceptionHandler:
-        .type   CallNMIExceptionHandler, @function
+        .type   NMIExceptionHandler, @function
 
-        movi    a0, NMIRegisterSaved
+        wsr     sp, excsave3	# excsave3 holds user stack
-        SAVE_REG a2, 2
+        movi    sp, .NMIHandlerStackTop - 0x40
-        SAVE_REG sp, 1
+        s32i    a0, sp, 0x00
-        SAVE_REG a3, 3
+        s32i    a2, sp, 0x04
-        rsr     a2, excsave3    # a2 is now former a0
+        s32i    a3, sp, 0x08
-        SAVE_REG a4, 4
+        s32i    a4, sp, 0x0c
-        SAVE_REG a2, 0
+        s32i    a5, sp, 0x10
-        rsr     a3, epc1
+        s32i    a6, sp, 0x14
-        rsr     a4, exccause
+        s32i    a7, sp, 0x18
-        SAVE_REG a3, -5
+        s32i    a8, sp, 0x1c
-        SAVE_REG a4, -4
+        s32i    a9, sp, 0x20
-        rsr     a3, excvaddr
+        s32i    a10, sp, 0x24
-        SAVE_REG a3, -3
+        s32i    a11, sp, 0x28
-        rsr     a3, excsave1
+        rsr     a0, epc1
-        SAVE_REG a3, -2
+        s32i    a0, sp, 0x2c
-        SAVE_REG a5, 5
+        rsr     a0, exccause
-        SAVE_REG a6, 6
+        s32i    a0, sp, 0x30
-        SAVE_REG a7, 7
+        rsr     a0, excsave1
-        SAVE_REG a8, 8
+        s32i    a0, sp, 0x34
-        SAVE_REG a9, 9
+        rsr     a0, excvaddr
-        SAVE_REG a10, 10
+        s32i    a0, sp, 0x38
-        SAVE_REG a11, 11
+        rsr     a0, sar
-        SAVE_REG a12, 12
+        s32i    a0, sp, 0x3c
-        SAVE_REG a13, 13
+        movi    a0, 0x23        # Override PS for NMI handler
-        SAVE_REG a14, 14
+        wsr     a0, ps
-        SAVE_REG a15, 15
-        movi    sp, .LNMIHandlerStackTop
-        movi    a0, 0
-        movi    a2, 0x23        # argument for handler
-        wsr     a2, ps
         rsync
-        rsr     a14, sar
+
-        s32i    a14, sp, 0      # this is also NMIRegisterSaved+0
+        /* mark the stack overflow point before we call the actual NMI handler */
+        movi    a0, NMIHandlerStack
+        movi    a2, NMI_STACK_CANARY
+        s32i    a2, a0, 0x00
+
         call0   sdk_wDev_ProcessFiq
-        l32i    a15, sp, 0
+
-        wsr     a15, sar
+        /* verify we didn't overflow */
-        movi    a2, 0x33
+        movi    a0, NMIHandlerStack
-        wsr     a2, ps
+        l32i    a3, a0, 0
+        movi    a2, NMI_STACK_CANARY
+        bne    a3, a2, .NMIFatalStackOverflow
+
+	l32i 	a0, sp, 0x3c
+	wsr	a0, sar
+	l32i	a0, sp, 0x38
+	wsr	a0, excvaddr
+	l32i	a0, sp, 0x34
+	wsr	a0, excsave1
+	l32i	a0, sp, 0x30
+	wsr	a0, exccause
+	l32i	a0, sp, 0x2c
+	wsr	a0, epc1
+	l32i	a11, sp, 0x28
+	l32i 	a10, sp, 0x24
+	l32i	a9, sp, 0x20
+	l32i	a8, sp, 0x1c
+	l32i	a7, sp, 0x18
+	l32i 	a6, sp, 0x14
+	l32i	a5, sp, 0x10
+	l32i	a4, sp, 0x0c
+	l32i	a3, sp, 0x08
+        movi    a0, 0x33    	  # Reset PS
+        wsr     a0, ps
         rsync
-        LOAD_REG a4, 4
+        /* set dport nmi status to 1 (wDev_ProcessFiq clears bit 0 and verifies it
-        LOAD_REG a5, 5
+         * stays cleared, see
-        LOAD_REG a6, 6
-        LOAD_REG a7, 7
-        LOAD_REG a8, 8
-        LOAD_REG a9, 9
-        LOAD_REG a10, 10
-        LOAD_REG a11, 11
-        LOAD_REG a12, 12
-        LOAD_REG a13, 13
-        LOAD_REG a14, 14
-        LOAD_REG a15, 15
-        LOAD_REG a2, -5
-        LOAD_REG a3, -4
-        wsr     a2, epc1
-        wsr     a3, exccause
-        LOAD_REG a2, -3
-        LOAD_REG a3, -2
-        wsr     a2, excvaddr
-        wsr     a3, excsave1
-        LOAD_REG a0, 0
-        /* set dport nmi status bit 0 (wDev_ProcessFiq clears & verifies this
-         * bit stays cleared, see
          * http://esp8266-re.foogod.com/wiki/WDev_ProcessFiq_%28IoT_RTOS_SDK_0.9.9%29)
          */
-        movi    a2, 0x3ff00000
+        movi    a0, 0x3ff00000
-        movi    a3, 0x1
+        movi    a2, 0x1
-        s32i    a3, a2, 0
+        s32i    a2, a0, 0
-        LOAD_REG a2, 2
+	l32i	a2, sp, 0x04
-        LOAD_REG a3, 3
+	l32i	a0, sp, 0x00
-        LOAD_REG a1, 1
+	movi	a1, 0x0
+	xsr	a1, excsave3       # Load stack back from excsave3, clear excsave3
         rfi     3
 
+        .section .rodata
+
+.NMIStackOverflowErrorMsg:
+        .string "\nFATAL: NMI Stack Overflow\n"
+
+        .section .vecbase.text, "x"
+
+.NMIFatalStackOverflow:
+        movi a2, .NMIStackOverflowErrorMsg
+        call0 printf
+.NMIInfiniteLoop:
+        j .NMIInfiniteLoop /* TODO: replace with call to abort() */
+
 /*********************** General UserException Handler ***********************/
 
         .section .vecbase.text, "x"
@@ -469,7 +480,6 @@ CallNMIExceptionHandler:
  * LoadStoreCause. */
 
         .literal_position
-
         .balign  4
 UserExceptionHandler:
         .type   UserExceptionHandler, @function
@@ -490,46 +500,28 @@ UserExceptionHandler:
         wsr     a0, ps
         rsync
         rsr     a2, exccause
-        beqi    a2, CAUSE_LVL1INT, UserHandleInterrupt
+        /* Any UserException cause other than a level 1 interrupt is fatal */
-        /* Any UserException cause other than level 1 interrupt should panic */
+        bnei    a2, CAUSE_LVL1INT, .LUserFailOtherExceptionCause
-UserFailOtherExceptionCause:
+.LUserHandleInterrupt:
-        break   1, 1
-        call0   sdk_user_fatal_exception_handler
-UserHandleInterrupt:
         rsil    a0, 1
         rsr     a2, intenable
         rsr     a3, interrupt
         movi    a4, 0x3fff
         and     a2, a2, a3
-        and     a2, a2, a4      # a2 = 0x3FFF & INTENABLE & INTERRUPT
+        and     a2, a2, a4       # a2 = 0x3FFF & INTENABLE & INTERRUPT
-UserHandleTimer:
-        movi    a3, 0xffbf
-        and     a3, a2, a3      # a3 = a2 with bit 6 cleared
-        bnez    a3, UserTimerDone   # If any non-timer interrupt bits set
-        movi    a3, 0x40
-        sub     a12, a2, a3     # a12 = a2 - 0x40 -- Will be zero if bit 6 set
-        call0   sdk__xt_timer_int  # tick timer interrupt
-        mov     a2, a12         # restore a2 from a12, ie zero
-        beqz    a2, UserIntDone
-UserTimerDone:
         call0   _xt_isr_handler
-        bnez    a2, UserHandleTimer
+        call0   sdk__xt_int_exit # once finished, jumps to _xt_user_exit via stack
-UserIntDone:
+
-        beqz    a2, UserIntExit
+        .literal_position
-        /* FIXME: this code will never be reached */
+.LUserFailOtherExceptionCause:
         break   1, 1
         call0   sdk_user_fatal_exception_handler
-UserIntExit:
-        call0   sdk__xt_int_exit  # jumps to _xt_user_exit. Never returns here
 
-        .section .text
+/* _xt_user_exit is pushed onto the stack as part of the user exception handler,
-
+   restores same set registers which were saved there and returns from exception */
-/* _xt_user_exit is used to exit interrupt context. */
-/* TODO: Find a better place for this to live. */
 _xt_user_exit:
         .global _xt_user_exit
         .type _xt_user_exit, @function
-
         l32i    a0, sp, 0x8
         wsr     a0, ps
         l32i    a0, sp, 0x4
@@ -538,4 +530,3 @@ _xt_user_exit:
         l32i    sp, sp, 0x10
         rsync
         rfe
-

core/include/esp/cpu.h

@@ -1,33 +0,0 @@
-/* esp/cpu.h
- *
- * Details relating to the ESP8266 Xtensa core.
- *
- */
-#ifndef _ESP_CPU_H
-#define _ESP_CPU_H
-#include <stdbool.h>
-
-/* Interrupt numbers for level 1 exception handler.
- *
- * Currently the UserExceptionVector calls down to _xt_isr_handler,
- * defined in port.c, for at least some of these interrupts. Some are handled
- * on the SDK side, though.
- */
-typedef enum {
-    INUM_SPI = 2,
-    INUM_GPIO = 4,
-    INUM_UART = 5,
-    INUM_MAX = 6, /* in some places this is documented as timer0 CCOMPARE0 interrupt */
-    INUM_SOFT = 7,
-    INUM_WDT = 8,
-    INUM_TIMER_FRC1 = 9,
-
-    /* FRC2 default handler. Configured by sdk_ets_timer_init, which
-       runs as part of default libmain.a startup code, assigns
-       interrupt handler to sdk_vApplicationTickHook+0x68
-     */
-    INUM_TIMER_FRC2 = 10,
-} xt_isr_num_t;
-
-#endif
-

core/include/esp/gpio.h

@@ -11,8 +11,7 @@
 #include <stdbool.h>
 #include "esp/gpio_regs.h"
 #include "esp/iomux.h"
-#include "esp/cpu.h"
+#include "esp/interrupts.h"
-#include "xtensa_interrupts.h"
 
 typedef enum {
     GPIO_INPUT,

core/include/esp/interrupts.h

@@ -1,4 +1,4 @@
-/* Xtensa interrupt management functions
+/* ESP8266 Xtensa interrupt management functions
  *
  * Some (w/ sdk_ prefix) are implemented in binary libs, rest are
  * inlines replacing functions in the binary libraries.
@@ -15,9 +15,27 @@
 #include <xtensa/hal.h>
 #include <common_macros.h>
 
+/* Interrupt numbers for level 1 exception handler. */
+typedef enum {
+    INUM_SPI = 2,
+    INUM_GPIO = 4,
+    INUM_UART = 5,
+    INUM_TICK = 6, /* RTOS timer tick, possibly xtensa CPU CCOMPARE0(?) */
+    INUM_SOFT = 7,
+    INUM_WDT = 8,
+    INUM_TIMER_FRC1 = 9,
+
+    /* FRC2 default handler. Configured by sdk_ets_timer_init, which
+       runs as part of default libmain.a startup code, assigns
+       interrupt handler to sdk_vApplicationTickHook+0x68
+     */
+    INUM_TIMER_FRC2 = 10,
+} xt_isr_num_t;
+
 void sdk__xt_int_exit (void);
 void _xt_user_exit (void);
 void sdk__xt_tick_timer_init (void);
+void sdk__xt_timer_int(void);
 void sdk__xt_timer_int1(void);
 
 INLINED uint32_t _xt_get_intlevel(void)

core/include/esp/timer.h

@@ -10,9 +10,8 @@
 #define _ESP_TIMER_H
 
 #include <stdbool.h>
-#include <xtensa_interrupts.h>
 #include "esp/timer_regs.h"
-#include "esp/cpu.h"
+#include "esp/interrupts.h"
 
 #ifdef	__cplusplus
 extern "C" {

core/include/esp8266.h

@@ -13,7 +13,7 @@
 
 #include "common_macros.h"
 #include "esp/registers.h"
-#include "esp/cpu.h"
+#include "esp/interrupts.h"
 #include "esp/iomux.h"
 #include "esp/gpio.h"
 #include "esp/timer.h"