Fix timing bugs during SD card init which become visible with -Os

src/main/drivers/sdcard.c

@@ -844,9 +844,6 @@ uint8_t SD_ReadByte(void)
 #define SET_CS_HIGH          GPIO_SetBits(SDCARD_SPI_CS_GPIO,   SDCARD_SPI_CS_PIN)
 #define SET_CS_LOW           GPIO_ResetBits(SDCARD_SPI_CS_GPIO, SDCARD_SPI_CS_PIN)
 
-#define DESELECT_SDCARD      SET_CS_HIGH
-#define SELECT_SDCARD        SET_CS_LOW
-
 #define SDCARD_INIT_NUM_DUMMY_BYTES 10
 #define SDCARD_MAXIMUM_BYTE_DELAY_FOR_CMD_REPLY 8
 // Chosen so that CMD8 will have the same CRC as CMD0:
@@ -858,6 +855,7 @@ uint8_t SD_ReadByte(void)
 typedef enum {
     SDCARD_STATE_NOT_PRESENT = 0,
     SDCARD_STATE_INITIALIZATION,
+    SDCARD_STATE_INITIALIZATION_RECEIVE_CID,
     SDCARD_STATE_READY,
     SDCARD_STATE_READING,
     SDCARD_STATE_WRITING,
@@ -887,6 +885,22 @@ static sdcard_t sdcard;
 
 STATIC_ASSERT(sizeof(sdcardCSD_t) == 16, sdcard_csd_bitfields_didnt_pack_properly);
 
+static void sdcard_select()
+{
+    SET_CS_LOW;
+}
+
+static void sdcard_deselect()
+{
+    // As per the SD-card spec, give the card 8 dummy clocks so it can finish its operation
+    //spiTransferByte(SDCARD_SPI_INSTANCE, 0xFF);
+
+    while (spiIsBusBusy(SDCARD_SPI_INSTANCE)) {
+    }
+
+    SET_CS_HIGH;
+}
+
 
 /**
  * The SD card spec requires 8 clock cycles to be sent by us on the bus after most commands so it can finish its
@@ -928,7 +942,7 @@ static uint8_t sdcard_waitForNonIdleByte(int maxDelay)
  * with the given argument, waits up to SDCARD_MAXIMUM_BYTE_DELAY_FOR_CMD_REPLY bytes for a reply, and returns the
  * first non-0xFF byte of the reply.
  *
- * You must select the card first with SELECT_SDCARD and deselect it afterwards with DESELECT_SDCARD.
+ * You must select the card first with sdcard_select() and deselect it afterwards with sdcard_deselect().
  *
  * Upon failure, 0xFF is returned.
  */
@@ -973,7 +987,7 @@ static bool sdcard_validateInterfaceCondition()
 
     sdcard.version = 0;
 
-    SELECT_SDCARD;
+    sdcard_select();
 
     uint8_t status = sdcard_sendCommand(SDCARD_COMMAND_SEND_IF_COND, (SDCARD_VOLTAGE_ACCEPTED_2_7_to_3_6 << 8) | SDCARD_IF_COND_CHECK_PATTERN);
 
@@ -994,14 +1008,14 @@ static bool sdcard_validateInterfaceCondition()
         }
     }
 
-    DESELECT_SDCARD;
+    sdcard_deselect();
 
     return sdcard.version > 0;
 }
 
 static bool sdcard_readOCRRegister(uint32_t *result)
 {
-    SELECT_SDCARD;
+    sdcard_select();
 
     uint8_t status = sdcard_sendCommand(SDCARD_COMMAND_READ_OCR, 0);
 
@@ -1010,27 +1024,39 @@ static bool sdcard_readOCRRegister(uint32_t *result)
     spiTransfer(SDCARD_SPI_INSTANCE, response, NULL, sizeof(response));
 
     if (status == 0) {
-        DESELECT_SDCARD;
+        sdcard_deselect();
 
         *result = (response[0] << 24) | (response[1] << 16) | (response[2] << 8) | response[3];
 
         return true;
     } else {
-        DESELECT_SDCARD;
+        sdcard_deselect();
 
         return false;
     }
 }
 
