Sd card usb presentation fixes

docs/workflows.md

@@ -44,6 +44,12 @@ A few boards have SD card automounting. (This is based on the ``DEFAULT_SD`` set
 ``mpconfigboard.h``.) The card is writable from CircuitPython by default and read-only to the host.
 `storage.remount()` can be used to remount the drive to the host as read-write.
 
+On most other boards, except for ``atmel-samd`` boards, an SD card mounted in user code
+at ``/sd`` will become visible after a few seconds on the attached host computer, as an
+additional drive besides CIRCUITPY and (if present) CPSAVES. It will present with the volume
+label on the SD card. Depending on the host operating system settings, the drive may or may not be
+auto-mounted on the host. Host writes to drives mounted by user code will not trigger a reload.
+
 ### CDC serial
 CircuitPython exposes one CDC USB interface for CircuitPython serial. This is a standard serial
 USB interface.

main.c

@@ -214,7 +214,7 @@ static void stop_mp(void) {
     mp_vfs_mount_t *vfs = MP_STATE_VM(vfs_mount_table);
 
     // Unmount all heap allocated vfs mounts.
-    while (gc_nbytes(vfs) > 0) {
+    while (gc_ptr_on_heap(vfs)) {
         vfs = vfs->next;
     }
     MP_STATE_VM(vfs_mount_table) = vfs;

ports/atmel-samd/mpconfigport.h

@@ -11,6 +11,9 @@
 
 // Definitions that control circuitpy_mpconfig.h:
 
+// On SAMD, presenting the SD card as a second LUN causes USB disconnect. This needs to be fixed eventually.
+#define CIRCUITPY_SDCARD_USB (0)
+
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
 #ifdef SAMD21

ports/cxd56/mpconfigport.h

@@ -8,6 +8,9 @@
 
 #define MICROPY_PY_SYS_PLATFORM                 "CXD56"
 
+// SD card socket on board is configured for sdioio, which is not supported for automatic USB presentation.
+#define CIRCUITPY_SDCARD_USB (0)
+
 // 64kiB stack
 #define CIRCUITPY_DEFAULT_STACK_SIZE            (0x10000)
 

ports/espressif/common-hal/_bleio/Characteristic.c

@@ -120,7 +120,7 @@ void common_hal_bleio_characteristic_deinit(bleio_characteristic_obj_t *self) {
         return;
     }
     if (self->current_value != NULL) {
-        if (gc_nbytes(self->current_value) > 0) {
+        if (gc_ptr_on_heap(self->current_value)) {
             m_free(self->current_value);
         } else {
             port_free(self->current_value);

ports/nordic/common-hal/_bleio/Characteristic.c

@@ -80,7 +80,7 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
         // to allocate.
         self->initial_value_len = initial_value_bufinfo->len;
         if (gc_alloc_possible()) {
-            if (gc_nbytes(initial_value_bufinfo->buf) > 0) {
+            if (gc_ptr_on_heap(initial_value_bufinfo->buf)) {
                 uint8_t *initial_value = m_malloc_without_collect(self->initial_value_len);
                 memcpy(initial_value, initial_value_bufinfo->buf, self->initial_value_len);
                 self->initial_value = initial_value;

ports/nordic/common-hal/_bleio/Service.c

@@ -131,7 +131,7 @@ void common_hal_bleio_service_add_characteristic(bleio_service_obj_t *self,
         BLE_GAP_CONN_SEC_MODE_SET_OPEN(&user_desc_md.read_perm);
         // If the description is on the Python heap, then have the SD copy it. If not, assume it's
         // static and will live for longer than the SD.
-        user_desc_md.vloc = gc_nbytes(user_description) > 0 ? BLE_GATTS_VLOC_STACK : BLE_GATTS_VLOC_USER;
+        user_desc_md.vloc = gc_ptr_on_heap(user_description) ? BLE_GATTS_VLOC_STACK : BLE_GATTS_VLOC_USER;
         char_md.p_user_desc_md = &user_desc_md;
         char_md.p_char_user_desc = (const uint8_t *)user_description;
         char_md.char_user_desc_max_size = strlen(user_description);

ports/nordic/common-hal/busio/UART.c

@@ -122,7 +122,7 @@ void uart_reset(void) {
 
 void common_hal_busio_uart_never_reset(busio_uart_obj_t *self) {
     // Don't never reset objects on the heap.
-    if (gc_alloc_possible() && gc_nbytes(self) > 0) {
+    if (gc_alloc_possible() && gc_ptr_on_heap(self)) {
         return;
     }
     for (size_t i = 0; i < MP_ARRAY_SIZE(nrfx_uartes); i++) {
@@ -346,7 +346,7 @@ size_t common_hal_busio_uart_write(busio_uart_obj_t *self, const uint8_t *data,
         RUN_BACKGROUND_TASKS;
     }
 
-    if (!nrfx_is_in_ram(data) && gc_alloc_possible() && gc_nbytes(tx_buf) > 0) {
+    if (!nrfx_is_in_ram(data) && gc_alloc_possible() && gc_ptr_on_heap(tx_buf)) {
         gc_free(tx_buf);
     }
 

py/circuitpy_mpconfig.h

@@ -513,6 +513,18 @@ void background_callback_run_all(void);
 
 // USB settings
 
+#ifndef CIRCUITPY_SDCARD_USB
+#if CIRCUITPY_USB_DEVICE
+#define CIRCUITPY_SDCARD_USB (CIRCUITPY_SDCARDIO && CIRCUITPY_USB_MSC)
+#else
+#define CIRCUITPY_SDCARD_USB (0)
+#endif
+#endif
+
+#if CIRCUITPY_SDCARD_USB && !(CIRCUITPY_SDCARDIO)
+#error CIRCUITPY_SDCARD_USB requires CIRCUITPY_SDCARDIO
+#endif
+
 // Debug level for TinyUSB. Only outputs over debug UART so it doesn't cause
 // additional USB logging.
 #ifndef CIRCUITPY_DEBUG_TINYUSB

py/gc.c

@@ -446,7 +446,7 @@ bool gc_is_locked(void) {
 }
 
 // CIRCUITPY-CHANGE: additional function
-bool gc_ptr_on_heap(void *ptr) {
+bool gc_ptr_on_heap(const void *ptr) {
     for (mp_state_mem_area_t *area = &MP_STATE_MEM(area); area != NULL; area = NEXT_AREA(area)) {
         if (ptr >= (void *)area->gc_pool_start   // must be above start of pool
             && ptr < (void *)area->gc_pool_end) {   // must be below end of pool

py/gc.h

@@ -87,7 +87,7 @@ void *gc_realloc(void *ptr, size_t n_bytes, bool allow_move);
 // CIRCUITPY-CHANGE
 // True if the pointer is on the MP heap. Doesn't require that it is the start
 // of a block.
-bool gc_ptr_on_heap(void *ptr);
+bool gc_ptr_on_heap(const void *ptr);
 
 typedef struct _gc_info_t {
     size_t total;

shared-bindings/_bleio/Adapter.c

@@ -339,7 +339,7 @@ static mp_obj_t bleio_adapter_start_scan(size_t n_args, const mp_obj_t *pos_args
     if (args[ARG_prefixes].u_obj != MP_OBJ_NULL) {
         mp_get_buffer_raise(args[ARG_prefixes].u_obj, &prefix_bufinfo, MP_BUFFER_READ);
         // An empty buffer may not be on the heap, but that doesn't matter.
-        if (prefix_bufinfo.len > 0 && gc_nbytes(prefix_bufinfo.buf) == 0) {
+        if (prefix_bufinfo.len > 0 && !gc_ptr_on_heap(prefix_bufinfo.buf)) {
             mp_raise_ValueError(MP_ERROR_TEXT("Prefix buffer must be on the heap"));
         }
     }

shared-module/sdcardio/__init__.c

@@ -123,5 +123,5 @@ void automount_sd_card(void) {
     sdcard_vfs->next = MP_STATE_VM(vfs_mount_table);
     MP_STATE_VM(vfs_mount_table) = sdcard_vfs;
     _mounted = true;
-    #endif
+    #endif // DEFAULT_SD_CARD_DETECT
 }

supervisor/shared/filesystem.c

@@ -148,8 +148,7 @@ bool filesystem_init(bool create_allowed, bool force_create) {
         res = f_mkdir(&circuitpy->fatfs, "/sd");
         #if CIRCUITPY_FULL_BUILD
         MAKE_FILE_WITH_OPTIONAL_CONTENTS(&circuitpy->fatfs, "/sd/placeholder.txt",
-            "SD cards mounted at /sd will hide this file from Python."
-            " SD cards are not visible via USB CIRCUITPY.\n");
+            "SD cards mounted at /sd will hide this file from Python.\n");
         #endif
         #endif
 

supervisor/shared/usb/usb_msc_flash.c

@@ -45,6 +45,11 @@ static bool ejected[LUN_COUNT] = { [0 ... (LUN_COUNT - 1)] = true};
 static bool eject_once[LUN_COUNT] = { [0 ... (LUN_COUNT - 1)] = false};
 static bool locked[LUN_COUNT] = { [0 ... (LUN_COUNT - 1)] = false};
 
+// Set to true if a write was in a file data or metadata area,
+// as opposed to in the filesystem metadata area (e.g., dirty bit).
+// Used to determine if an auto-reload is warranted.
+static bool content_write[LUN_COUNT] = { [0 ... (LUN_COUNT - 1)] = false};
+
 #include "tusb.h"
 
 static const uint8_t usb_msc_descriptor_template[] = {
@@ -132,7 +137,7 @@ static fs_user_mount_t *get_vfs(int lun) {
     if (lun == SAVES_LUN) {
         const char *path_under_mount;
         fs_user_mount_t *saves = filesystem_for_path("/saves", &path_under_mount);
-        if (saves != root && (saves->blockdev.flags & MP_BLOCKDEV_FLAG_NATIVE) != 0 && gc_nbytes(saves) == 0) {
+        if (saves != root && (saves->blockdev.flags & MP_BLOCKDEV_FLAG_NATIVE) != 0 && !gc_ptr_on_heap(saves)) {
             return saves;
         }
     }
@@ -141,6 +146,7 @@ static fs_user_mount_t *get_vfs(int lun) {
     if (lun == SDCARD_LUN) {
         const char *path_under_mount;
         fs_user_mount_t *sdcard = filesystem_for_path("/sd", &path_under_mount);
+        // If "/sd" is on the root filesystem, nothing has been mounted there.
         if (sdcard != root && (sdcard->blockdev.flags & MP_BLOCKDEV_FLAG_NATIVE) != 0) {
             return sdcard;
         } else {
@@ -290,34 +296,50 @@ int32_t tud_msc_write10_cb(uint8_t lun, uint32_t lba, uint32_t offset, uint8_t *
     if (vfs == NULL) {
         return -1;
     }
+
     disk_write(vfs, buffer, lba, block_count);
     // Since by getting here we assume the mount is read-only to
-    // MicroPython let's update the cached FatFs sector if it's the one
+    // CircuitPython let's update the cached FatFs sector if it's the one
     // we just wrote.
+    if
     #if FF_MAX_SS != FF_MIN_SS
-    if (vfs->fatfs.ssize == MSC_FLASH_BLOCK_SIZE) {
+    (vfs->fatfs.ssize == MSC_FLASH_BLOCK_SIZE)
     #else
     // The compiler can optimize this away.
-    if (FF_MAX_SS == FILESYSTEM_BLOCK_SIZE) {
-        #endif
+    (FF_MAX_SS == FILESYSTEM_BLOCK_SIZE)
+    #endif
+    {
         if (lba == vfs->fatfs.winsect && lba > 0) {
             memcpy(vfs->fatfs.win,
                 buffer + MSC_FLASH_BLOCK_SIZE * (vfs->fatfs.winsect - lba),
                 MSC_FLASH_BLOCK_SIZE);
         }
     }
 
+    // A write to an lba below fatbase is in the filesystem metadata (BPB) area or the "Reserved Region",
+    // and is probably setting or clearing the dirty bit. This should not trigger auto-reload.
+    // All other writes will trigger auto-reload.
+    if (lba >= vfs->fatfs.fatbase) {
+        content_write[lun] = true;
+    }
+
     return block_count * MSC_FLASH_BLOCK_SIZE;
 }
 
 // Callback invoked when WRITE10 command is completed (status received and accepted by host).
 // used to flush any pending cache.
 void tud_msc_write10_complete_cb(uint8_t lun) {
-    (void)lun;
-
-    // This write is complete; initiate an autoreload.
     autoreload_resume(AUTORELOAD_SUSPEND_USB);
-    autoreload_trigger();
+
+    // This write is complete; initiate an autoreload if this was a file data or metadata write,
+    // not just a dirty-bit write.
+    if (content_write[lun] &&
+        // Fast path: lun == 0 is CIRCUITPY, which can always trigger auto-reload if enabled.
+        // Don't autoreload if this lun was mounted by the user: that will cause a VM stop and an unmount.
+        (lun == 0 || !gc_ptr_on_heap(get_vfs(lun)))) {
+        autoreload_trigger();
+        content_write[lun] = false;
+    }
 }
 
 // Invoked when received SCSI_CMD_INQUIRY
@@ -337,7 +359,7 @@ bool tud_msc_test_unit_ready_cb(uint8_t lun) {
         return false;
     }
 
-    #if CIRCUITPY_SDCARDIO
+    #if CIRCUITPY_SDCARD_USB
     if (lun == SDCARD_LUN) {
         automount_sd_card();
     }