From 3e1fe1ff80f4410c9e05e190ced7550d3fe769a8 Mon Sep 17 00:00:00 2001 From: Martin Turski Date: Sun, 13 Aug 2023 22:57:38 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7=20Configurable=20SD=20card=20retry?= =?UTF-8?q?/timeout=20(#25340)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Scott Lahteine --- Marlin/src/HAL/STM32/msc_sd.cpp | 66 ++++++---- Marlin/src/sd/Sd2Card.cpp | 208 +++++++++++++++++++++++--------- Marlin/src/sd/Sd2Card.h | 5 - 3 files changed, 193 insertions(+), 86 deletions(-) diff --git a/Marlin/src/HAL/STM32/msc_sd.cpp b/Marlin/src/HAL/STM32/msc_sd.cpp index a40bec9d644d..f03f533a7195 100644 --- a/Marlin/src/HAL/STM32/msc_sd.cpp +++ b/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 @@ class Sd2CardUSBMscHandler : public USBMscHandler { // single block if (blkLen == 1) { hal.watchdog_refresh(); - sd2card->writeBlock(blkAddr, pBuf); - return true; + return sd2card->writeBlock(blkAddr, pBuf); } // multi block optimization - sd2card->writeStart(blkAddr, blkLen); - while (blkLen--) { - hal.watchdog_refresh(); - sd2card->writeData(pBuf); - pBuf += BLOCK_SIZE; + bool done = false; + for (uint16_t rcount = SD_MULTIBLOCK_RETRY_CNT; !done && rcount--;) { + uint8_t *cBuf = pBuf; + sd2card->writeStart(blkAddr); + 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 @@ class Sd2CardUSBMscHandler : public USBMscHandler { // single block if (blkLen == 1) { hal.watchdog_refresh(); - sd2card->readBlock(blkAddr, pBuf); - return true; + return sd2card->readBlock(blkAddr, pBuf); } // multi block optimization - sd2card->readStart(blkAddr); - while (blkLen--) { - hal.watchdog_refresh(); - sd2card->readData(pBuf); - pBuf += BLOCK_SIZE; + bool done = false; + for (uint16_t rcount = SD_MULTIBLOCK_RETRY_CNT; !done && rcount--;) { + uint8_t *cBuf = pBuf; + sd2card->readStart(blkAddr); + 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; - } - bool IsReady() { - return diskIODriver()->isReady(); + if (done) sd2card->readStop(); + return done; } + + bool IsReady() { return diskIODriver()->isReady(); } }; Sd2CardUSBMscHandler usbMscHandler; diff --git a/Marlin/src/sd/Sd2Card.cpp b/Marlin/src/sd/Sd2Card.cpp index 7deebd4776a2..25fc35e6ab93 100644 --- a/Marlin/src/sd/Sd2Card.cpp +++ b/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 - waitNotBusy(SD_WRITE_TIMEOUT); + #if 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; - if (!readCSD(&csd)) goto FAIL; - - // check for single block erase - if (!csd.v1.erase_blk_en) { - // erase size mask - uint8_t m = (csd.v1.sector_size_high << 1) | csd.v1.sector_size_low; - if ((firstBlock & m) != 0 || ((lastBlock + 1) & m) != 0) { - // error card can't erase specified area - error(SD_CARD_ERROR_ERASE_SINGLE_BLOCK); - goto FAIL; + bool success = false; + do { + + csd_t csd; + if (!readCSD(&csd)) break; + + // check for single block erase + if (!csd.v1.erase_blk_en) { + // erase size mask + uint8_t m = (csd.v1.sector_size_high << 1) | csd.v1.sector_size_low; + 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 (cardCommand(CMD32, firstBlock) || cardCommand(CMD33, lastBlock) || cardCommand(CMD38, 0)) { - error(SD_CARD_ERROR_ERASE); - goto FAIL; - } - if (!waitNotBusy(SD_ERASE_TIMEOUT)) { - error(SD_CARD_ERROR_ERASE_TIMEOUT); - goto FAIL; - } - chipDeselect(); - return true; - FAIL: + if (type_ != SD_CARD_TYPE_SDHC) { firstBlock <<= 9; lastBlock <<= 9; } + if (cardCommand(CMD32, firstBlock) || cardCommand(CMD33, lastBlock) || cardCommand(CMD38, 0)) { + error(SD_CARD_ERROR_ERASE); + break; + } + #if SD_ERASE_TIMEOUT + if (!waitNotBusy(SD_ERASE_TIMEOUT)) { + error(SD_CARD_ERROR_ERASE_TIMEOUT); + break; + } + #else + while (spiRec() != 0xFF) {} + #endif + + success = true; + + } while (0); + chipDeselect(); - return false; + return success; } /** @@ -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 (!cardCommand(CMD24, blockNumber)) { - if (writeData(DATA_START_BLOCK, src)) { - 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 - if (!success) error(SD_CARD_ERROR_WRITE_PROGRAMMING); - } - else - error(SD_CARD_ERROR_WRITE_TIMEOUT); + if (type() != SD_CARD_TYPE_SDHC) blockNumber <<= 9; // Use address if not SDHC card + bool success = !cardCommand(CMD24, blockNumber); + if (!success) { + error(SD_CARD_ERROR_CMD24); + } + else if (writeData(DATA_START_BLOCK, src)) { + #if SD_WRITE_TIMEOUT + success = waitNotBusy(SD_WRITE_TIMEOUT); // Wait for flashing to complete + if (!success) 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)) { - error(SD_CARD_ERROR_WRITE_MULTIPLE); - success = false; - } + + bool success = false; + do { + + // 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); - } - else - error(SD_CARD_ERROR_STOP_TRAN); + + #if SD_WRITE_TIMEOUT + if (!waitNotBusy(SD_WRITE_TIMEOUT)) break; + #else + while (spiRec() != 0xFF) {} + #endif + + success = true; + + } while (0); chipDeselect(); return success; diff --git a/Marlin/src/sd/Sd2Card.h b/Marlin/src/sd/Sd2Card.h index 49569af5121f..dd021364e033 100644 --- a/Marlin/src/sd/Sd2Card.h +++ b/Marlin/src/sd/Sd2Card.h @@ -39,11 +39,6 @@ #include -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)