+typedef enum {
+    SDCARD_RECEIVE_SUCCESS,
+    SDCARD_RECEIVE_BLOCK_IN_PROGRESS,
+    SDCARD_RECEIVE_ERROR,
+} sdcardReceiveBlockStatus_e;
+
 /**
  * Attempt to receive a data block from the SD card.
  *
  * Return true on success, otherwise the card has not responded yet and you should retry later.
  */
-static bool sdcard_receiveDataBlock(uint8_t *buffer, int count)
+static sdcardReceiveBlockStatus_e sdcard_receiveDataBlock(uint8_t *buffer, int count)
 {
-    if (sdcard_waitForNonIdleByte(SDCARD_MAXIMUM_BYTE_DELAY_FOR_CMD_REPLY) != SDCARD_SINGLE_BLOCK_READ_START_TOKEN) {
+    uint8_t dataToken = sdcard_waitForNonIdleByte(8);
-        return false;
+
+    if (dataToken == 0xFF) {
+        return SDCARD_RECEIVE_BLOCK_IN_PROGRESS;
+    }
+
+    if (dataToken != SDCARD_SINGLE_BLOCK_READ_START_TOKEN) {
+        return SDCARD_RECEIVE_ERROR;
     }
 
     spiTransfer(SDCARD_SPI_INSTANCE, buffer, NULL, count);
@@ -1039,7 +1065,7 @@ static bool sdcard_receiveDataBlock(uint8_t *buffer, int count)
     spiTransferByte(SDCARD_SPI_INSTANCE, 0xFF);
     spiTransferByte(SDCARD_SPI_INSTANCE, 0xFF);
 
-    return true;
+    return SDCARD_RECEIVE_SUCCESS;
 }
 
 /**
@@ -1076,17 +1102,12 @@ static bool sdcard_sendDataBlock(uint8_t *buffer, int count)
     return (dataResponseToken & 0x1F) == 0x05;
 }
 
-static bool sdcard_fetchCID()
+static bool sdcard_receiveCID()
 {
     uint8_t cid[16];
 
-    SELECT_SDCARD;
+    if (sdcard_receiveDataBlock(cid, sizeof(cid)) != SDCARD_RECEIVE_SUCCESS) {
-
+        sdcard_deselect();
-    uint8_t status = sdcard_sendCommand(SDCARD_COMMAND_SEND_CID, 0);
-
-    if (status != 0 || !sdcard_receiveDataBlock(cid, sizeof(cid))) {
-        DESELECT_SDCARD;
-
         return false;
     }
 
@@ -1103,7 +1124,7 @@ static bool sdcard_fetchCID()
     sdcard.metadata.productionYear = (((cid[13] & 0x0F) << 4) | (cid[14] >> 4)) + 2000;
     sdcard.metadata.productionMonth = cid[14] & 0x0F;
 
-    DESELECT_SDCARD;
+    sdcard_deselect();
 
     return true;
 }
@@ -1112,11 +1133,12 @@ static bool sdcard_fetchCSD()
 {
     uint32_t readBlockLen, blockCount, blockCountMult, capacityBytes;
 
-    SELECT_SDCARD;
+    sdcard_select();
 
+    // The CSD command's data block will arrive within 8 idle clock cycles (SD card spec)
     bool success =
         sdcard_sendCommand(SDCARD_COMMAND_SEND_CSD, 0) == 0
-        && sdcard_receiveDataBlock((uint8_t*) &sdcard.csd, sizeof(sdcard.csd))
+        && sdcard_receiveDataBlock((uint8_t*) &sdcard.csd, sizeof(sdcard.csd)) == SDCARD_RECEIVE_SUCCESS
         && SDCARD_GET_CSD_FIELD(sdcard.csd, 1, TRAILER) == 1;
 
     if (success) {
@@ -1139,73 +1161,51 @@ static bool sdcard_fetchCSD()
         }
     }
 
-    DESELECT_SDCARD;
+    sdcard_deselect();
 
     return success;
 }
 
 /**
- * Call once SDcard has finished its initialisation phase to read ID data from the card and complete our init.
+ * Call after the CID and CSD data have been read to set our preferred settings into the card (frequency and blocksize).
  *
  * Returns true on success, false on card init failure.
  */
-static bool sdcard_completeInit()
+static bool sdcard_setConfigurationAndFinalClock()
 {
-    if (sdcard.version == 2) {
-        // Check for high capacity card
-        uint32_t ocr;
-
-        if (!sdcard_readOCRRegister(&ocr)) {
-            return false;
-        }
-
-        sdcard.highCapacity = (ocr & (1 << 30)) != 0;
-    } else {
-        // Version 1 cards are always low-capacity
-        sdcard.highCapacity = false;
-    }
-
-    if (!sdcard_fetchCID() || !sdcard_fetchCSD())
-        return false;
-    
     /* The spec is a little iffy on what the default block size is for Standard Size cards (it can be changed on
      * standard size cards) so let's just set it to 512 explicitly so we don't have a problem.
      */
     if (!sdcard.highCapacity) {
-        SELECT_SDCARD;
+        sdcard_select();
 
         if (sdcard_sendCommand(SDCARD_COMMAND_SET_BLOCKLEN, SDCARD_BLOCK_SIZE) != 0) {
+            sdcard_deselect();
             return false;
         }
 
-        DESELECT_SDCARD;
+        sdcard_deselect();
     }
 
     spiSetDivisor(SDCARD_SPI_INSTANCE, SDCARD_SPI_FULL_SPEED_CLOCK_DIVIDER);
 
-    sdcard.state = SDCARD_STATE_READY;
-
     return true;
 }
 
 /**
  * Check if the SD Card has completed its startup sequence. Must be called with sdcard.state == SDCARD_STATE_INITIALIZATION.
  *
- * Changes sdcard.state to SDCARD_STATE_READY on success and returns true, returns false otherwise.
+ * Returns true if the card has finished its init process.
  */
 static bool sdcard_checkInitDone() {
-    SELECT_SDCARD;
+    sdcard_select();
 
     uint8_t status = sdcard_sendAppCommand(SDCARD_ACOMMAND_SEND_OP_COND, sdcard.version == 2 ? 1 << 30 /* We support high capacity cards */ : 0);
 
-    DESELECT_SDCARD;
+    sdcard_deselect();
 
     // When card init is complete, the idle bit in the response becomes zero.
-    if (status == 0x00) {
+    return status == 0x00;
-        return sdcard_completeInit();
-    }
-
-    return false;
 }
 
 bool sdcard_init()
@@ -1221,15 +1221,15 @@ bool sdcard_init()
 
     spiTransfer(SDCARD_SPI_INSTANCE, NULL, NULL, SDCARD_INIT_NUM_DUMMY_BYTES);
 
-    // Wait for that transmission to finish before we enable the SDCard, so it receives the required number of cycles
+    // Wait for that transmission to finish before we enable the SDCard, so it receives the required number of cycles:
     while (spiIsBusBusy(SDCARD_SPI_INSTANCE)) {
     }
 
-    SELECT_SDCARD;
+    sdcard_select();
 
     uint8_t initStatus = sdcard_sendCommand(SDCARD_COMMAND_GO_IDLE_STATE, 0);
 
-    DESELECT_SDCARD;
+    sdcard_deselect();
 
     if (initStatus != SDCARD_R1_STATUS_BIT_IDLE)
         return false;
@@ -1256,33 +1256,89 @@ bool sdcard_init()
  */
 void sdcard_poll()
 {
+    doMore:
     switch (sdcard.state) {
-        case SDCARD_STATE_READING:
+        case SDCARD_STATE_INITIALIZATION:
-            if (sdcard_receiveDataBlock(sdcard.pendingOperation.buffer, SDCARD_BLOCK_SIZE)) {
+            if (sdcard_checkInitDone()) {
-                DESELECT_SDCARD;
+                if (sdcard.version == 2) {
+                    // Check for high capacity card
+                    uint32_t ocr;
 
-                sdcard.state = SDCARD_STATE_READY;
+                    if (!sdcard_readOCRRegister(&ocr)) {
+                        break;
+                    }
 
-                if (sdcard.pendingOperation.callback) {
+                    sdcard.highCapacity = (ocr & (1 << 30)) != 0;
-                    sdcard.pendingOperation.callback(
+                } else {
-                        SDCARD_BLOCK_OPERATION_READ,
+                    // Version 1 cards are always low-capacity
-                        sdcard.pendingOperation.blockIndex,
+                    sdcard.highCapacity = false;
-                        sdcard.pendingOperation.buffer,
+                }
-                        sdcard.pendingOperation.callbackData
+
-                    );
+                if (sdcard_fetchCSD()) {
+                    sdcard_select();
+
+                    uint8_t status = sdcard_sendCommand(SDCARD_COMMAND_SEND_CID, 0);
+
+                    if (status == 0) {
+                        sdcard.state = SDCARD_STATE_INITIALIZATION_RECEIVE_CID;
+                        goto doMore;
+                    } else {
+                        sdcard_deselect();
+                    }
                 }
             }
         break;
-        case SDCARD_STATE_INITIALIZATION:
+        case SDCARD_STATE_INITIALIZATION_RECEIVE_CID:
-            sdcard_checkInitDone();
+            if (sdcard_receiveCID()) {
+                if (sdcard_setConfigurationAndFinalClock()) {
+                    sdcard.state = SDCARD_STATE_READY;
+                } else {
+                    // TODO we could reset the card here and try again
+                }
+            }
         break;
         case SDCARD_STATE_WRITING:
             if (sdcard_waitForIdle(SDCARD_MAXIMUM_BYTE_DELAY_FOR_CMD_REPLY)) {
-                DESELECT_SDCARD;
+                sdcard_deselect();
                 sdcard.state = SDCARD_STATE_READY;
             }
 
         break;
+        case SDCARD_STATE_READING:
+            switch (sdcard_receiveDataBlock(sdcard.pendingOperation.buffer, SDCARD_BLOCK_SIZE)) {
+                case SDCARD_RECEIVE_SUCCESS:
+                    sdcard_deselect();
+
+                    sdcard.state = SDCARD_STATE_READY;
+
+                    if (sdcard.pendingOperation.callback) {
+                        sdcard.pendingOperation.callback(
+                            SDCARD_BLOCK_OPERATION_READ,
+                            sdcard.pendingOperation.blockIndex,
+                            sdcard.pendingOperation.buffer,
+                            sdcard.pendingOperation.callbackData
+                        );
+                    }
+                break;
+                case SDCARD_RECEIVE_ERROR:
+                    sdcard_deselect();
+
+                    sdcard.state = SDCARD_STATE_READY;
+
+                    if (sdcard.pendingOperation.callback) {
+                        sdcard.pendingOperation.callback(
+                            SDCARD_BLOCK_OPERATION_READ,
+                            sdcard.pendingOperation.blockIndex,
+                            NULL,
+                            sdcard.pendingOperation.callbackData
+                        );
+                    }
+                break;
+                case SDCARD_RECEIVE_BLOCK_IN_PROGRESS:
+                    ;
+                break;
+            }
+        break;
         default:
             ;
     }
@@ -1301,7 +1357,7 @@ bool sdcard_writeBlock(uint32_t blockIndex, uint8_t *buffer)
     if (sdcard.state != SDCARD_STATE_READY)
         return false;
 
-    SELECT_SDCARD;
+    sdcard_select();
 
     // Standard size cards use byte addressing, high capacity cards use block addressing
     uint8_t status = sdcard_sendCommand(SDCARD_COMMAND_WRITE_BLOCK, sdcard.highCapacity ? blockIndex : blockIndex * SDCARD_BLOCK_SIZE);
@@ -1312,7 +1368,7 @@ bool sdcard_writeBlock(uint32_t blockIndex, uint8_t *buffer)
         // Leave the card selected while the write is in progress
         return true;
     } else {
-        DESELECT_SDCARD;
+        sdcard_deselect();
         return false;
     }
 }
@@ -1330,7 +1386,7 @@ bool sdcard_readBlock(uint32_t blockIndex, uint8_t *buffer, sdcard_operationComp
     if (sdcard.state != SDCARD_STATE_READY)
         return false;
 
-    SELECT_SDCARD;
+    sdcard_select();
 
     // Standard size cards use byte addressing, high capacity cards use block addressing
     uint8_t status = sdcard_sendCommand(SDCARD_COMMAND_READ_SINGLE_BLOCK, sdcard.highCapacity ? blockIndex : blockIndex * SDCARD_BLOCK_SIZE);
@@ -1346,7 +1402,7 @@ bool sdcard_readBlock(uint32_t blockIndex, uint8_t *buffer, sdcard_operationComp
 
         return true;
     } else {
-        DESELECT_SDCARD;
+        sdcard_deselect();
 
         return false;
     }

src/main/io/asyncfatfs/asyncfatfs.c

@@ -550,9 +550,15 @@ static void afatfs_sdcardReadComplete(sdcardBlockOperation_e operation, uint32_t
         if (afatfs.cacheDescriptor[i].state != AFATFS_CACHE_STATE_EMPTY
             && afatfs.cacheDescriptor[i].sectorIndex == sectorIndex
         ) {
-            afatfs_assert(afatfs_cacheSectorGetMemory(i) == buffer && afatfs.cacheDescriptor[i].state == AFATFS_CACHE_STATE_READING);
+            if (buffer == NULL) {
+                // Read failed, mark the sector as empty and whoever asked for it will ask for it again later to retry
+                afatfs.cacheDescriptor[i].state = AFATFS_CACHE_STATE_EMPTY;
+            } else {
+                afatfs_assert(afatfs_cacheSectorGetMemory(i) == buffer && afatfs.cacheDescriptor[i].state == AFATFS_CACHE_STATE_READING);
+
+                afatfs.cacheDescriptor[i].state = AFATFS_CACHE_STATE_IN_SYNC;
+            }
 
-            afatfs.cacheDescriptor[i].state = AFATFS_CACHE_STATE_IN_SYNC;
             break;
         }
     }
@@ -1019,6 +1025,9 @@ static afatfsFindClusterStatus_e afatfs_findClusterWithCondition(afatfsClusterSe
         case CLUSTER_SEARCH_FREE_SECTOR:
             jump = 1;
         break;
+        default:
+            afatfs_assert(false);
+            return AFATFS_FIND_CLUSTER_FATAL;
     }
 
     while (*cluster < searchLimit) {
@@ -3043,7 +3052,7 @@ static void afatfs_initContinue()
                     afatfs_chdir(NULL);
 
 #ifdef AFATFS_USE_FREEFILE
-                    afatfs_createFile(&afatfs.freeFile, AFATFS_FREESPACE_FILENAME, FAT_FILE_ATTRIBUTE_SYSTEM,
+                    afatfs_createFile(&afatfs.freeFile, AFATFS_FREESPACE_FILENAME, FAT_FILE_ATTRIBUTE_SYSTEM | FAT_FILE_ATTRIBUTE_READ_ONLY,
                         AFATFS_FILE_MODE_CREATE | AFATFS_FILE_MODE_RETAIN_DIRECTORY, afatfs_freeFileCreated);
                     afatfs.initPhase = AFATFS_INITIALIZATION_FREEFILE_CREATING;
 #else
@@ -3151,8 +3160,6 @@ void afatfs_init()
     afatfs.filesystemState = AFATFS_FILESYSTEM_STATE_INITIALIZATION;
     afatfs.initPhase = AFATFS_INITIALIZATION_READ_MBR;
     afatfs.lastClusterAllocated = 1;
-
-    afatfs_poll();
 }
 
 /**