Interupts are not always cleared after soft reset (notably GPIO2)

[mac-michael]

After a soft reset when there was an int on GPIO2 mcu can fall into infinite loop.
Probably the GPIO2 is triggered because of the second UART - U1TXD which is transmitting data during boot.

The isr is continuously trigger (esp8266_gpio_isr) but not forwarded into upper layers for processing because it's not enabled at the gpio lib level. Booting is halted and processors is reset by the hardware wdt.

IRAM static void esp8266_gpio_isr(void *arg) {
  uint32_t int_st = GPIO_REG_READ(GPIO_STATUS_ADDRESS);
  for (uint32_t i = 0, mask = 1; i < 16; i++, mask <<= 1) {
    if (!(s_int_config[i] & INT_ENA) || !(int_st & mask)) continue;
    mgos_gpio_hal_int_cb(i);
  }
  (void) arg;
}

[rojer]

good catch!
we should definitely fix this, but i think instead of clearing upon initialization we should disable interrupts before soft reset, since the rate of interrupts may be so high as not to allow the device to boot to the init function.

[mac-michael]

but the int handler is set and enabled in the next lines: _xt_isr_unmask\ETS_INTR_ENABLE
and in these lines exactly the infinite loop starts.

enum mgos_init_result mgos_gpio_hal_init(void) { [...]
  for (int i = 0; i < GPIO_PIN_COUNT; i++) {
    mgos_gpio_hal_clear_int(i);
  }
#ifdef RTOS_SDK
  _xt_isr_attach(ETS_GPIO_INUM, (void *) esp8266_gpio_isr, NULL);
  _xt_isr_unmask(1 << ETS_GPIO_INUM);
#else
  ETS_GPIO_INTR_ATTACH(esp8266_gpio_isr, NULL);
  ETS_INTR_ENABLE(ETS_GPIO_INUM);
#endif

[rojer]

there's gpio interrupt number in the CPU and then there are individual GPIO mode bits for each pin that are configured in mgos_gpio_hal_set_int_mode. all of those need to be set to GPIO_PIN_INTR_DISABLE before soft reset.

[rojer]

this actually raises a larger issue of the state of other peripherals on soft reset...
in fact, we may want to make soft reset less soft, so that hardware gets into known state and is reinitialized.
on other platforms we use reset that actually performs peripheral reset.

[mac-michael]

this actually raises a larger issue of the state of other peripherals on soft reset...

true.
For example, I've empirically checked pwm and see inconsistency there too:

if (mgos_sys_config_get_my_enable())
  mgos_pwm_set(14, 10, 0.5);

I enabled it with config-set --try-once:
a) when I called Sys.Reboot the led stops flashing at the beginning of the boot sequence, but stays on. Sometimes it doesn't - when I add ~5sec delay in the mgos_core_init for example.
b) when hardware reset is done the led stops and do not turn on.

I see that pwm for esp8266 uses hardware timer which has some deinit code before restart.

void mgos_system_restart(void) {
  mgos_event_trigger(MGOS_EVENT_REBOOT, NULL);
  mgos_vfs_umount_all();
  mgos_hw_timers_deinit();
#ifdef MGOS_HAVE_WIFI
  mgos_wifi_disconnect();
  mgos_wifi_deinit();
#endif
  LOG(LL_INFO, ("Restarting"));
  mgos_debug_flush();
  mgos_dev_system_restart();
}

I haven't investigated this any deeper but it looks like some registers are not set up correctly.

Maybe there should be a deinit function for each module or the responsible module should register for MGOS_EVENT_REBOOT and do any necessary cleanup. Or there should be explicit code in mgos_system_restart to reset all peripherals before software reset.
On the other hand it could be a breaking change - I bet that some devices relay on such behavior during a soft reset - for example controlling relays as not to trigger spurious switches.

Maybe there should be two versions of reset available: soft and hard.

Other option is to leave it as it is but disable all the interrupts so no to fall into an infinite loop during startup and leave the responsibility of the correct init on the dev.
Personally I think that the last option should be good enough.
What do you think?

in fact, we may want to make soft reset less soft, so that hardware gets into known state and is reinitialized.
on other platforms we use reset that actually performs peripheral reset.
I've look how it's done for esp32.

void esp_restart(void)
Restart PRO and APP CPUs.
This function can be called both from PRO and APP CPUs. After successful restart, CPU reset reason will be SW_CPU_RESET. Peripherals (except for WiFi, BT, UART0, SPI1, and legacy timers) are not reset. This function does not return.

void mgos_dev_system_restart(void) {
  esp_restart();
}
enum mgos_init_result mgos_gpio_hal_init() {
  /* Soft reset does not clear GPIO_PINn_INT_ENA, we have to do it ourselves. */
  for (int i = 0; i < GPIO_PIN_COUNT; i++) {
    if (GPIO_IS_VALID_GPIO(i)) gpio_intr_disable(i);
  }
  esp_err_t r = gpio_isr_register(esp32_gpio_isr, NULL, 0, &s_int_handle);
  if (r != ESP_OK) return MGOS_INIT_GPIO_INIT_FAILED;
  r = esp_intr_enable(s_int_handle);
  if (r != ESP_OK) return MGOS_INIT_GPIO_INIT_FAILED;
[...]
}

[mac-michael]

One more info:
If we do it at mcu init a call to mgos_gpio_hal_clear_int is needed.
When only mgos_gpio_hal_disable_int is executed an infinite isr loop starts.

enum mgos_init_result mgos_gpio_hal_init(void) {
  // Interupts are not cleared after soft reset (notably GPIO2)
  // Probably the GPIO2 is triggered because of the second UART - U1TXD, 
  // which is transmitting data during boot. Clear everything
  for (int i = 0; i < GPIO_PIN_COUNT; i++) {
    mgos_gpio_hal_disable_int(i);
    mgos_gpio_hal_clear_int(i);
  }

[mac-michael]

I've seen changes in the mongoose os repo but they do not include calling mgos_gpio_hal_clear_int during initialization so I've updated the pull request.