rp2_sd: Cleanup and fixes

- Use SD_SECTOR_SIZE define instead of magic number 512
- Replace uint8_t buf[1] with plain uint8_t
- Check sd_spi_read_dma precondition that sd_dma_context state has to be
  DMA_IDLE explicitly and return false instead of relying on assert
- sd_spi_dma_isr is time critical

software/src/rp2_sd/module.c

@@ -67,12 +67,12 @@ static mp_obj_t sdcard_readblocks(mp_obj_t self_obj, mp_obj_t block_obj, mp_obj_
     mp_buffer_info_t bufinfo;
     if (!mp_get_buffer(buf_obj, &bufinfo, MP_BUFFER_WRITE))
         mp_raise_ValueError("Not a write buffer");
-    if (bufinfo.len % 512 != 0)
+    if (bufinfo.len % SD_SECTOR_SIZE != 0)
         mp_raise_ValueError("Buffer length is invalid");
-    const int nblocks = bufinfo.len / 512;
+    const int nblocks = bufinfo.len / SD_SECTOR_SIZE;
     for (int block = 0; block < nblocks; block++) {
         // TODO: Implement CMD18 read multiple blocks
-        if (!sd_readblock(&self->sd_context, start_block + block, bufinfo.buf + block * 512))
+        if (!sd_readblock(&self->sd_context, start_block + block, bufinfo.buf + block * SD_SECTOR_SIZE))
             mp_raise_OSError(MP_EIO);
     }
     return mp_const_none;
@@ -87,7 +87,7 @@ static mp_obj_t sdcard_ioctl(mp_obj_t self_obj, mp_obj_t op_obj, mp_obj_t arg_ob
     case 4:
         return mp_obj_new_int(self->sd_context.blocks);
     case 5:
-        return mp_obj_new_int(512);
+        return mp_obj_new_int(SD_SECTOR_SIZE);
     default:
         return mp_const_none;
     }

software/src/rp2_sd/sd.c

@@ -61,28 +61,28 @@ static bool sd_check_interface_condition(void)
 
 static bool sd_send_op_cond(void)
 {
-    uint8_t buf[1];
+    uint8_t buf;
     bool use_acmd = true;
     for (int timeout = 0; timeout < 500; ++timeout) {
         bool result = false;
         if (use_acmd)
-            result = sd_acmd(41, 0x40000000, 1, buf);
+            result = sd_acmd(41, 0x40000000, 1, &buf);
         else
-            result = sd_cmd(1, 0x40000000, 1, buf);
+            result = sd_cmd(1, 0x40000000, 1, &buf);
         if (!result) {
-            if (use_acmd && buf[0] & SD_R1_ILLEGAL_COMMAND) {
+            if (use_acmd && buf & SD_R1_ILLEGAL_COMMAND) {
 #ifdef SD_DEBUG
                 printf("sd_init: card does not understand ACMD41, try CMD1...\n");
 #endif
                 continue;
-            } else if (buf[0] != 0x01) {
+            } else if (buf != 0x01) {
                 printf("sd_init: send_op_cond failed\n");
                 return false;
             } else {
                 continue;
             }
         }
-        if (buf[0] == 0x00) {
+        if (buf == 0x00) {
             return true;
         }
     }
@@ -162,7 +162,7 @@ static bool sd_read_csd(struct sd_context *sd_context)
             break;
         }
         case 1: {
-            blocksize = 512;
+            blocksize = SD_SECTOR_SIZE;
             const unsigned c_size = (buf[7] & 0x3f) << 16 | buf[8] << 8 | buf[9];
             blocks = (c_size + 1) * 1024;
             version = 2;
@@ -241,19 +241,19 @@ bool sd_deinit(struct sd_context *sd_context)
     return true;
 }
 
-bool sd_readblock(struct sd_context *sd_context, size_t sector_num, uint8_t buffer[static 512])
+bool sd_readblock(struct sd_context *sd_context, size_t sector_num, uint8_t buffer[static SD_SECTOR_SIZE])
 {
     if (!sd_context->initialized || sector_num >= sd_context->blocks)
         return false;
 
-    return sd_cmd_read(17, sector_num, 512, buffer);
+    return sd_cmd_read(17, sector_num, SD_SECTOR_SIZE, buffer);
 }
 
-bool sd_readblock_start(struct sd_context *sd_context, size_t sector_num, uint8_t buffer[static 512])
+bool sd_readblock_start(struct sd_context *sd_context, size_t sector_num, uint8_t buffer[static SD_SECTOR_SIZE])
 {
     if (!sd_context->initialized || sector_num >= sd_context->blocks)
         return false;
-    return sd_cmd_read_start(17, sector_num, 512, buffer);
+    return sd_cmd_read_start(17, sector_num, SD_SECTOR_SIZE, buffer);
 }
 
 bool sd_readblock_complete(struct sd_context *sd_context)

software/src/rp2_sd/sd.h

@@ -4,6 +4,8 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#define SD_SECTOR_SIZE 512
+
 struct sd_context {
     size_t blocks;
     bool initialized;
@@ -14,8 +16,8 @@ struct sd_context {
 bool sd_init(struct sd_context *context, int mosi, int miso, int sck, int ss, int rate);
 bool sd_deinit(struct sd_context *sd_context);
 
-bool sd_readblock(struct sd_context *context, size_t sector_num, uint8_t buffer[static 512]);
+bool sd_readblock(struct sd_context *context, size_t sector_num, uint8_t buffer[static SD_SECTOR_SIZE]);
 
-bool sd_readblock_start(struct sd_context *context, size_t sector_num, uint8_t buffer[static 512]);
+bool sd_readblock_start(struct sd_context *context, size_t sector_num, uint8_t buffer[static SD_SECTOR_SIZE]);
 bool sd_readblock_complete(struct sd_context *context);
 bool sd_readblock_is_complete(struct sd_context *context);

software/src/rp2_sd/sd_spi.c

@@ -1,7 +1,4 @@
 #include "hardware/pio.h"
-#ifdef NDEBUG
-#undef NDEBUG
-#endif
 
 #include "sd_spi.h"
 #include "sd_util.h"
@@ -69,7 +66,7 @@ static void __time_critical_func(sd_spi_read_blocking)(uint8_t wrdata, uint8_t *
     assert(pio_sm_is_rx_fifo_empty(SD_PIO, sd_spi_context.spi_sm));
 }
 
-static void sd_spi_dma_isr(void)
+static void __time_critical_func(sd_spi_dma_isr)(void)
 {
     if (dma_channel_get_irq0_status(sd_spi_context.spi_dma_rd)) {
         dma_channel_acknowledge_irq0(sd_spi_context.spi_dma_rd);
@@ -120,9 +117,10 @@ void sd_spi_wait_complete(void)
 
 bool sd_cmd_read_is_complete(void) { return sd_spi_context.sd_dma_context.state == DMA_IDLE; }
 
-static void sd_spi_read_dma(uint8_t wrdata, uint8_t *data, size_t len)
+static bool sd_spi_read_dma(uint8_t wrdata, uint8_t *data, size_t len)
 {
-    assert(sd_spi_context.sd_dma_context.state == DMA_IDLE);
+    if (sd_spi_context.sd_dma_context.state != DMA_IDLE)
+        return false;
     channel_config_set_chain_to(&sd_spi_context.spi_dma_rd_cfg, sd_spi_context.spi_dma_rd);
     channel_config_set_irq_quiet(&sd_spi_context.spi_dma_rd_cfg, false);
     dma_channel_configure(sd_spi_context.spi_dma_rd, &sd_spi_context.spi_dma_rd_cfg,
@@ -134,6 +132,7 @@ static void sd_spi_read_dma(uint8_t wrdata, uint8_t *data, size_t len)
     sd_spi_context.sd_dma_context.read_buf = data;
     sd_spi_context.sd_dma_context.wrdata = wrdata;
     dma_start_channel_mask((1 << sd_spi_context.spi_dma_rd) | (1 << sd_spi_context.spi_dma_wr));
+    return true;
 }
 
 static void sd_spi_cmd_send(const uint8_t cmd, const uint32_t arg)
@@ -193,7 +192,8 @@ bool sd_cmd_read_start(uint8_t cmd, uint32_t arg, unsigned datalen, uint8_t data
     }
     if (!got_r1 || buf[0] != 0x00)
         goto abort;
-    sd_spi_read_dma(0xff, data, datalen);
+    if (!sd_spi_read_dma(0xff, data, datalen))
+        goto abort;
     return true;
 
 abort:
@@ -204,10 +204,10 @@ abort:
 
 bool sd_cmd_read_complete(void)
 {
-    uint8_t buf[1];
+    uint8_t buf;
     sd_spi_wait_complete();
     gpio_put(sd_spi_context.ss, true);
-    sd_spi_read_blocking(0xff, buf, 1);
+    sd_spi_read_blocking(0xff, &buf, 1);
     return (sd_spi_context.sd_dma_context.read_token_buf == 0xfe);
 }