🔧 Configurable SD card retry/timeout (#25340)

Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

Marlin/src/HAL/STM32/msc_sd.cpp

@@ -33,6 +33,12 @@
 #define BLOCK_SIZE 512
 #define PRODUCT_ID 0x29
 
+#ifndef SD_MULTIBLOCK_RETRY_CNT
+  #define SD_MULTIBLOCK_RETRY_CNT 1
+#elif SD_MULTIBLOCK_RETRY_CNT < 1
+  #error "SD_MULTIBLOCK_RETRY_CNT must be greater than or equal to 1."
+#endif
+
 class Sd2CardUSBMscHandler : public USBMscHandler {
 public:
   DiskIODriver* diskIODriver() {
@@ -58,19 +64,29 @@ public:
     // single block
     if (blkLen == 1) {
       hal.watchdog_refresh();
-      sd2card->writeBlock(blkAddr, pBuf);
+      return sd2card->writeBlock(blkAddr, pBuf);
-      return true;
     }
 
     // multi block optimization
-    sd2card->writeStart(blkAddr, blkLen);
+    bool done = false;
-    while (blkLen--) {
+    for (uint16_t rcount = SD_MULTIBLOCK_RETRY_CNT; !done && rcount--;) {
-      hal.watchdog_refresh();
+      uint8_t *cBuf = pBuf;
-      sd2card->writeData(pBuf);
+      sd2card->writeStart(blkAddr);
-      pBuf += BLOCK_SIZE;
+      bool okay = true;                   // Assume success
+      for (uint32 i = blkLen; i--;) {
+        hal.watchdog_refresh();
+        if (!sd2card->writeData(cBuf)) {  // Write. Did it fail?
+          sd2card->writeStop();           // writeStop for new writeStart
+          okay = false;                   // Failed, so retry
+          break;                          // Go to while... below
+        }
+        cBuf += BLOCK_SIZE;
+      }
+      done = okay;                        // Done if no error occurred
     }
-    sd2card->writeStop();
+
-    return true;
+    if (done) sd2card->writeStop();
+    return done;
   }
 
   bool Read(uint8_t *pBuf, uint32_t blkAddr, uint16_t blkLen) {
@@ -78,24 +94,32 @@ public:
     // single block
     if (blkLen == 1) {
       hal.watchdog_refresh();
-      sd2card->readBlock(blkAddr, pBuf);
+      return sd2card->readBlock(blkAddr, pBuf);
-      return true;
     }
 
     // multi block optimization
-    sd2card->readStart(blkAddr);
+    bool done = false;
-    while (blkLen--) {
+    for (uint16_t rcount = SD_MULTIBLOCK_RETRY_CNT; !done && rcount--;) {
-      hal.watchdog_refresh();
+      uint8_t *cBuf = pBuf;
-      sd2card->readData(pBuf);
+      sd2card->readStart(blkAddr);
-      pBuf += BLOCK_SIZE;
+      bool okay = true;                   // Assume success
+      for (uint32 i = blkLen; i--;) {
+        hal.watchdog_refresh();
+        if (!sd2card->readData(cBuf)) {   // Read. Did it fail?
+          sd2card->readStop();            // readStop for new readStart
+          okay = false;                   // Failed, so retry
+          break;                          // Go to while... below
+        }
+        cBuf += BLOCK_SIZE;
+      }
+      done = okay;                        // Done if no error occurred
     }
-    sd2card->readStop();
+
-    return true;
+    if (done) sd2card->readStop();
+    return done;
   }
 
-  bool IsReady() {
+  bool IsReady() { return diskIODriver()->isReady(); }
-    return diskIODriver()->isReady();
-  }
 };
 
 Sd2CardUSBMscHandler usbMscHandler;

Marlin/src/sd/Sd2Card.cpp

@@ -40,6 +40,37 @@
 
 #include "../MarlinCore.h"
 
+#if DISABLED(SD_NO_DEFAULT_TIMEOUT)
+  #ifndef SD_INIT_TIMEOUT
+    #define SD_INIT_TIMEOUT 2000u   // (ms) Init timeout
+  #elif SD_INIT_TIMEOUT < 0
+    #error "SD_INIT_TIMEOUT must be greater than or equal to 0."
+  #endif
+  #ifndef SD_ERASE_TIMEOUT
+    #define SD_ERASE_TIMEOUT 10000u // (ms) Erase timeout
+  #elif SD_ERASE_TIMEOUT < 0
+    #error "SD_ERASE_TIMEOUT must be greater than or equal to 0."
+  #endif
+  #ifndef SD_READ_TIMEOUT
+    #define SD_READ_TIMEOUT 300u    // (ms) Read timeout
+  #elif SD_READ_TIMEOUT < 0
+    #error "SD_READ_TIMEOUT must be greater than or equal to 0."
+  #endif
+  #ifndef SD_WRITE_TIMEOUT
+    #define SD_WRITE_TIMEOUT 600u   // (ms) Write timeout
+  #elif SD_WRITE_TIMEOUT < 0
+    #error "SD_WRITE_TIMEOUT must be greater than or equal to 0."
+  #endif
+#endif // SD_NO_DEFAULT_TIMEOUT
+
+#if ENABLED(SD_CHECK_AND_RETRY)
+  #ifndef SD_RETRY_COUNT
+    #define SD_RETRY_COUNT 3
+  #elif SD_RETRY_COUNT < 1
+    #error "SD_RETRY_COUNT must be greater than or equal to 1."
+  #endif
+#endif
+
 #if ENABLED(SD_CHECK_AND_RETRY)
   static bool crcSupported = true;
 
@@ -97,15 +128,16 @@ uint8_t DiskIODriver_SPI_SD::cardCommand(const uint8_t cmd, const uint32_t arg)
   // Select card
   chipSelect();
 
-  // Wait up to 300 ms if busy
+  #if SD_WRITE_TIMEOUT
-  waitNotBusy(SD_WRITE_TIMEOUT);
+    waitNotBusy(SD_WRITE_TIMEOUT);  // Wait up to 600 ms (by default) if busy
+  #endif
 
   uint8_t *pa = (uint8_t *)(&arg);
 
   #if ENABLED(SD_CHECK_AND_RETRY)
 
     // Form message
-    uint8_t d[6] = {(uint8_t) (cmd | 0x40), pa[3], pa[2], pa[1], pa[0] };
+    uint8_t d[6] = { uint8_t(cmd | 0x40), pa[3], pa[2], pa[1], pa[0] };
 
     // Add crc
     d[5] = CRC7(d, 5);
@@ -186,33 +218,42 @@ void DiskIODriver_SPI_SD::chipSelect() {
 bool DiskIODriver_SPI_SD::erase(uint32_t firstBlock, uint32_t lastBlock) {
   if (ENABLED(SDCARD_READONLY)) return false;
 
-  csd_t csd;
+  bool success = false;
-  if (!readCSD(&csd)) goto FAIL;
+  do {
 
-  // check for single block erase
+    csd_t csd;
-  if (!csd.v1.erase_blk_en) {
+    if (!readCSD(&csd)) break;
-    // erase size mask
+
-    uint8_t m = (csd.v1.sector_size_high << 1) | csd.v1.sector_size_low;
+    // check for single block erase
-    if ((firstBlock & m) != 0 || ((lastBlock + 1) & m) != 0) {
+    if (!csd.v1.erase_blk_en) {
-      // error card can't erase specified area
+      // erase size mask
-      error(SD_CARD_ERROR_ERASE_SINGLE_BLOCK);
+      uint8_t m = (csd.v1.sector_size_high << 1) | csd.v1.sector_size_low;
-      goto FAIL;
+      if ((firstBlock & m) || ((lastBlock + 1) & m)) {
+        // error card can't erase specified area
+        error(SD_CARD_ERROR_ERASE_SINGLE_BLOCK);
+        break;
+      }
     }
-  }
+    if (type_ != SD_CARD_TYPE_SDHC) { firstBlock <<= 9; lastBlock <<= 9; }
-  if (type_ != SD_CARD_TYPE_SDHC) { firstBlock <<= 9; lastBlock <<= 9; }
+    if (cardCommand(CMD32, firstBlock) || cardCommand(CMD33, lastBlock) || cardCommand(CMD38, 0)) {
-  if (cardCommand(CMD32, firstBlock) || cardCommand(CMD33, lastBlock) || cardCommand(CMD38, 0)) {
+      error(SD_CARD_ERROR_ERASE);
-    error(SD_CARD_ERROR_ERASE);
+      break;
-    goto FAIL;
+    }
-  }
+    #if SD_ERASE_TIMEOUT
-  if (!waitNotBusy(SD_ERASE_TIMEOUT)) {
+      if (!waitNotBusy(SD_ERASE_TIMEOUT)) {
-    error(SD_CARD_ERROR_ERASE_TIMEOUT);
+        error(SD_CARD_ERROR_ERASE_TIMEOUT);
-    goto FAIL;
+        break;
-  }
+      }
+    #else
+      while (spiRec() != 0xFF) {}
+    #endif
+
+    success = true;
+
+  } while (0);
+
   chipDeselect();
-  return true;
+  return success;
-  FAIL:
-  chipDeselect();
-  return false;
 }
 
 /**
@@ -245,8 +286,15 @@ bool DiskIODriver_SPI_SD::init(const uint8_t sckRateID, const pin_t chipSelectPi
 
   errorCode_ = type_ = 0;
   chipSelectPin_ = chipSelectPin;
+
   // 16-bit init start time allows over a minute
-  const millis_t init_timeout = millis() + SD_INIT_TIMEOUT;
+  #if SD_INIT_TIMEOUT
+    const millis_t init_timeout = millis() + SD_INIT_TIMEOUT;
+    #define INIT_TIMEOUT() ELAPSED(millis(), init_timeout)
+  #else
+    #define INIT_TIMEOUT() false
+  #endif
+
   uint32_t arg;
 
   hal.watchdog_refresh(); // In case init takes too long
@@ -274,7 +322,7 @@ bool DiskIODriver_SPI_SD::init(const uint8_t sckRateID, const pin_t chipSelectPi
 
   // Command to go idle in SPI mode
   while ((status_ = cardCommand(CMD0, 0)) != R1_IDLE_STATE) {
-    if (ELAPSED(millis(), init_timeout)) {
+    if (INIT_TIMEOUT()) {
       error(SD_CARD_ERROR_CMD0);
       goto FAIL;
     }
@@ -300,7 +348,7 @@ bool DiskIODriver_SPI_SD::init(const uint8_t sckRateID, const pin_t chipSelectPi
       break;
     }
 
-    if (ELAPSED(millis(), init_timeout)) {
+    if (INIT_TIMEOUT()) {
       error(SD_CARD_ERROR_CMD8);
       goto FAIL;
     }
@@ -312,11 +360,12 @@ bool DiskIODriver_SPI_SD::init(const uint8_t sckRateID, const pin_t chipSelectPi
   arg = type() == SD_CARD_TYPE_SD2 ? 0x40000000 : 0;
   while ((status_ = cardAcmd(ACMD41, arg)) != R1_READY_STATE) {
     // Check for timeout
-    if (ELAPSED(millis(), init_timeout)) {
+    if (INIT_TIMEOUT()) {
       error(SD_CARD_ERROR_ACMD41);
       goto FAIL;
     }
   }
+
   // If SD2 read OCR register to check for SDHC card
   if (type() == SD_CARD_TYPE_SD2) {
     if (cardCommand(CMD58, 0)) {
@@ -327,6 +376,7 @@ bool DiskIODriver_SPI_SD::init(const uint8_t sckRateID, const pin_t chipSelectPi
     // Discard rest of ocr - contains allowed voltage range
     for (uint8_t i = 0; i < 3; ++i) spiRec();
   }
+
   chipDeselect();
 
   ready = true;
@@ -353,7 +403,7 @@ bool DiskIODriver_SPI_SD::readBlock(uint32_t blockNumber, uint8_t * const dst) {
   if (type() != SD_CARD_TYPE_SDHC) blockNumber <<= 9;   // Use address if not SDHC card
 
   #if ENABLED(SD_CHECK_AND_RETRY)
-    uint8_t retryCnt = 3;
+    uint8_t retryCnt = SD_RETRY_COUNT;
     for (;;) {
       if (cardCommand(CMD17, blockNumber))
         error(SD_CARD_ERROR_CMD17);
@@ -458,9 +508,15 @@ bool DiskIODriver_SPI_SD::readData(uint8_t * const dst) {
 bool DiskIODriver_SPI_SD::readData(uint8_t * const dst, const uint16_t count) {
   bool success = false;
 
-  const millis_t read_timeout = millis() + SD_READ_TIMEOUT;
+  #if SD_READ_TIMEOUT
+    const millis_t read_timeout = millis() + SD_READ_TIMEOUT;
+    #define READ_TIMEOUT() ELAPSED(millis(), read_timeout)
+  #else
+    #define READ_TIMEOUT() false
+  #endif
+
   while ((status_ = spiRec()) == 0xFF) {      // Wait for start block token
-    if (ELAPSED(millis(), read_timeout)) {
+    if (READ_TIMEOUT()) {
       error(SD_CARD_ERROR_READ_TIMEOUT);
       goto FAIL;
     }
@@ -469,7 +525,7 @@ bool DiskIODriver_SPI_SD::readData(uint8_t * const dst, const uint16_t count) {
   if (status_ == DATA_START_BLOCK) {
     spiRead(dst, count);                      // Transfer data
 
-    const uint16_t recvCrc = (spiRec() << 8) | spiRec();
+    const uint16_t recvCrc = ((uint16_t)spiRec() << 8) | (uint16_t)spiRec();
     #if ENABLED(SD_CHECK_AND_RETRY)
       success = !crcSupported || recvCrc == CRC_CCITT(dst, count);
       if (!success) error(SD_CARD_ERROR_READ_CRC);
@@ -574,20 +630,23 @@ bool DiskIODriver_SPI_SD::writeBlock(uint32_t blockNumber, const uint8_t * const
     return 0 == SDHC_CardWriteBlock(src, blockNumber);
   #endif
 
-  bool success = false;
+  if (type() != SD_CARD_TYPE_SDHC) blockNumber <<= 9; // Use address if not SDHC card
-  if (type() != SD_CARD_TYPE_SDHC) blockNumber <<= 9;   // Use address if not SDHC card
+  bool success = !cardCommand(CMD24, blockNumber);
-  if (!cardCommand(CMD24, blockNumber)) {
+  if (!success) {
-    if (writeData(DATA_START_BLOCK, src)) {
+    error(SD_CARD_ERROR_CMD24);
-      if (waitNotBusy(SD_WRITE_TIMEOUT)) {              // Wait for flashing to complete
+  }
-        success = !(cardCommand(CMD13, 0) || spiRec()); // Response is r2 so get and check two bytes for nonzero
+  else if (writeData(DATA_START_BLOCK, src)) {
-        if (!success) error(SD_CARD_ERROR_WRITE_PROGRAMMING);
+    #if SD_WRITE_TIMEOUT
-      }
+      success = waitNotBusy(SD_WRITE_TIMEOUT);        // Wait for flashing to complete
-      else
+      if (!success) error(SD_CARD_ERROR_WRITE_TIMEOUT);
-        error(SD_CARD_ERROR_WRITE_TIMEOUT);
+    #else
+      while (spiRec() != 0xFF) {}
+    #endif
+    if (success) {
+      success = !(cardCommand(CMD13, 0) || spiRec()); // Response is r2 so get and check two bytes for nonzero
+      if (!success) error(SD_CARD_ERROR_WRITE_PROGRAMMING);
     }
   }
-  else
-    error(SD_CARD_ERROR_CMD24);
 
   chipDeselect();
   return success;
@@ -601,13 +660,25 @@ bool DiskIODriver_SPI_SD::writeBlock(uint32_t blockNumber, const uint8_t * const
 bool DiskIODriver_SPI_SD::writeData(const uint8_t * const src) {
   if (ENABLED(SDCARD_READONLY)) return false;
 
-  bool success = true;
   chipSelect();
-  // Wait for previous write to finish
+
-  if (!waitNotBusy(SD_WRITE_TIMEOUT) || !writeData(WRITE_MULTIPLE_TOKEN, src)) {
+  bool success = false;
-    error(SD_CARD_ERROR_WRITE_MULTIPLE);
+  do {
-    success = false;
+
-  }
+    // Wait for previous write to finish
+    #if SD_WRITE_TIMEOUT
+      if (!waitNotBusy(SD_WRITE_TIMEOUT)) {
+        error(SD_CARD_ERROR_WRITE_MULTIPLE);
+        break;
+      }
+    #else
+      while (spiRec() != 0xFF) {}
+    #endif
+
+    success = writeData(WRITE_MULTIPLE_TOKEN, src);
+
+  } while (0);
+
   chipDeselect();
   return success;
 }
@@ -665,14 +736,31 @@ bool DiskIODriver_SPI_SD::writeStart(uint32_t blockNumber, const uint32_t eraseC
 bool DiskIODriver_SPI_SD::writeStop() {
   if (ENABLED(SDCARD_READONLY)) return false;
 
-  bool success = false;
   chipSelect();
-  if (waitNotBusy(SD_WRITE_TIMEOUT)) {
+
+  bool success = false;
+  do {
+
+    #if SD_WRITE_TIMEOUT
+      if (!waitNotBusy(SD_WRITE_TIMEOUT)) {
+        error(SD_CARD_ERROR_STOP_TRAN);
+        break;
+      }
+    #else
+      while (spiRec() != 0xFF) {}
+    #endif
+
     spiSend(STOP_TRAN_TOKEN);
-    success = waitNotBusy(SD_WRITE_TIMEOUT);
+
-  }
+    #if SD_WRITE_TIMEOUT
-  else
+      if (!waitNotBusy(SD_WRITE_TIMEOUT)) break;
-    error(SD_CARD_ERROR_STOP_TRAN);
+    #else
+      while (spiRec() != 0xFF) {}
+    #endif
+
+    success = true;
+
+  } while (0);
 
   chipDeselect();
   return success;

Marlin/src/sd/Sd2Card.h

@@ -39,11 +39,6 @@
 
 #include <stdint.h>
 
-uint16_t const SD_INIT_TIMEOUT  = 2000,  // (ms) Init timeout
-               SD_ERASE_TIMEOUT = 10000, // (ms) Erase timeout
-               SD_READ_TIMEOUT  = 300,   // (ms) Read timeout
-               SD_WRITE_TIMEOUT = 600;   // (ms) Write timeout
-
 // SD card errors
 typedef enum : uint8_t {
   SD_CARD_ERROR_CMD0               = 0x01, // Timeout error for command CMD0 (initialize card in SPI mode)