Merge pull request #10699 from dhalbert/spi-sd-race-fixes

finalisers for SPI; clean up sdcardio; reset all pins after finalisers; avoid USB SD races

Author: tannewt

main.c

@@ -43,6 +43,7 @@
 #include "supervisor/shared/external_flash/external_flash.h"
 
 #include "shared-bindings/microcontroller/__init__.h"
+#include "shared-bindings/microcontroller/Pin.h"
 #include "shared-bindings/microcontroller/Processor.h"
 #include "shared-bindings/supervisor/__init__.h"
 #include "shared-bindings/supervisor/Runtime.h"
@@ -124,6 +125,7 @@ static void reset_devices(void) {
 
 static uint8_t *_heap;
 static uint8_t *_pystack;
+static volatile bool _vm_is_running = false;
 
 static const char line_clear[] = "\x1b[2K\x1b[0G";
 
@@ -207,9 +209,12 @@ static void start_mp(safe_mode_t safe_mode) {
 
     // Always return to root
     common_hal_os_chdir("/");
+
+    _vm_is_running = true;
 }
 
 static void stop_mp(void) {
+    _vm_is_running = false;
     #if MICROPY_VFS
 
     // Unmount all heap allocated vfs mounts.
@@ -409,8 +414,13 @@ static void cleanup_after_vm(mp_obj_t exception) {
 
     // Free the heap last because other modules may reference heap memory and need to shut down.
     filesystem_flush();
+
+    // Runs finalisers while shutting down the heap.
     stop_mp();
 
+    // Don't reset pins until finalisers have run.
+    reset_all_pins();
+
     // Let the workflows know we've reset in case they want to restart.
     supervisor_workflow_reset();
 }
@@ -513,6 +523,7 @@ static bool __attribute__((noinline)) run_code_py(safe_mode_t safe_mode, bool *s
 
 
         // Finished executing python code. Cleanup includes filesystem flush and a board reset.
+        _vm_is_running = false;
         cleanup_after_vm(_exec_result.exception);
         _exec_result.exception = NULL;
 
@@ -1201,6 +1212,10 @@ void NORETURN nlr_jump_fail(void *val) {
     }
 }
 
+bool vm_is_running(void) {
+    return _vm_is_running;
+}
+
 #ifndef NDEBUG
 static void NORETURN __fatal_error(const char *msg) {
     #if CIRCUITPY_DEBUG == 0

ports/analog/common-hal/busio/SPI.c

@@ -42,7 +42,6 @@
 typedef enum {
     SPI_FREE = 0,
     SPI_BUSY,
-    SPI_NEVER_RESET,
 } spi_status_t;
 
 // Set each bit to indicate an active SPI
@@ -61,6 +60,9 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self,
     // Check for NULL Pointer
     assert(self);
 
+    // Ensure the object starts in its deinit state.
+    common_hal_busio_spi_mark_deinit(self);
+
     // Assign SPI ID based on pins
     int spi_id = pinsToSpi(mosi, miso, sck);
     if (spi_id == -1) {
@@ -118,15 +120,17 @@ void common_hal_busio_spi_never_reset(busio_spi_obj_t *self) {
     common_hal_never_reset_pin(self->miso);
     common_hal_never_reset_pin(self->sck);
     common_hal_never_reset_pin(self->nss);
-
-    spi_status[self->spi_id] = SPI_NEVER_RESET;
 }
 
 // Check SPI status, deinited or not
 bool common_hal_busio_spi_deinited(busio_spi_obj_t *self) {
     return self->sck == NULL;
 }
 
+void common_hal_busio_spi_mark_deinit(busio_spi_obj_t *self) {
+    self->sck = NULL;
+}
+
 // Deinit SPI obj
 void common_hal_busio_spi_deinit(busio_spi_obj_t *self) {
 
@@ -138,8 +142,9 @@ void common_hal_busio_spi_deinit(busio_spi_obj_t *self) {
 
     self->mosi = NULL;
     self->miso = NULL;
-    self->sck = NULL;
     self->nss = NULL;
+
+    common_hal_busio_spi_mark_deinit(self);
 }
 
 // Configures the SPI bus. The SPI object must be locked.

ports/analog/supervisor/port.c

@@ -166,7 +166,6 @@ void reset_cpu(void) {
 
 // Reset MCU state
 void reset_port(void) {
-    reset_all_pins();
 }
 
 // Reset to the bootloader

ports/atmel-samd/common-hal/busio/SPI.c

@@ -53,7 +53,7 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self,
     }
 
     // Ensure the object starts in its deinit state.
-    self->clock_pin = NO_PIN;
+    common_hal_busio_spi_mark_deinit(self);
 
     // Special case for SAMR21 boards. (feather_radiofruit_zigbee)
     #if defined(PIN_PC19F_SERCOM4_PAD0)
@@ -184,18 +184,24 @@ bool common_hal_busio_spi_deinited(busio_spi_obj_t *self) {
     return self->clock_pin == NO_PIN;
 }
 
+void common_hal_busio_spi_mark_deinit(busio_spi_obj_t *self) {
+    self->clock_pin = NO_PIN;
+}
+
 void common_hal_busio_spi_deinit(busio_spi_obj_t *self) {
     if (common_hal_busio_spi_deinited(self)) {
         return;
     }
     allow_reset_sercom(self->spi_desc.dev.prvt);
 
+    // Mark as deinit early in case we are used in an interrupt.
+    common_hal_busio_spi_mark_deinit(self);
+
     spi_m_sync_disable(&self->spi_desc);
     spi_m_sync_deinit(&self->spi_desc);
     reset_pin_number(self->clock_pin);
     reset_pin_number(self->MOSI_pin);
     reset_pin_number(self->MISO_pin);
-    self->clock_pin = NO_PIN;
 }
 
 bool common_hal_busio_spi_configure(busio_spi_obj_t *self,

ports/atmel-samd/supervisor/port.c

@@ -411,8 +411,6 @@ void reset_port(void) {
         reset_ticks();
     }
 
-    reset_all_pins();
-
     // Output clocks for debugging.
     // not supported by SAMD51G; uncomment for SAMD51J or update for 51G
     // #ifdef SAM_D5X_E5X

ports/broadcom/common-hal/busio/SPI.c

@@ -30,31 +30,8 @@ static SPI0_Type *spi[NUM_SPI] = {SPI0, NULL, NULL};
 static SPI1_Type *aux_spi[NUM_SPI] = {NULL, SPI1, SPI2};
 #endif
 
-static bool never_reset_spi[NUM_SPI];
 static bool spi_in_use[NUM_SPI];
 
-void reset_spi(void) {
-    for (size_t i = 0; i < NUM_SPI; i++) {
-        if (never_reset_spi[i]) {
-            continue;
-        }
-
-        if (i == 1 || i == 2) {
-            if (i == 1) {
-                AUX->ENABLES_b.SPI_1 = false;
-            } else {
-                AUX->ENABLES_b.SPI_2 = false;
-            }
-            aux_spi[i]->CNTL0 = 0;
-        } else {
-            // Set CS back to default. All 0 except read enable.
-            spi[i]->CS = SPI0_CS_REN_Msk;
-        }
-
-        spi_in_use[i] = false;
-    }
-}
-
 void common_hal_busio_spi_construct(busio_spi_obj_t *self,
     const mcu_pin_obj_t *clock, const mcu_pin_obj_t *mosi,
     const mcu_pin_obj_t *miso, bool half_duplex) {
@@ -67,6 +44,9 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self,
         mp_raise_NotImplementedError(MP_ERROR_TEXT("Half duplex SPI is not implemented"));
     }
 
+    // Ensure the object starts in its deinit state.
+    common_hal_busio_spi_mark_deinit(self);
+
     // BCM_VERSION != 2711 have 3 SPI but as listed in peripherals/gen/pins.c two are on
     // index 0, once one index 0 SPI is found the other will throw an invalid_pins error.
     for (size_t i = 0; i < NUM_SPI; i++) {
@@ -118,8 +98,6 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self,
 }
 
 void common_hal_busio_spi_never_reset(busio_spi_obj_t *self) {
-    never_reset_spi[self->index] = true;
-
     common_hal_never_reset_pin(self->clock);
     common_hal_never_reset_pin(self->MOSI);
     common_hal_never_reset_pin(self->MISO);
@@ -129,16 +107,19 @@ bool common_hal_busio_spi_deinited(busio_spi_obj_t *self) {
     return self->clock == NULL;
 }
 
+void common_hal_busio_spi_mark_deinit(busio_spi_obj_t *self) {
+    self->clock = NULL;
+}
+
 void common_hal_busio_spi_deinit(busio_spi_obj_t *self) {
     if (common_hal_busio_spi_deinited(self)) {
         return;
     }
-    never_reset_spi[self->index] = false;
 
     common_hal_reset_pin(self->clock);
     common_hal_reset_pin(self->MOSI);
     common_hal_reset_pin(self->MISO);
-    self->clock = NULL;
+
     spi_in_use[self->index] = false;
 
     if (self->index == 1 ||
@@ -149,7 +130,12 @@ void common_hal_busio_spi_deinit(busio_spi_obj_t *self) {
         } else if (self->index == 2) {
             AUX->ENABLES_b.SPI_2 = false;
         }
+    } else {
+        // Set CS back to default. All 0 except read enable.
+        spi[self->index]->CS = SPI0_CS_REN_Msk;
     }
+
+    common_hal_busio_spi_mark_deinit(self);
 }
 
 bool common_hal_busio_spi_configure(busio_spi_obj_t *self,

ports/broadcom/common-hal/busio/SPI.h

@@ -23,5 +23,3 @@ typedef struct {
     uint8_t bits;
     uint8_t index;
 } busio_spi_obj_t;
-
-void reset_spi(void);

ports/broadcom/supervisor/port.c

@@ -67,7 +67,6 @@ safe_mode_t port_init(void) {
 void reset_port(void) {
     #if CIRCUITPY_BUSIO
     reset_i2c();
-    reset_spi();
     reset_uart();
     #endif
 
@@ -85,8 +84,6 @@ void reset_port(void) {
     #if CIRCUITPY_AUDIOCORE
     audio_dma_reset();
     #endif
-
-    reset_all_pins();
 }
 
 void reset_to_bootloader(void) {

ports/cxd56/common-hal/busio/SPI.c

@@ -54,7 +54,7 @@ void common_hal_busio_spi_deinit(busio_spi_obj_t *self) {
         return;
     }
 
-    self->spi_dev = NULL;
+    common_hal_busio_spi_mark_deinit(self);
 
     reset_pin_number(self->clock_pin->number);
     reset_pin_number(self->mosi_pin->number);
@@ -65,6 +65,10 @@ bool common_hal_busio_spi_deinited(busio_spi_obj_t *self) {
     return self->spi_dev == NULL;
 }
 
+void common_hal_busio_spi_mark_deinit(busio_spi_obj_t *self) {
+    self->spi_dev = NULL;
+}
+
 bool common_hal_busio_spi_configure(busio_spi_obj_t *self, uint32_t baudrate, uint8_t polarity, uint8_t phase, uint8_t bits) {
     uint8_t mode;
 

ports/cxd56/supervisor/port.c

@@ -66,8 +66,6 @@ void reset_port(void) {
     #if CIRCUITPY_RTC
     rtc_reset();
     #endif
-
-    reset_all_pins();
 }
 
 void reset_to_bootloader(void) {