From 89c481c606b0afaab2098473ad7d8dd96a54d47b Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 24 Sep 2015 10:17:07 +1000 Subject: [PATCH] Simplify interrupt and RTOS timer tick handlers RTOS Timer tick handler is now the same as any other ISR. This causes a few subtle behaviour changes that seem OK but are worth noting: * RTOS tick handler sdk__xt_timer_int() is now called from one stack frame deeper (inside _xt_isr_handler()), whereas before it was called from the level above in UserHandleInterrupt. I can't see any way that the extra ~40 bytes of stack use here hurt, though. * sdk__xt_timer_int() was previous called after all other interrupts flagged in the handler, now it's called before the TIMER FRC1 & FRC2 handlers. The tick handler doesn't appear to do anything particularly timing intensive, though. * GPIO interrupt (value 3) is now lower priority than the SPI interrupt (value 2), whereas before it would have been called before SPI if both interrupts triggered at once. --- FreeRTOS/Source/portable/esp8266/port.c | 2 + core/esp_interrupts.c | 55 ++++++++----------------- core/exception_vectors.S | 39 +++++------------- core/include/esp/interrupts.h | 3 +- 4 files changed, 31 insertions(+), 68 deletions(-) diff --git a/FreeRTOS/Source/portable/esp8266/port.c b/FreeRTOS/Source/portable/esp8266/port.c index 89c95a1..fc452f0 100644 --- a/FreeRTOS/Source/portable/esp8266/port.c +++ b/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(); diff --git a/core/esp_interrupts.c b/core/esp_interrupts.c index 481011a..4fd68b7 100644 --- a/core/esp_interrupts.c +++ b/core/esp_interrupts.c @@ -14,47 +14,26 @@ void IRAM _xt_isr_attach(uint8_t i, _xt_isr func) isr[i] = func; } -/* This ISR handler is taken directly from the FreeRTOS port and - probably could use a cleanup. +/* Generic ISR handler. + + Handles all flags set for interrupts in 'intset'. */ -uint16_t IRAM _xt_isr_handler(uint16_t i) +uint16_t IRAM _xt_isr_handler(uint16_t intset) { - 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; - } + /* 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); } - _xt_clear_ints(1<