esp8266: NMI not disabled when timer1 ISR cleared

Sming/Arch/Esp8266/Components/driver/hw_timer.cpp

@@ -36,6 +36,28 @@ static void IRAM_ATTR nmi_handler()
 	nmi_callback.func(nmi_callback.arg);
 }
 
+/*
+ * The `ETS_FRC_TIMER1_NMI_INTR_ATTACH` macro calls SDK `NmiTimSetFunc` which
+ * doesn't actually disable NMI (a known bug).
+ * If we subsequently enable FRC interrupts, the timer won't work so we need to properly
+ * disable NMI manually.
+ *
+ * The NmiTimSetFunc code looks like this:
+ *
+ * 		uint32_t value = REG_READ(NMI_INT_ENABLE_REG);
+ * 		value &= ~0x1f;
+ * 		value |= 0x0f;
+ * 		REG_WRITE(NMI_INT_ENABLE_REG, value);
+ *
+ * Note that there is no published documentation for this register.
+ * Clearing it to zero appears to work but may have unintended side-effects.
+ */
+static void IRAM_ATTR hw_timer1_disable_nmi()
+{
+	auto value = REG_READ(NMI_INT_ENABLE_REG);
+	REG_WRITE(NMI_INT_ENABLE_REG, value & ~0x1f);
+}
+
 void hw_timer1_attach_interrupt(hw_timer_source_type_t source_type, hw_timer_callback_t callback, void* arg)
 {
 	if(source_type == TIMER_NMI_SOURCE) {
@@ -47,10 +69,18 @@ void hw_timer1_attach_interrupt(hw_timer_source_type_t source_type, hw_timer_cal
 			ETS_FRC_TIMER1_NMI_INTR_ATTACH(nmi_handler);
 		}
 	} else {
+		hw_timer1_disable_nmi();
 		ETS_FRC_TIMER1_INTR_ATTACH(callback, arg);
 	}
 }
 
+void IRAM_ATTR hw_timer1_detach_interrupt(void)
+{
+	hw_timer1_disable();
+	hw_timer1_disable_nmi();
+	ETS_FRC_TIMER1_INTR_ATTACH(NULL, NULL);
+}
+
 void hw_timer_init(void)
 {
 	constexpr uint32_t FRC2_ENABLE_TIMER = BIT7;

Sming/Arch/Esp8266/Components/driver/include/driver/hw_timer.h

@@ -123,12 +123,7 @@ __forceinline void IRAM_ATTR hw_timer1_disable(void)
 /**
  * @brief Detach interrupt from the timer
  */
-__forceinline void IRAM_ATTR hw_timer1_detach_interrupt(void)
-{
-	hw_timer1_disable();
-	ETS_FRC_TIMER1_NMI_INTR_ATTACH(NULL);
-	ETS_FRC_TIMER1_INTR_ATTACH(NULL, NULL);
-}
+void hw_timer1_detach_interrupt(void);
 
 /**
  * @brief Get timer1 count

Sming/Arch/Esp8266/Components/esp8266/include/espinc/eagle_soc.h

@@ -126,6 +126,7 @@
 //}}
 
 //Interrupt remap control registers define{{
+#define NMI_INT_ENABLE_REG          (PERIPHS_DPORT_BASEADDR)
 #define EDGE_INT_ENABLE_REG         (PERIPHS_DPORT_BASEADDR + 0x04)
 #define WDT_EDGE_INT_ENABLE()       SET_PERI_REG_MASK(EDGE_INT_ENABLE_REG, BIT0)
 #define TM1_EDGE_INT_ENABLE()       SET_PERI_REG_MASK(EDGE_INT_ENABLE_REG, BIT1)

tests/HostTests/modules/Timers.cpp

@@ -79,9 +79,6 @@ class CallbackTimerTest : public TestGroup
 				statusTimer.stop();
 				Serial.print("statusTimer stopped: ");
 				Serial.println(statusTimer);
-
-				// Release any allocated delegate memory
-				timer64.setCallback(TimerDelegate(nullptr));
 				done = true;
 			}
 
@@ -113,17 +110,6 @@ class CallbackTimerTest : public TestGroup
 
 		Serial.println(statusTimer);
 
-		{
-			auto tmp = new Timer;
-			tmp->initializeMs<1200>(
-				[](void* arg) {
-					auto self = static_cast<CallbackTimerTest*>(arg);
-					Serial << self->timer64 << _F(" fired") << endl;
-				},
-				this);
-			tmp->startOnce();
-		}
-
 		if(1) {
 			longTimer.setCallback([this]() {
 				--activeTimerCount;
@@ -222,7 +208,6 @@ class CallbackTimerTest : public TestGroup
 private:
 	Timer statusTimer;
 	unsigned statusTimerCount = 0;
-	Timer timer64;
 	HardwareTimerTest timer1;
 	Timer longTimer;
 	uint32_t longStartTicks = 0;