Merge pull request #5510 from jepler/cmd25-v2

sdcardio: Use CMD25 across multiple writeblocks() calls

Author: dhalbert

ports/atmel-samd/boards/dynalora_usb/mpconfigboard.mk

@@ -17,4 +17,3 @@ CIRCUITPY_SDCARDIO = 1
 
 FROZEN_MPY_DIRS += $(TOP)/frozen/Adafruit_CircuitPython_NeoPixel
 FROZEN_MPY_DIRS += $(TOP)/frozen/Adafruit_CircuitPython_RFM9x
-FROZEN_MPY_DIRS += $(TOP)/frozen/Adafruit_CircuitPython_SD

ports/atmel-samd/boards/xinabox_cs11/mpconfigboard.mk

@@ -22,6 +22,3 @@ CIRCUITPY_TOUCHIO=0
 CIRCUITPY_USB_MIDI=0
 CIRCUITPY_RTC=0
 CIRCUITPY_SDCARDIO=1
-
-# Include these Python libraries in firmware.
-FROZEN_MPY_DIRS += $(TOP)/frozen/Adafruit_CircuitPython_SD

shared-bindings/sdcardio/SDCard.c

@@ -77,12 +77,11 @@
 //|             os.listdir('/sd')"""
 
 STATIC mp_obj_t sdcardio_sdcard_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
-    enum { ARG_spi, ARG_cs, ARG_baudrate, ARG_sdio, NUM_ARGS };
+    enum { ARG_spi, ARG_cs, ARG_baudrate, NUM_ARGS };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_spi, MP_ARG_OBJ, {.u_obj = mp_const_none } },
         { MP_QSTR_cs, MP_ARG_OBJ, {.u_obj = mp_const_none } },
         { MP_QSTR_baudrate, MP_ARG_INT, {.u_int = 8000000} },
-        { MP_QSTR_sdio, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_int = 8000000} },
     };
     MP_STATIC_ASSERT(MP_ARRAY_SIZE(allowed_args) == NUM_ARGS);
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
@@ -150,6 +149,23 @@ mp_obj_t sdcardio_sdcard_readblocks(mp_obj_t self_in, mp_obj_t start_block_in, m
 
 MP_DEFINE_CONST_FUN_OBJ_3(sdcardio_sdcard_readblocks_obj, sdcardio_sdcard_readblocks);
 
+//|     def sync(self) -> None:
+//|         """Ensure all blocks written are actually committed to the SD card
+//|
+//|         :return: None"""
+//|         ...
+mp_obj_t sdcardio_sdcard_sync(mp_obj_t self_in) {
+    sdcardio_sdcard_obj_t *self = (sdcardio_sdcard_obj_t *)self_in;
+    int result = common_hal_sdcardio_sdcard_sync(self);
+    if (result < 0) {
+        mp_raise_OSError(-result);
+    }
+    return mp_const_none;
+}
+
+MP_DEFINE_CONST_FUN_OBJ_1(sdcardio_sdcard_sync_obj, sdcardio_sdcard_sync);
+
+
 //|     def writeblocks(self, start_block: int, buf: ReadableBuffer) -> None:
 //|
 //|         """Write one or more blocks to the card
@@ -177,6 +193,7 @@ STATIC const mp_rom_map_elem_t sdcardio_sdcard_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_count), MP_ROM_PTR(&sdcardio_sdcard_count_obj) },
     { MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&sdcardio_sdcard_deinit_obj) },
     { MP_ROM_QSTR(MP_QSTR_readblocks), MP_ROM_PTR(&sdcardio_sdcard_readblocks_obj) },
+    { MP_ROM_QSTR(MP_QSTR_sync), MP_ROM_PTR(&sdcardio_sdcard_sync_obj) },
     { MP_ROM_QSTR(MP_QSTR_writeblocks), MP_ROM_PTR(&sdcardio_sdcard_writeblocks_obj) },
 };
 STATIC MP_DEFINE_CONST_DICT(sdcardio_sdcard_locals_dict, sdcardio_sdcard_locals_dict_table);

shared-module/sdcardio/SDCard.c

