Fix sd_mmc_spi_mem capacity; clean up USB code (#12134)

- Fix an error in the return value of `sd_mmc_spi_read_capacity` which was causing the host OS to read beyond the last sector in the card.
- Clean up the USB flashdrive code and add better debugging.

Marlin/src/HAL/HAL_DUE/usb/sd_mmc_spi_mem.cpp

@@ -24,10 +24,12 @@ Ctrl_status sd_mmc_spi_test_unit_ready(void) {
   return CTRL_GOOD;
 }
 
+// NOTE: This function is defined as returning the address of the last block
+// in the card, which is cardSize() - 1
 Ctrl_status sd_mmc_spi_read_capacity(uint32_t *nb_sector) {
   if (!IS_SD_INSERTED || IS_SD_PRINTING || IS_SD_FILE_OPEN || !card.cardOK)
     return CTRL_NO_PRESENT;
-  *nb_sector = card.getSd2Card().cardSize();
+  *nb_sector = card.getSd2Card().cardSize() - 1;
   return CTRL_GOOD;
 }
 

Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.cpp

@@ -24,49 +24,22 @@
 
 #if ENABLED(USB_FLASH_DRIVE_SUPPORT)
 
+#include "../../core/serial.h"
+
 #include "lib/Usb.h"
 #include "lib/masstorage.h"
 
 #include "Sd2Card_FlashDrive.h"
 
-#include <SPI.h>
-
-#include "../../core/serial.h"
-
 USB usb;
 BulkOnly bulk(&usb);
 
 Sd2Card::state_t Sd2Card::state;
-uint32_t         Sd2Card::block;
-
-bool Sd2Card::usbHostReady() {
-  return state == USB_HOST_INITIALIZED;
-}
-
-bool Sd2Card::isInserted() {
-  return usb.getUsbTaskState() == USB_STATE_RUNNING;
-}
-
-// Marlin calls this whenever an SD card is detected, so this method
-// should not be used to initialize the USB host library
-bool Sd2Card::init(uint8_t sckRateID, uint8_t chipSelectPin) {
-  if (!usbHostReady()) return false;
 
-  if (!bulk.LUNIsGood(0)) {
-    SERIAL_ECHOLNPGM("LUN zero is not good\n");
-    return false;
-  }
-
-  SERIAL_ECHOLNPAIR("LUN Capacity: ",bulk.GetCapacity(0));
-
-  const uint32_t sectorSize = bulk.GetSectorSize(0);
-  if (sectorSize != 512) {
-    SERIAL_ECHOLNPAIR("Expecting sector size of 512, got: ",sectorSize);
-    return false;
-  }
-
-  return true;
-}
+// The USB library needs to be called periodically to detect USB thumbdrive
+// insertion and removals. Call this idle() function periodically to allow
+// the USB libary to monitor for such events. This function also takes care
+// of initializing the USB library for the first time.
 
 void Sd2Card::idle() {
   static uint32_t next_retry;
@@ -100,60 +73,84 @@ void Sd2Card::idle() {
 
       if (lastUsbTaskState == USB_STATE_RUNNING && newUsbTaskState != USB_STATE_RUNNING) {
         // the user pulled the flash drive. Make sure the bulk storage driver releases the address
-        SERIAL_ECHOLNPGM("Drive removed\n");
+        #ifdef USB_DEBUG
+          SERIAL_ECHOLNPGM("USB drive removed");
+        #endif
         //bulk.Release();
       }
-      if (lastUsbTaskState != USB_STATE_RUNNING && newUsbTaskState == USB_STATE_RUNNING)
-        SERIAL_ECHOLNPGM("Drive inserted\n");
+      if (lastUsbTaskState != USB_STATE_RUNNING && newUsbTaskState == USB_STATE_RUNNING) {
+        #ifdef USB_DEBUG
+          SERIAL_ECHOLNPGM("USB drive inserted");
+        #endif
+      }
       break;
   }
 }
 
-uint32_t Sd2Card::cardSize() {
-  if (!usbHostReady()) return 0;
-  return bulk.GetCapacity(0);
-}
-
-bool Sd2Card::readData(uint8_t* dst) {
-  return readBlock(block++, dst);
-}
+// Marlin calls this function to check whether an USB drive is inserted.
+// This is equivalent to polling the SD_DETECT when using SD cards.
+bool Sd2Card::isInserted() {
+  return usb.getUsbTaskState() == USB_STATE_RUNNING;
+};
 
-bool Sd2Card::readStart(uint32_t blockNumber) {
-  block = blockNumber;
-  return true;
-}
+// Marlin calls this to initialize an SD card once it is inserted.
+bool Sd2Card::init(uint8_t sckRateID, uint8_t chipSelectPin) {
+  if (!ready()) return false;
 
-bool Sd2Card::readStop() {
-  return usbHostReady();
-}
+  if (!bulk.LUNIsGood(0)) {
+    SERIAL_ECHOLNPGM("LUN zero is not good");
+    return false;
+  }
 
-bool Sd2Card::readBlock(uint32_t block, uint8_t* dst) {
-  if (!usbHostReady()) {
-    SERIAL_ECHOLNPGM("Read from uninitalized USB host");
+  const uint32_t sectorSize = bulk.GetSectorSize(0);
+  if (sectorSize != 512) {
+    SERIAL_ECHOLNPAIR("Expecting sector size of 512, got:", sectorSize);
     return false;
   }
-  return bulk.Read(0, block, 512, 1, dst) == 0;
-}
 
-bool Sd2Card::writeData(const uint8_t* src) {
-  return writeBlock(block++, src);
+  #ifdef USB_DEBUG
+    lun0_capacity = bulk.GetCapacity(0);
+    SERIAL_ECHOLNPAIR("LUN Capacity (in blocks): ", lun0_capacity);
+  #endif
+  return true;
 }
 
-bool Sd2Card::writeStart(uint32_t blockNumber, uint32_t eraseCount) {
-  block = blockNumber;
-  return true;
+// Returns the capacity of the card in blocks.
+uint32_t Sd2Card::cardSize() {
+  if (!ready()) return 0;
+  #ifndef USB_DEBUG
+    const uint32_t
+  #endif
+  lun0_capacity = bulk.GetCapacity(0);
+  return lun0_capacity;
 }
 
-bool Sd2Card::writeStop() {
-  return usbHostReady();
+bool Sd2Card::readBlock(uint32_t block, uint8_t* dst) {
+  if (!ready()) return false;
+  #ifdef USB_DEBUG
+    if (block >= lun0_capacity) {
+      SERIAL_ECHOLNPAIR("Attempt to read past end of LUN: ", block);
+      return false;
+    }
+    #if USB_DEBUG > 1
+      SERIAL_ECHOLNPAIR("Read block ", block);
+    #endif
+  #endif
+  return bulk.Read(0, block, 512, 1, dst) == 0;
 }
 
-bool Sd2Card::writeBlock(uint32_t blockNumber, const uint8_t* src) {
-  if (!usbHostReady()) {
-    SERIAL_ECHOLNPGM("Write to uninitalized USB host");
-    return false;
-  }
-  return bulk.Write(0, blockNumber, 512, 1, src) == 0;
+bool Sd2Card::writeBlock(uint32_t block, const uint8_t* src) {
+  if (!ready()) return false;
+  #ifdef USB_DEBUG
+    if (block >= lun0_capacity) {
+      SERIAL_ECHOLNPAIR("Attempt to write past end of LUN: ", block);
+      return false;
+    }
+    #if USB_DEBUG > 1
+      SERIAL_ECHOLNPAIR("Write block ", block);
+    #endif
+  #endif
+  return bulk.Write(0, block, 512, 1, src) == 0;
 }
 
 #endif // USB_FLASH_DRIVE_SUPPORT

Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.h

@@ -28,6 +28,13 @@
 #ifndef _SD2CARD_FLASHDRIVE_H_
 #define _SD2CARD_FLASHDRIVE_H_
 
+/* Uncomment USB_DEBUG to enable debugging.
+ *    1 - basic debugging and bounds checking
+ *    2 - print each block access
+ */
+//#define USB_DEBUG 1
+
+
 #include "../SdFatConfig.h"
 #include "../SdInfo.h"
 
@@ -63,29 +70,34 @@ class Sd2Card {
       USB_HOST_INITIALIZED
     } state_t;
 
-    static state_t  state;
-    static uint32_t block;
+    static state_t state;
+
+    uint32_t pos;
+    #ifdef USB_DEBUG
+      uint32_t lun0_capacity;
+    #endif
 
-    static bool usbHostReady();
+    static inline bool ready() {return state == USB_HOST_INITIALIZED;}
 
   public:
+    bool init(uint8_t sckRateID = 0, uint8_t chipSelectPin = SD_CHIP_SELECT_PIN);
+
     static void idle();
 
-    static bool isInserted();
+    bool readStart(uint32_t block)                       { pos = block; return ready(); }
+    bool readData(uint8_t* dst)                          { return readBlock(pos++, dst); }
+    bool readStop()                                      { return true; }
 
-    uint32_t cardSize();
+    bool writeStart(uint32_t block, uint32_t eraseCount) { pos = block; return ready(); }
+    bool writeData(uint8_t* src)                         { return writeBlock(pos++, src); }
+    bool writeStop()                                     { return true; }
 
-    bool init(uint8_t sckRateID = 0, uint8_t chipSelectPin = SD_CHIP_SELECT_PIN);
 
-    bool readData(uint8_t* dst);
-    bool readStart(uint32_t blockNumber);
-    bool readStop();
     bool readBlock(uint32_t block, uint8_t* dst);
-
-    bool writeData(const uint8_t* src);
-    bool writeStart(uint32_t blockNumber, uint32_t eraseCount);
-    bool writeStop();
     bool writeBlock(uint32_t blockNumber, const uint8_t* src);
+
+    uint32_t cardSize();
+    static bool isInserted();
 };
 
 #endif  // _SD2CARD_FLASHDRIVE_H_