Merge pull request adafruit#10887 from dhalbert/fix-sdcard-spi-share

tannewt · web-flow · commit d1255a5ac020 · 2026-03-13T14:55:29.000-07:00
USB SDCard handling improvements and related changes
diff --git a/ports/espressif/background.c b/ports/espressif/background.c
@@ -13,8 +13,8 @@
 #include "freertos/task.h"
 
 void port_background_tick(void) {
-    // Zero delay in case FreeRTOS wants to switch to something else.
-    vTaskDelay(0);
+    // Yield with zero delay in case FreeRTOS wants to switch to something else.
+    port_task_yield();
 }
 
 void port_background_task(void) {
diff --git a/ports/espressif/common-hal/busio/SPI.c b/ports/espressif/common-hal/busio/SPI.c
@@ -72,11 +72,6 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self,
         mp_raise_ValueError(MP_ERROR_TEXT("All SPI peripherals are in use"));
     }
 
-    self->mutex = xSemaphoreCreateMutex();
-    if (self->mutex == NULL) {
-        mp_raise_RuntimeError(MP_ERROR_TEXT("Unable to create lock"));
-    }
-
     esp_err_t result = spi_bus_initialize(self->host_id, &bus_config, SPI_DMA_CH_AUTO);
     if (result == ESP_ERR_NO_MEM) {
         mp_raise_msg(&mp_type_MemoryError, MP_ERROR_TEXT("ESP-IDF memory allocation failed"));
@@ -162,11 +157,17 @@ bool common_hal_busio_spi_configure(busio_spi_obj_t *self,
     return true;
 }
 
-bool common_hal_busio_spi_try_lock(busio_spi_obj_t *self) {
+// Wait as long as needed for the lock. This is used by SD card access from USB.
+// Overrides the default busy-wait implementation in shared-bindings/busio/SPI.c
+bool common_hal_busio_spi_wait_for_lock(busio_spi_obj_t *self, uint32_t timeout_ms) {
     if (common_hal_busio_spi_deinited(self)) {
         return false;
     }
-    return xSemaphoreTake(self->mutex, 1) == pdTRUE;
+    return xSemaphoreTake(self->mutex, pdMS_TO_TICKS(timeout_ms)) == pdTRUE;
+}
+
+bool common_hal_busio_spi_try_lock(busio_spi_obj_t *self) {
+    return common_hal_busio_spi_wait_for_lock(self, 0);
 }
 
 bool common_hal_busio_spi_has_lock(busio_spi_obj_t *self) {
diff --git a/ports/espressif/supervisor/port.c b/ports/espressif/supervisor/port.c
@@ -251,12 +251,12 @@ safe_mode_t port_init(void) {
     #define pin_GPIOn(n) pin_GPIO##n
     #define pin_GPIOn_EXPAND(x) pin_GPIOn(x)
 
-    #ifdef CONFIG_CONSOLE_UART_TX_GPIO
-    common_hal_never_reset_pin(&pin_GPIOn_EXPAND(CONFIG_CONSOLE_UART_TX_GPIO));
+    #ifdef CONFIG_ESP_CONSOLE_UART_TX_GPIO
+    common_hal_never_reset_pin(&pin_GPIOn_EXPAND(CONFIG_ESP_CONSOLE_UART_TX_GPIO));
     #endif
 
-    #ifdef CONFIG_CONSOLE_UART_RX_GPIO
-    common_hal_never_reset_pin(&pin_GPIOn_EXPAND(CONFIG_CONSOLE_UART_RX_GPIO));
+    #ifdef CONFIG_ESP_CONSOLE_UART_RX_GPIO
+    common_hal_never_reset_pin(&pin_GPIOn_EXPAND(CONFIG_ESP_CONSOLE_UART_RX_GPIO));
     #endif
 
     #ifndef ENABLE_JTAG
@@ -404,8 +404,8 @@ void reset_port(void) {
     watchdog_reset();
     #endif
 
-    // Yield so the idle task can run and do any IDF cleanup needed.
-    port_yield();
+    // Yield so the idle task, at priority 0, can run and do any IDF cleanup needed.
+    port_task_sleep_ms(4);
 }
 
 void reset_to_bootloader(void) {
@@ -483,8 +483,13 @@ void port_wake_main_task_from_isr(void) {
     }
 }
 
-void port_yield(void) {
-    vTaskDelay(4);
+// Yield to other tasks at the same priority.
+void port_task_yield(void) {
+    vTaskDelay(0);
+}
+
+void port_task_sleep_ms(uint32_t msecs) {
+    vTaskDelay(pdMS_TO_TICKS(msecs));
 }
 
 void sleep_timer_cb(void *arg) {
diff --git a/ports/espressif/supervisor/usb.c b/ports/espressif/supervisor/usb.c
@@ -56,7 +56,8 @@ static void usb_device_task(void *param) {
             tud_task();
             tud_cdc_write_flush();
         }
-        vTaskDelay(1);
+        // Yield with zero delay to switch to any other tasks at same priority.
+        port_task_yield();
     }
 }
 #endif // CIRCUITPY_USB_DEVICE
@@ -112,7 +113,7 @@ void init_usb_hardware(void) {
         "usbd",
         USBD_STACK_SIZE,
         NULL,
-        5,
+        1,
         usb_device_stack,
         &usb_device_taskdef,
         xPortGetCoreID());
diff --git a/ports/raspberrypi/supervisor/port.c b/ports/raspberrypi/supervisor/port.c
@@ -600,7 +600,7 @@ __attribute__((used)) void __not_in_flash_func(isr_hardfault)(void) {
     }
 }
 
-void port_yield(void) {
+void port_task_yield(void) {
     #if CIRCUITPY_CYW43
     cyw43_arch_poll();
     #endif
diff --git a/ports/renode/supervisor/port.c b/ports/renode/supervisor/port.c
@@ -210,7 +210,7 @@ __attribute__((used)) void HardFault_Handler(void) {
     }
 }
 
-void port_yield(void) {
+void port_task_yield(void) {
 }
 
 void port_boot_info(void) {
diff --git a/ports/zephyr-cp/supervisor/port.c b/ports/zephyr-cp/supervisor/port.c
@@ -170,10 +170,14 @@ void port_wake_main_task_from_isr(void) {
     k_event_set(&main_needed, 1);
 }
 
-void port_yield(void) {
+void port_task_yield(void) {
     k_yield();
 }
 
+void port_task_sleep_ms(uint32_t msecs) {
+    k_msleep(msecs);
+}
+
 void port_boot_info(void) {
 }
 
diff --git a/py/circuitpy_defns.mk b/py/circuitpy_defns.mk
@@ -201,6 +201,9 @@ endif
 ifeq ($(CIRCUITPY_DISPLAYIO),1)
 SRC_PATTERNS += displayio/%
 endif
+ifeq ($(CIRCUITPY_DOTCLOCKFRAMEBUFFER),1)
+SRC_PATTERNS += dotclockframebuffer/%
+endif
 ifeq ($(CIRCUITPY_DUALBANK),1)
 SRC_PATTERNS += dualbank/%
 endif
@@ -228,9 +231,6 @@ endif
 ifeq ($(CIRCUITPY_FOURWIRE),1)
 SRC_PATTERNS += fourwire/%
 endif
-ifeq ($(CIRCUITPY_QSPIBUS),1)
-SRC_PATTERNS += qspibus/%
-endif
 ifeq ($(CIRCUITPY_FRAMEBUFFERIO),1)
 SRC_PATTERNS += framebufferio/%
 endif
@@ -357,8 +357,8 @@ endif
 ifeq ($(CIRCUITPY_RGBMATRIX),1)
 SRC_PATTERNS += rgbmatrix/%
 endif
-ifeq ($(CIRCUITPY_DOTCLOCKFRAMEBUFFER),1)
-SRC_PATTERNS += dotclockframebuffer/%
+ifeq ($(CIRCUITPY_QSPIBUS),1)
+SRC_PATTERNS += qspibus/%
 endif
 ifeq ($(CIRCUITPY_RP2PIO),1)
 SRC_PATTERNS += rp2pio/%
diff --git a/py/mpconfig.h b/py/mpconfig.h
@@ -2205,6 +2205,12 @@ typedef double mp_float_t;
 #define MP_INLINE inline MP_NO_INSTRUMENT
 #endif
 
+// CIRCUITPY-CHANGE
+// Modifier for functions whose return value should not be ignored
+#ifndef MP_WARN_UNUSED_RESULT
+#define MP_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
+#endif
+
 // Modifier for functions which should be never inlined
 #ifndef MP_NOINLINE
 #define MP_NOINLINE __attribute__((noinline))
diff --git a/shared-bindings/busdisplay/BusDisplay.c b/shared-bindings/busdisplay/BusDisplay.c
@@ -122,7 +122,6 @@
 //|         :param int native_frames_per_second: Number of display refreshes per second that occur with the given init_sequence.
 //|         :param bool backlight_on_high: If True, pulling the backlight pin high turns the backlight on.
 //|         :param bool SH1107_addressing: Special quirk for SH1107, use upper/lower column set and page set
-//|         :param int set_vertical_scroll: This parameter is accepted but ignored for backwards compatibility. It will be removed in a future release.
 //|         :param int backlight_pwm_frequency: The frequency to use to drive the PWM for backlight brightness control. Default is 50000.
 //|         """
 //|         ...
@@ -133,7 +132,7 @@ static mp_obj_t busdisplay_busdisplay_make_new(const mp_obj_type_t *type, size_t
            ARG_rotation, ARG_color_depth, ARG_grayscale, ARG_pixels_in_byte_share_row,
            ARG_bytes_per_cell, ARG_reverse_pixels_in_byte, ARG_reverse_bytes_in_word,
            ARG_set_column_command, ARG_set_row_command, ARG_write_ram_command,
-           ARG_set_vertical_scroll, ARG_backlight_pin, ARG_brightness_command,
+           ARG_backlight_pin, ARG_brightness_command,
            ARG_brightness, ARG_single_byte_bounds, ARG_data_as_commands,
            ARG_auto_refresh, ARG_native_frames_per_second, ARG_backlight_on_high,
            ARG_SH1107_addressing, ARG_backlight_pwm_frequency };
@@ -154,7 +153,6 @@ static mp_obj_t busdisplay_busdisplay_make_new(const mp_obj_type_t *type, size_t
         { MP_QSTR_set_column_command, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 0x2a} },
         { MP_QSTR_set_row_command, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 0x2b} },
         { MP_QSTR_write_ram_command, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 0x2c} },
-        { MP_QSTR_set_vertical_scroll, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 0x0} },
         { MP_QSTR_backlight_pin, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none} },
         { MP_QSTR_brightness_command, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = NO_BRIGHTNESS_COMMAND} },
         { MP_QSTR_brightness, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = MP_OBJ_NEW_SMALL_INT(1)} },
diff --git a/shared-bindings/busio/SPI.c b/shared-bindings/busio/SPI.c
@@ -9,17 +9,17 @@
 
 #include <string.h>
 
+#include "py/binary.h"
+#include "py/mperrno.h"
+#include "py/objproperty.h"
+#include "py/runtime.h"
 #include "shared-bindings/microcontroller/Pin.h"
 #include "shared-bindings/busio/SPI.h"
 #include "shared-bindings/util.h"
-
 #include "shared/runtime/buffer_helper.h"
 #include "shared/runtime/context_manager_helpers.h"
-#include "py/binary.h"
-#include "py/mperrno.h"
-#include "py/objproperty.h"
-#include "py/runtime.h"
-
+#include "shared/runtime/interrupt_char.h"
+#include "supervisor/shared/tick.h"
 
 //| class SPI:
 //|     """A 3-4 wire serial protocol
@@ -494,3 +494,17 @@ MP_DEFINE_CONST_OBJ_TYPE(
 busio_spi_obj_t *validate_obj_is_spi_bus(mp_obj_t obj, qstr arg_name) {
     return mp_arg_validate_type(obj, &busio_spi_type, arg_name);
 }
+
+// Wait as long as needed for the lock. This is used by SD card access from USB.
+// The default implementation is to busy-wait while running the background tasks. espressif is different.
+bool common_hal_busio_spi_wait_for_lock(busio_spi_obj_t *self, uint32_t timeout_ms) {
+    uint64_t deadline = supervisor_ticks_ms64() + timeout_ms;
+    while (supervisor_ticks_ms64() < deadline &&
+           !mp_hal_is_interrupted()) {
+        if (common_hal_busio_spi_try_lock(self)) {
+            return true;
+        }
+        RUN_BACKGROUND_TASKS;
+    }
+    return false;
+}
diff --git a/shared-bindings/busio/SPI.h b/shared-bindings/busio/SPI.h
@@ -54,3 +54,7 @@ uint8_t common_hal_busio_spi_get_polarity(busio_spi_obj_t *self);
 extern void common_hal_busio_spi_never_reset(busio_spi_obj_t *self);
 
 extern busio_spi_obj_t *validate_obj_is_spi_bus(mp_obj_t obj_in, qstr arg_name);
+
+// Wait as long as needed for the lock. This is used by SD card access from USB.
+// For most ports, busy-wait while running the background tasks.
+MP_WEAK bool common_hal_busio_spi_wait_for_lock(busio_spi_obj_t *self, uint32_t timeout_ms);
diff --git a/shared-module/busdisplay/BusDisplay.c b/shared-module/busdisplay/BusDisplay.c
@@ -6,6 +6,7 @@
 
 #include "shared-bindings/busdisplay/BusDisplay.h"
 
+#include "py/mphal.h"
 #include "py/runtime.h"
 #if CIRCUITPY_FOURWIRE
 #include "shared-bindings/fourwire/FourWire.h"
@@ -16,6 +17,7 @@
 #if CIRCUITPY_PARALLELDISPLAYBUS
 #include "shared-bindings/paralleldisplaybus/ParallelBus.h"
 #endif
+#include "shared/runtime/interrupt_char.h"
 #include "shared-bindings/microcontroller/Pin.h"
 #include "shared-bindings/time/__init__.h"
 #include "shared-module/displayio/__init__.h"
@@ -73,6 +75,9 @@ void common_hal_busdisplay_busdisplay_construct(busdisplay_busdisplay_obj_t *sel
         uint8_t *data = cmd + 2;
         while (!displayio_display_bus_begin_transaction(&self->bus)) {
             RUN_BACKGROUND_TASKS;
+            if (mp_hal_is_interrupted()) {
+                mp_raise_RuntimeError_varg(MP_ERROR_TEXT("%q init failed"), MP_QSTR_display);
+            }
         }
         if (self->bus.data_as_commands) {
             uint8_t full_command[data_size + 1];
@@ -288,6 +293,7 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are
 
         displayio_display_bus_set_region_to_update(&self->bus, &self->core, &subrectangle);
 
+        // Can't acquire display bus; skip the rest of the data.
         if (!displayio_display_bus_begin_transaction(&self->bus)) {
             return false;
         }
diff --git a/shared-module/displayio/__init__.c b/shared-module/displayio/__init__.c
@@ -146,6 +146,9 @@ static void common_hal_displayio_release_displays_impl(bool keep_primary) {
         displays[i].display_base.type = &mp_type_NoneType;
     }
     for (uint8_t i = 0; i < CIRCUITPY_DISPLAY_LIMIT; i++) {
+        if (i == primary_display_number) {
+            continue;
+        }
         mp_const_obj_t bus_type = display_buses[i].bus_base.type;
         if (bus_type == NULL || bus_type == &mp_type_NoneType) {
             continue;
@@ -197,16 +200,19 @@ static void common_hal_displayio_release_displays_impl(bool keep_primary) {
         display_buses[i].bus_base.type = &mp_type_NoneType;
     }
 
-    supervisor_stop_terminal();
+    if (!keep_primary) {
+        supervisor_stop_terminal();
+    }
 }
 
 void common_hal_displayio_release_displays(void) {
     common_hal_displayio_release_displays_impl(false);
 }
 
 void reset_displays(void) {
-    // In CircuitPython 10, release secondary displays before doing anything else:
-    // common_hal_displayio_release_displays_impl(true);
+    // TODO: In CircuitPython 11, uncomment the call.
+    // Release secondary displays.
+    // common_hal_displayio_release_displays_impl(/*keep_primary*/ true);
 
     // The SPI buses used by FourWires may be allocated on the heap so we need to move them inline.
     for (uint8_t i = 0; i < CIRCUITPY_DISPLAY_LIMIT; i++) {
diff --git a/shared-module/displayio/bus_core.c b/shared-module/displayio/bus_core.c
@@ -101,11 +101,16 @@ void displayio_display_bus_construct(displayio_display_bus_t *self,
     self->bus = bus;
 }
 
+// This is just a hint, and is not a reliable result, since the bus could be grabbed in between this called
+// and the attempt to use the bus. Use displayio_display_bus_begin_transaction(), which is atomic.
 bool displayio_display_bus_is_free(displayio_display_bus_t *self) {
     return !self->bus || self->bus_free(self->bus);
 }
 
-bool displayio_display_bus_begin_transaction(displayio_display_bus_t *self) {
+MP_WARN_UNUSED_RESULT bool displayio_display_bus_begin_transaction(displayio_display_bus_t *self) {
+    if (!self->bus) {
+        return false;
+    }
     mp_obj_base_t *bus_base = MP_OBJ_TO_PTR(self->bus);
     if (bus_base->type == &mp_type_NoneType) {
         return false;
@@ -144,7 +149,9 @@ void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, d
     }
 
     // Set column.
-    displayio_display_bus_begin_transaction(self);
+    if (!displayio_display_bus_begin_transaction(self)) {
+        return;
+    }
     uint8_t data[5];
     data[0] = self->column_command;
     uint8_t data_length = 1;
@@ -183,7 +190,9 @@ void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, d
 
     if (self->set_current_column_command != NO_COMMAND) {
         uint8_t command = self->set_current_column_command;
-        displayio_display_bus_begin_transaction(self);
+        if (!displayio_display_bus_begin_transaction(self)) {
+            return;
+        }
         self->send(self->bus, DISPLAY_COMMAND, chip_select, &command, 1);
         // Only send the first half of data because it is the first coordinate.
         self->send(self->bus, DISPLAY_DATA, chip_select, data, data_length / 2);
@@ -192,7 +201,9 @@ void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, d
 
 
     // Set row.
-    displayio_display_bus_begin_transaction(self);
+    if (!displayio_display_bus_begin_transaction(self)) {
+        return;
+    }
     data[0] = self->row_command;
     data_length = 1;
     if (!self->data_as_commands) {
@@ -227,7 +238,9 @@ void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, d
 
     if (self->set_current_row_command != NO_COMMAND) {
         uint8_t command = self->set_current_row_command;
-        displayio_display_bus_begin_transaction(self);
+        if (!displayio_display_bus_begin_transaction(self)) {
+            return;
+        }
         self->send(self->bus, DISPLAY_COMMAND, chip_select, &command, 1);
         // Only send the first half of data because it is the first coordinate.
         self->send(self->bus, DISPLAY_DATA, chip_select, data, data_length / 2);
diff --git a/shared-module/epaperdisplay/EPaperDisplay.c b/shared-module/epaperdisplay/EPaperDisplay.c
diff --git a/shared-module/sdcardio/SDCard.c b/shared-module/sdcardio/SDCard.c
diff --git a/supervisor/port.h b/supervisor/port.h
diff --git a/supervisor/shared/port.c b/supervisor/shared/port.c
diff --git a/supervisor/shared/usb/usb.c b/supervisor/shared/usb/usb.c
diff --git a/supervisor/shared/web_workflow/web_workflow.c b/supervisor/shared/web_workflow/web_workflow.c

Original file line number	Diff line number	Diff line change
`@@ -13,8 +13,8 @@`
`13`	`13`	`#include "freertos/task.h"`
`14`	`14`
`15`	`15`	`void port_background_tick(void) {`
`16`		`- // Zero delay in case FreeRTOS wants to switch to something else.`
`17`		`- vTaskDelay(0);`
	`16`	`+ // Yield with zero delay in case FreeRTOS wants to switch to something else.`
	`17`	`+ port_task_yield();`
`18`	`18`	`}`
`19`	`19`
`20`	`20`	`void port_background_task(void) {`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,8 @@ static void usb_device_task(void *param) {`
`56`	`56`	`tud_task();`
`57`	`57`	`tud_cdc_write_flush();`
`58`	`58`	`}`
`59`		`- vTaskDelay(1);`
	`59`	`+ // Yield with zero delay to switch to any other tasks at same priority.`
	`60`	`+ port_task_yield();`
`60`	`61`	`}`
`61`	`62`	`}`
`62`	`63`	`#endif // CIRCUITPY_USB_DEVICE`
`@@ -112,7 +113,7 @@ void init_usb_hardware(void) {`
`112`	`113`	`"usbd",`
`113`	`114`	`USBD_STACK_SIZE,`
`114`	`115`	`NULL,`
`115`		`- 5,`
	`116`	`+ 1,`
`116`	`117`	`usb_device_stack,`
`117`	`118`	`&usb_device_taskdef,`
`118`	`119`	`xPortGetCoreID());`
Original file line number	Diff line number	Diff line change
`@@ -600,7 +600,7 @@ __attribute__((used)) void __not_in_flash_func(isr_hardfault)(void) {`
`600`	`600`	`}`
`601`	`601`	`}`
`602`	`602`
`603`		`-void port_yield(void) {`
	`603`	`+void port_task_yield(void) {`
`604`	`604`	`#if CIRCUITPY_CYW43`
`605`	`605`	`cyw43_arch_poll();`
`606`	`606`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,7 @@ __attribute__((used)) void HardFault_Handler(void) {`
`210`	`210`	`}`
`211`	`211`	`}`
`212`	`212`
`213`		`-void port_yield(void) {`
	`213`	`+void port_task_yield(void) {`
`214`	`214`	`}`
`215`	`215`
`216`	`216`	`void port_boot_info(void) {`
Original file line number	Diff line number	Diff line change
`@@ -170,10 +170,14 @@ void port_wake_main_task_from_isr(void) {`
`170`	`170`	`k_event_set(&main_needed, 1);`
`171`	`171`	`}`
`172`	`172`
`173`		`-void port_yield(void) {`
	`173`	`+void port_task_yield(void) {`
`174`	`174`	`k_yield();`
`175`	`175`	`}`
`176`	`176`
	`177`	`+void port_task_sleep_ms(uint32_t msecs) {`
	`178`	`+ k_msleep(msecs);`
	`179`	`+}`
	`180`	`+`
`177`	`181`	`void port_boot_info(void) {`
`178`	`182`	`}`
`179`	`183`