@@ -93,19 +93,53 @@ static uint8_t CRC7(const uint8_t *data, uint8_t n) {
 }
 
 #define READY_TIMEOUT_NS (300 * 1000 * 1000) // 300ms
-STATIC void wait_for_ready(sdcardio_sdcard_obj_t *self) {
+STATIC int wait_for_ready(sdcardio_sdcard_obj_t *self) {
     uint64_t deadline = common_hal_time_monotonic_ns() + READY_TIMEOUT_NS;
     while (common_hal_time_monotonic_ns() < deadline) {
         uint8_t b;
         common_hal_busio_spi_read(self->bus, &b, 1, 0xff);
         if (b == 0xff) {
-            break;
+            return 0;
+        }
+    }
+    return -ETIMEDOUT;
+}
+
+// Note: this is never called while "in cmd25" (in fact, it's only used by `exit_cmd25`)
+STATIC bool cmd_nodata(sdcardio_sdcard_obj_t *self, int cmd, int response) {
+    uint8_t cmdbuf[2] = {cmd, 0xff};
+
+    assert(!self->in_cmd25);
+
+    common_hal_busio_spi_write(self->bus, cmdbuf, sizeof(cmdbuf));
+
+    // Wait for the response (response[7] == response)
+    for (int i = 0; i < CMD_TIMEOUT; i++) {
+        common_hal_busio_spi_read(self->bus, cmdbuf, 1, 0xff);
+        if (cmdbuf[0] == response) {
+            return 0;
         }
     }
+    return -EIO;
+}
+
+
+STATIC int exit_cmd25(sdcardio_sdcard_obj_t *self) {
+    if (self->in_cmd25) {
+        DEBUG_PRINT("exit cmd25\n");
+        self->in_cmd25 = false;
+        return cmd_nodata(self, TOKEN_STOP_TRAN, 0);
+    }
+    return 0;
 }
 
 // In Python API, defaults are response=None, data_block=True, wait=True
 STATIC int cmd(sdcardio_sdcard_obj_t *self, int cmd, int arg, void *response_buf, size_t response_len, bool data_block, bool wait) {
+    int r = exit_cmd25(self);
+    if (r < 0) {
+        return r;
+    }
+
     DEBUG_PRINT("cmd % 3d [%02x] arg=% 11d [%08x] len=%d%s%s\n", cmd, cmd, arg, arg, response_len, data_block ? " data" : "", wait ? " wait" : "");
     uint8_t cmdbuf[6];
     cmdbuf[0] = cmd | 0x40;
@@ -116,7 +150,10 @@ STATIC int cmd(sdcardio_sdcard_obj_t *self, int cmd, int arg, void *response_buf
     cmdbuf[5] = CRC7(cmdbuf, 5);
 
     if (wait) {
-        wait_for_ready(self);
+        r = wait_for_ready(self);
+        if (r < 0) {
+            return r;
+        }
     }
 
     common_hal_busio_spi_write(self->bus, cmdbuf, sizeof(cmdbuf));
@@ -161,21 +198,6 @@ STATIC int block_cmd(sdcardio_sdcard_obj_t *self, int cmd_, int block, void *res
     return cmd(self, cmd_, block * self->cdv, response_buf, response_len, true, true);
 }
 
-STATIC bool cmd_nodata(sdcardio_sdcard_obj_t *self, int cmd, int response) {
-    uint8_t cmdbuf[2] = {cmd, 0xff};
-
-    common_hal_busio_spi_write(self->bus, cmdbuf, sizeof(cmdbuf));
-
-    // Wait for the response (response[7] == response)
-    for (int i = 0; i < CMD_TIMEOUT; i++) {
-        common_hal_busio_spi_read(self->bus, cmdbuf, 1, 0xff);
-        if (cmdbuf[0] == response) {
-            return 0;
-        }
-    }
-    return -EIO;
-}
-
 STATIC const compressed_string_t *init_card_v1(sdcardio_sdcard_obj_t *self) {
     for (int i = 0; i < CMD_TIMEOUT; i++) {
         if (cmd(self, 41, 0, NULL, 0, true, true) == 0) {
@@ -298,6 +320,7 @@ void common_hal_sdcardio_sdcard_deinit(sdcardio_sdcard_obj_t *self) {
     if (!self->bus) {
         return;
     }
+    common_hal_sdcardio_sdcard_sync(self);
     self->bus = 0;
     common_hal_digitalio_digitalinout_deinit(&self->cs);
 }
@@ -423,37 +446,43 @@ STATIC int _write(sdcardio_sdcard_obj_t *self, uint8_t token, void *buf, size_t
 STATIC int writeblocks(sdcardio_sdcard_obj_t *self, uint32_t start_block, mp_buffer_info_t *buf) {
     common_hal_sdcardio_check_for_deinit(self);
     uint32_t nblocks = buf->len / 512;
-    if (nblocks == 1) {
-        //  Use CMD24 to write a single block
-        int r = block_cmd(self, 24, start_block, NULL, 0, true, true);
-        if (r < 0) {
-            return r;
-        }
-        r = _write(self, TOKEN_DATA, buf->buf, buf->len);
-        if (r < 0) {
-            return r;
-        }
-    } else {
+
+    DEBUG_PRINT("cmd25? %d next_block %d start_block %d\n", self->in_cmd25, self->next_block, start_block);
+
+    if (!self->in_cmd25 || start_block != self->next_block) {
+        DEBUG_PRINT("entering CMD25 at %d\n", (int)start_block);
         //  Use CMD25 to write multiple block
         int r = block_cmd(self, 25, start_block, NULL, 0, true, true);
         if (r < 0) {
             return r;
         }
+        self->in_cmd25 = true;
+    }
 
-        uint8_t *ptr = buf->buf;
-        while (nblocks--) {
-            r = _write(self, TOKEN_CMD25, ptr, 512);
-            if (r < 0) {
-                return r;
-            }
-            ptr += 512;
-        }
+    self->next_block = start_block;
 
-        cmd_nodata(self, TOKEN_STOP_TRAN, 0);
+    uint8_t *ptr = buf->buf;
+    while (nblocks--) {
+        int r = _write(self, TOKEN_CMD25, ptr, 512);
+        if (r < 0) {
+            self->in_cmd25 = false;
+            return r;
+        }
+        self->next_block++;
+        ptr += 512;
     }
+
     return 0;
 }
 
+int common_hal_sdcardio_sdcard_sync(sdcardio_sdcard_obj_t *self) {
+    common_hal_sdcardio_check_for_deinit(self);
+    lock_and_configure_bus(self);
+    int r = exit_cmd25(self);
+    extraclock_and_unlock_bus(self);
+    return r;
+}
+
 int common_hal_sdcardio_sdcard_writeblocks(sdcardio_sdcard_obj_t *self, uint32_t start_block, mp_buffer_info_t *buf) {
     common_hal_sdcardio_check_for_deinit(self);
     if (buf->len % 512 != 0) {

shared-module/sdcardio/SDCard.h

@@ -41,11 +41,14 @@ typedef struct {
     int cdv;
     int baudrate;
     uint32_t sectors;
+    uint32_t next_block;
+    bool in_cmd25;
 } sdcardio_sdcard_obj_t;
 
 void common_hal_sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *spi, mcu_pin_obj_t *cs, int baudrate);
 void common_hal_sdcardio_sdcard_deinit(sdcardio_sdcard_obj_t *self);
 void common_hal_sdcardio_sdcard_check_for_deinit(sdcardio_sdcard_obj_t *self);
 int common_hal_sdcardio_sdcard_get_blockcount(sdcardio_sdcard_obj_t *self);
 int common_hal_sdcardio_sdcard_readblocks(sdcardio_sdcard_obj_t *self, uint32_t start_block, mp_buffer_info_t *buf);
+int common_hal_sdcardio_sdcard_sync(sdcardio_sdcard_obj_t *self);
 int common_hal_sdcardio_sdcard_writeblocks(sdcardio_sdcard_obj_t *self, uint32_t start_block, mp_buffer_info_t *buf);