Check background pending before sleep

There is a race between when we run background tasks and when we
sleep. If an interrupt happens between the two, then we may delay
executing the background task. On some ports we checked this for
TinyUSB already. On iMX RT, we didn't which caused USB issues.
This PR makes it more generic for all background tasks including
USB.

Fixes #5086 and maybe others.

Author: tannewt

ports/atmel-samd/supervisor/port.c

@@ -80,12 +80,11 @@
 #include "shared_timers.h"
 #include "reset.h"
 
+#include "supervisor/background_callback.h"
 #include "supervisor/shared/safe_mode.h"
 #include "supervisor/shared/stack.h"
 #include "supervisor/shared/tick.h"
 
-#include "tusb.h"
-
 #if CIRCUITPY_GAMEPADSHIFT
 #include "shared-module/gamepadshift/__init__.h"
 #endif
@@ -628,7 +627,7 @@ void port_idle_until_interrupt(void) {
     }
     #endif
     common_hal_mcu_disable_interrupts();
-    if (!tud_task_event_ready() && sleep_ok && !_woken_up) {
+    if (!background_callback_pending() && sleep_ok && !_woken_up) {
         __DSB();
         __WFI();
     }

ports/esp32s2/supervisor/port.c

@@ -48,6 +48,7 @@
 #include "common-hal/watchdog/WatchDogTimer.h"
 #include "common-hal/socketpool/Socket.h"
 #include "common-hal/wifi/__init__.h"
+#include "supervisor/background_callback.h"
 #include "supervisor/memory.h"
 #include "supervisor/shared/tick.h"
 #include "shared-bindings/rtc/__init__.h"
@@ -329,7 +330,9 @@ void port_interrupt_after_ticks(uint32_t ticks) {
 // On the ESP we use FreeRTOS notifications instead of interrupts so this is a
 // bit of a misnomer.
 void port_idle_until_interrupt(void) {
-    xTaskNotifyWait(0x01, 0x01, NULL, portMAX_DELAY);
+    if (!background_callback_pending()) {
+        xTaskNotifyWait(0x01, 0x01, NULL, portMAX_DELAY);
+    }
 }
 
 // Wrap main in app_main that the IDF expects.

ports/mimxrt10xx/Makefile

@@ -81,11 +81,13 @@ CFLAGS += -Os -ftree-vrp -DNDEBUG -ffreestanding
 CFLAGS += -DCFG_TUSB_MCU=OPT_MCU_MIMXRT10XX -DCFG_TUD_MIDI_RX_BUFSIZE=512 -DCFG_TUD_CDC_RX_BUFSIZE=512 -DCFG_TUD_MIDI_TX_BUFSIZE=512 -DCFG_TUD_CDC_TX_BUFSIZE=512 -DCFG_TUD_MSC_BUFSIZE=1024
 
 #Debugging/Optimization
+# Never set -fno-inline because we use inline to move small functions into routines that must be
+# in RAM. If inlining is disallowed, then we may end up calling a function in flash when we cannot.
 ifeq ($(DEBUG), 1)
   # You may want to disable -flto if it interferes with debugging.
   # CFLAGS += -flto -flto-partition=none
   # You may want to enable these flags to make setting breakpoints easier.
-  CFLAGS += -fno-inline -fno-ipa-sra
+  CFLAGS += -fno-ipa-sra
 else
   #CFLAGS += -flto -flto-partition=none
 endif

ports/mimxrt10xx/linking/common.ld

@@ -51,7 +51,6 @@ SECTIONS
         KEEP(* (.boot_hdr.dcd_data))
         . = ALIGN(4);
     } > FLASH_IVT
-    image_vector_table = LOADADDR(.ivt);
 
     .text :
     {

ports/mimxrt10xx/supervisor/port.c

@@ -40,10 +40,11 @@
 #include "common-hal/pwmio/PWMOut.h"
 #include "common-hal/rtc/RTC.h"
 #include "common-hal/busio/SPI.h"
+#include "shared-bindings/microcontroller/__init__.h"
 
 #include "reset.h"
 
-#include "tusb.h"
+#include "supervisor/background_callback.h"
 
 #if CIRCUITPY_GAMEPADSHIFT
 #include "shared-module/gamepadshift/__init__.h"
@@ -400,10 +401,15 @@ void port_idle_until_interrupt(void) {
         __set_FPSCR(__get_FPSCR() & ~(0x9f));
         (void)__get_FPSCR();
     }
-    NVIC_ClearPendingIRQ(SNVS_HP_WRAPPER_IRQn);
-    CLOCK_SetMode(kCLOCK_ModeWait);
-    __WFI();
-    CLOCK_SetMode(kCLOCK_ModeRun);
+
+    common_hal_mcu_disable_interrupts();
+    if (!background_callback_pending()) {
+        NVIC_ClearPendingIRQ(SNVS_HP_WRAPPER_IRQn);
+        CLOCK_SetMode(kCLOCK_ModeWait);
+        __WFI();
+        CLOCK_SetMode(kCLOCK_ModeRun);
+    }
+    common_hal_mcu_enable_interrupts();
 }
 
 /**

ports/nrf/supervisor/port.c

@@ -25,8 +25,10 @@
  * THE SOFTWARE.
  */
 
-#include <stdint.h>
 #include "supervisor/port.h"
+
+#include <stdint.h>
+#include "supervisor/background_callback.h"
 #include "supervisor/board.h"
 
 #include "nrfx/hal/nrf_clock.h"
@@ -39,6 +41,8 @@
 #include "nrf/power.h"
 #include "nrf/timers.h"
 
+#include "nrf_nvic.h"
+
 #include "common-hal/microcontroller/Pin.h"
 #include "common-hal/_bleio/__init__.h"
 #include "common-hal/analogio/AnalogIn.h"
@@ -360,7 +364,12 @@ void port_idle_until_interrupt(void) {
 
     sd_softdevice_is_enabled(&sd_enabled);
     if (sd_enabled) {
-        sd_app_evt_wait();
+        uint8_t is_nested_critical_region;
+        sd_nvic_critical_region_enter(&is_nested_critical_region);
+        if (!background_callback_pending()) {
+            sd_app_evt_wait();
+        }
+        sd_nvic_critical_region_exit(is_nested_critical_region);
     } else {
         // Call wait for interrupt ourselves if the SD isn't enabled.
         // Note that `wfi` should be called with interrupts disabled,
@@ -376,11 +385,7 @@ void port_idle_until_interrupt(void) {
         // function (whether or not SD is enabled)
         int nested = __get_PRIMASK();
         __disable_irq();
-        bool ok = true;
-        #if CIRCUITPY_USB
-        ok = !tud_task_event_ready();
-        #endif
-        if (ok) {
+        if (!background_callback_pending()) {
             __DSB();
             __WFI();
         }

ports/raspberrypi/supervisor/port.c

@@ -27,6 +27,7 @@
 #include <string.h>
 #include <stdlib.h>
 
+#include "supervisor/background_callback.h"
 #include "supervisor/board.h"
 #include "supervisor/port.h"
 
@@ -54,8 +55,6 @@
 #include "src/common/pico_time/include/pico/time.h"
 #include "src/common/pico_binary_info/include/pico/binary_info.h"
 
-#include "tusb.h"
-
 #include "pico/bootrom.h"
 #include "hardware/watchdog.h"
 
@@ -233,7 +232,7 @@ void port_interrupt_after_ticks(uint32_t ticks) {
 
 void port_idle_until_interrupt(void) {
     common_hal_mcu_disable_interrupts();
-    if (!tud_task_event_ready()) {
+    if (!background_callback_pending()) {
 //	asm volatile ("dsb 0xF":::"memory");
 //        __wfi();
     }

ports/stm/supervisor/port.c

@@ -26,11 +26,13 @@
  */
 
 #include <stdint.h>
-#include "supervisor/port.h"
+#include "supervisor/background_callback.h"
 #include "supervisor/board.h"
+#include "supervisor/port.h"
 #include "lib/timeutils/timeutils.h"
 
 #include "common-hal/microcontroller/Pin.h"
+#include "shared-bindings/microcontroller/__init__.h"
 
 #ifdef CIRCUITPY_AUDIOPWMIO
 #include "common-hal/audiopwmio/PWMAudioOut.h"
@@ -366,7 +368,11 @@ void port_idle_until_interrupt(void) {
     if (stm32_peripherals_rtc_alarm_triggered(PERIPHERALS_ALARM_A)) {
         return;
     }
-    __WFI();
+    common_hal_mcu_disable_interrupts();
+    if (!background_callback_pending()) {
+        __WFI();
+    }
+    common_hal_mcu_enable_interrupts();
 }
 
 // Required by __libc_init_array in startup code if we are compiling using

shared-bindings/storage/__init__.c

@@ -165,7 +165,11 @@ MP_DEFINE_CONST_FUN_OBJ_0(storage_erase_filesystem_obj, storage_erase_filesystem
 //|     ...
 //|
 STATIC mp_obj_t storage_disable_usb_drive(void) {
+    #if CIRCUITPY_USB_MSC
     if (!common_hal_storage_disable_usb_drive()) {
+    #else
+    if (true) {
+        #endif
         mp_raise_RuntimeError(translate("Cannot change USB devices now"));
     }
     return mp_const_none;
@@ -186,7 +190,11 @@ MP_DEFINE_CONST_FUN_OBJ_0(storage_disable_usb_drive_obj, storage_disable_usb_dri
 //|     ...
 //|
 STATIC mp_obj_t storage_enable_usb_drive(void) {
+    #if CIRCUITPY_USB_MSC
     if (!common_hal_storage_enable_usb_drive()) {
+    #else
+    if (true) {
+        #endif
         mp_raise_RuntimeError(translate("Cannot change USB devices now"));
     }
     return mp_const_none;

supervisor/background_callback.h

@@ -27,6 +27,8 @@
 #ifndef CIRCUITPY_INCLUDED_SUPERVISOR_BACKGROUND_CALLBACK_H
 #define CIRCUITPY_INCLUDED_SUPERVISOR_BACKGROUND_CALLBACK_H
 
+#include <stdbool.h>
+
 /** Background callbacks are a linked list of tasks to call in the background.
  *
  * Include a member of type `background_callback_t` inside an object
@@ -69,6 +71,10 @@ void background_callback_add(background_callback_t *cb, background_callback_fun
  * whenever the list is non-empty */
 void background_callback_run_all(void);
 
+/* True when a background callback is pending. Helpful for checking background state when
+ * interrupts are disabled. */
+bool background_callback_pending(void);
+
 /* During soft reset, remove all pending callbacks and clear the critical section flag */
 void background_callback_reset(void);
 

supervisor/shared/background_callback.c

@@ -67,9 +67,13 @@ void background_callback_add(background_callback_t *cb, background_callback_fun
     background_callback_add_core(cb);
 }
 
+bool PLACE_IN_ITCM(background_callback_pending)(void) {
+    return callback_head != NULL;
+}
+
 static bool in_background_callback;
 void PLACE_IN_ITCM(background_callback_run_all)() {
-    if (!callback_head) {
+    if (!background_callback_pending()) {
         return;
     }
     CALLBACK_CRITICAL_BEGIN;

supervisor/shared/usb/usb.c

@@ -91,7 +91,7 @@ void usb_init(void) {
 
 // Set up USB defaults before any USB changes are made in boot.py
 void usb_set_defaults(void) {
-    #if CIRCUITPY_STORAGE
+    #if CIRCUITPY_STORAGE && CIRCUITPY_USB_MSC
     storage_usb_set_defaults();
     #endif