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.
This commit is contained in:
Scott Shawcroft 2021-08-19 12:18:13 -07:00
parent 0569c202ef
commit d2860b58b0
No known key found for this signature in database
GPG Key ID: 0DFD512649C052DA
12 changed files with 62 additions and 25 deletions

View File

@ -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();
}

View File

@ -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,8 +330,10 @@ 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) {
if (!background_callback_pending()) {
xTaskNotifyWait(0x01, 0x01, NULL, portMAX_DELAY);
}
}
// Wrap main in app_main that the IDF expects.
extern void main(void);

View File

@ -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

View File

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

View File

@ -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,11 +401,16 @@ void port_idle_until_interrupt(void) {
__set_FPSCR(__get_FPSCR() & ~(0x9f));
(void)__get_FPSCR();
}
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();
}
/**
* \brief Default interrupt handler for unused IRQs.

View File

@ -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) {
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();
}

View File

@ -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();
}

View File

@ -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,8 +368,12 @@ void port_idle_until_interrupt(void) {
if (stm32_peripherals_rtc_alarm_triggered(PERIPHERALS_ALARM_A)) {
return;
}
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
// -nostdlib/-nostartfiles.

View File

@ -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;

View File

@ -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);

View File

@ -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;

View File

@ -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