From 5b7dd553d3543d2e1a9305eca6060e35fe2fbed9 Mon Sep 17 00:00:00 2001 From: Marcio Teixeira Date: Thu, 18 Oct 2018 10:34:07 -0600 Subject: [PATCH] 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 | 4 +- .../sd/usb_flashdrive/Sd2Card_FlashDrive.cpp | 137 +++++++++--------- .../sd/usb_flashdrive/Sd2Card_FlashDrive.h | 44 ++++-- 3 files changed, 98 insertions(+), 87 deletions(-) diff --git a/Marlin/src/HAL/HAL_DUE/usb/sd_mmc_spi_mem.cpp b/Marlin/src/HAL/HAL_DUE/usb/sd_mmc_spi_mem.cpp index 4de46104e2..df79fe6a7f 100644 --- a/Marlin/src/HAL/HAL_DUE/usb/sd_mmc_spi_mem.cpp +++ b/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; } diff --git a/Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.cpp b/Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.cpp index 369b3d38d4..94821463ad 100644 --- a/Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.cpp +++ b/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 - -#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); -} +// 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::readData(uint8_t* dst) { - return readBlock(block++, dst); -} +// 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::readStart(uint32_t blockNumber) { - block = blockNumber; + if (!bulk.LUNIsGood(0)) { + SERIAL_ECHOLNPGM("LUN zero is not good"); + return false; + } + + const uint32_t sectorSize = bulk.GetSectorSize(0); + if (sectorSize != 512) { + SERIAL_ECHOLNPAIR("Expecting sector size of 512, got:", sectorSize); + return false; + } + + #ifdef USB_DEBUG + lun0_capacity = bulk.GetCapacity(0); + SERIAL_ECHOLNPAIR("LUN Capacity (in blocks): ", lun0_capacity); + #endif return true; } -bool Sd2Card::readStop() { - return usbHostReady(); +// 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::readBlock(uint32_t block, uint8_t* dst) { - if (!usbHostReady()) { - SERIAL_ECHOLNPGM("Read from uninitalized USB host"); - return false; - } + 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::writeData(const uint8_t* src) { - return writeBlock(block++, src); -} - -bool Sd2Card::writeStart(uint32_t blockNumber, uint32_t eraseCount) { - block = blockNumber; - return true; -} - -bool Sd2Card::writeStop() { - return usbHostReady(); -} - -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 diff --git a/Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.h b/Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.h index c364063156..3b5a5fcba3 100644 --- a/Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.h +++ b/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; - static bool usbHostReady(); + uint32_t pos; + #ifdef USB_DEBUG + uint32_t lun0_capacity; + #endif + + static inline bool ready() {return state == USB_HOST_INITIALIZED;} public: - static void idle(); - - static bool isInserted(); - - uint32_t cardSize(); - 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); + static void idle(); - bool writeData(const uint8_t* src); - bool writeStart(uint32_t blockNumber, uint32_t eraseCount); - bool writeStop(); + bool readStart(uint32_t block) { pos = block; return ready(); } + bool readData(uint8_t* dst) { return readBlock(pos++, dst); } + bool readStop() { return true; } + + 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 readBlock(uint32_t block, uint8_t* dst); bool writeBlock(uint32_t blockNumber, const uint8_t* src); + + uint32_t cardSize(); + static bool isInserted(); }; #endif // _SD2CARD_FLASHDRIVE_H_