espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.44k stars 7.38k forks source link

USBMSC writing 1st sector data to SD, then writing zeros #7106

Open frankcohen opened 2 years ago

frankcohen commented 2 years ago

Board

ESP32-S3-Mini-1

Device Description

S3 connects to USB directly. NAND/SD memory connected over SPI. Wiring guide at https://github.com/frankcohen/ReflectionsOS/blob/main/Devices/Hoober/Software/USB-MSC/USB-MSC-Wiring-Guide.jpg

Hardware Configuration

define NAND_SPI_MOSI 11

define NAND_SPI_MISO 13

define NAND_SPI_SCK 12

define NAND_SPI_CS 42

Version

v2.0.4

IDE Name

Arduino IDE 1.8.19

Operating System

MacOS 12.5

Flash frequency

20 Mhz

PSRAM enabled

no

Upload speed

115200

Description

I wrote an Adruino sketch to mount a SD card connected to an ESP32-S3 over SPI as a USB MSC volume. I plug the ESP32-S3 USB into my Mac laptop (MacOS Monterey 12.5) and the volume appears mounted correctly. I copy a text file to the volume, the original file is 2,526 bytes. The file on the ESP32 volume is also 2,526 bytes. However, only the first 512 bytes are copied from the original file, the additional bytes are zeroes.

I am reading and writing the data using: return SD.writeRAW( (uint8_t) buffer, lba) ? bufsize : -1 ; and return SD.readRAW( (uint8_t) buffer, lba) ? bufsize : -1 ;

Perhaps I should be returning bufsize/512?

Sketch

#include "SD.h"
#include "SPI.h"
#include "USB.h"
#include "USBMSC.h"

USBMSC msc;

#define NAND_SPI_MOSI 11
#define NAND_SPI_MISO 13
#define NAND_SPI_SCK 12
#define NAND_SPI_CS 42

bool sd_changed = false;
bool sd_inited = false;

static int32_t onWrite(uint32_t lba, uint32_t offset, uint8_t* buffer, uint32_t bufsize)
{
Serial.printf("MSC WRITE: lba: %u, offset: %u, bufsize: %u\n", lba, offset, bufsize);
return SD.writeRAW( (uint8_t*) buffer, lba) ? bufsize : -1 ;
}

static int32_t onRead(uint32_t lba, uint32_t offset, void* buffer, uint32_t bufsize)
{
Serial.printf("MSC READ: lba: %u, offset: %u, bufsize: %u\n", lba, offset, bufsize);
return SD.readRAW( (uint8_t*) buffer, lba) ? bufsize : -1 ;
}

static bool onStartStop(uint8_t power_condition, bool start, bool load_eject){
Serial.printf("MSC START/STOP: power: %u, start: %u, eject: %u\n", power_condition, start, load_eject);
return true;
}

void setup() {
Serial.begin(115200);
Serial.setDebugOutput(true);
long time = millis();
while (!Serial && ( millis() < time + 5000) ); // wait up to 5 seconds for Arduino Serial Monitor
Serial.println("");
Serial.println("MSC research for Hoober");

pinMode(NAND_SPI_CS, OUTPUT);
digitalWrite(NAND_SPI_CS, LOW);

static SPIClass* spi = NULL;
spi = new SPIClass(FSPI);
spi->begin(NAND_SPI_SCK, NAND_SPI_MISO, NAND_SPI_MOSI, NAND_SPI_CS);

if ( !SD.begin( NAND_SPI_CS, *spi, 20000000 ) )
{
Serial.println(F("Storage initialization failed"));
Serial.println("Stopped");
while(1);
}
else
{
Serial.println(F("Storage initialization success"));
}

Serial.print( "card size = " );
Serial.print( SD.cardSize() );
Serial.print( ", numSectors = " );
Serial.print( SD.numSectors() );
Serial.print( ", bytes per sector = " );
Serial.print( SD.cardSize() / SD.numSectors() );
Serial.print( ", total bytes = " );
Serial.print( SD.totalBytes() );
Serial.print( ", usedBytes = " );
Serial.print( SD.usedBytes() );

Serial.print(", SD Card Type: ");
if(SD.cardType() == CARD_MMC){
Serial.println("MMC");
} else if(SD.cardType() == CARD_SD){
Serial.println("SDSC");
} else if(SD.cardType() == CARD_SDHC){
Serial.println("SDHC");
} else if(SD.cardType() == CARD_NONE){
Serial.println("No SD card attached");
}else {
Serial.println("UNKNOWN");
}

msc.vendorID("REF32");
msc.productID("USB_MSC");
msc.productRevision("1.0");
msc.onRead(onRead);
msc.onWrite(onWrite);
msc.onStartStop(onStartStop);
msc.mediaPresent(true);
msc.begin(SD.numSectors(), SD.cardSize() / SD.numSectors() );

USB.begin();

sd_changed = true; // to print contents initially
}

void listDir(fs::FS &fs, const char * dirname, uint8_t levels){
Serial.printf("Listing directory: %s\n", dirname);

File root = fs.open(dirname);
if(!root){
Serial.println("Failed to open directory");
return;
}
if(!root.isDirectory()){
Serial.println("Not a directory");
return;
}

File file = root.openNextFile();
while(file){
if(file.isDirectory()){
Serial.print(" DIR : ");
Serial.println(file.name());
if(levels){
listDir(fs, file.path(), levels -1);
}
} else {
Serial.print(" FILE: ");
Serial.print(file.name());
Serial.print(" SIZE: ");
Serial.println(file.size());
}
file = root.openNextFile();
}
}

void loop() {
if ( sd_changed )
{
Serial.println("SD contents:");
listDir(SD, "/", 0);
Serial.println();
sd_changed = false;
}

delay(1000); // refresh every 1 second
}

Debug Message

...
MSC READ: lba: 432, offset: 0, bufsize: 512
MSC READ: lba: 432, offset: 0, bufsize: 4096
MSC READ: lba: 440, offset: 0, bufsize: 4096
MSC WRITE: lba: 432, offset: 0, bufsize: 4096
MSC WRITE: lba: 440, offset: 0, bufsize: 4096
MSC READ: lba: 448, offset: 0, bufsize: 4096
MSC READ: lba: 456, offset: 0, bufsize: 4096
MSC WRITE: lba: 448, offset: 0, bufsize: 4096
MSC WRITE: lba: 456, offset: 0, bufsize: 4096
MSC READ: lba: 432, offset: 0, bufsize: 4096
MSC READ: lba: 440, offset: 0, bufsize: 4096
MSC WRITE: lba: 432, offset: 0, bufsize: 4096
...

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

frankcohen commented 2 years ago

Could it be esp32/hardware/eps32/2.0.3/libraries/SD/src/sd_diskio.cpp (line 694) is causing the problem?

bool sd_read_raw(uint8_t pdrv, uint8_t* buffer, DWORD sector) { return ff_sd_read(pdrv, buffer, sector, 1) == ESP_OK; }

It hard sets the number of sectors to read at 1 (512 bytes).

-Frank

frankcohen commented 2 years ago

Appears to be a bug between the USBMSC and SD code. For this to work with Espressif's ESP32 code I changed line 694 esp32/hardware/eps32/2.0.3/libraries/SD/src/sd_diskio.cpp:

return ff_sd_read(pdrv, buffer, sector, 1) == ESP_OK;

Change '1' to '8'

and line 699 change '1 to '8'

return ff_sd_write(pdrv, buffer, sector, 8) == ESP_OK;

I opened issue: https://github.com/espressif/arduino-esp32/issues/7106

-Frank

chegewara commented 2 years ago

The part of code you are suggesting is correct. The problem is either with driver on your PC or with SD card formatting.

MSC READ: lba: 432, offset: 0, bufsize: 512   <--- here bufsize and sector size is 512
MSC READ: lba: 432, offset: 0, bufsize: 4096 <--- here for some reason bufsize is 4096

I think the problem may be with MSC driver code, which is returning wrong sector size to PC.

Either way suggested change is wrong.

PS maybe the change in this particular function should be done to pass sector size to it, with default 512, then

return ff_sd_write(pdrv, buffer, sector, sector_size / 512) == ESP_OK;

but, this will also require to pass offset from MSC read/write callback and ff_sd_write does not use offset

EDIT i would rather suggest to test this in your code, without editing library

return SD.readRAW( (uint8_t*) buffer, lba) ? 512 : -1 ;
frankcohen commented 2 years ago

Thanks for the tips. I prefer not to change the library, when I can avoid it. Passing in the sector size is what I looked for originally and didn't find it. The MSC driver is MacOS 12.5 writing to a surface-mounted 1 Gbyte NAND over MSC/ESP32-S3/SPI.

A different question (since you sound like a great expert), with the MSC sketch running in the ESP32-S3, how does the MSC get cycles from the S3? There's nothing in the loop() to keep feeding the S3 processor time. I'm noting a >90% drop in processing time measured from the loop() when the MSC runs. -Frank

chegewara commented 2 years ago

I only sound like an expert, but some may not agree with that. USB is running its own task in background and i am guessing it may be the case, but its only guessing. @me-no-dev is author of esp32 arduino library and knows more about it

frankcohen commented 2 years ago

Thanks! I am glad to provide a patch for this issue, or test one proposed by @me-no-dev or anyone else. I'm glad to support the ESP32 effort. I will open a separate ticket on the MSC load issue.

anatoli-dp commented 1 year ago

The part of code you are suggesting is correct. The problem is either with driver on your PC or with SD card formatting.

MSC READ: lba: 432, offset: 0, bufsize: 512   <--- here bufsize and sector size is 512
MSC READ: lba: 432, offset: 0, bufsize: 4096 <--- here for some reason bufsize is 4096

I think the problem may be with MSC driver code, which is returning wrong sector size to PC.

Either way suggested change is wrong.

PS maybe the change in this particular function should be done to pass sector size to it, with default 512, then

return ff_sd_write(pdrv, buffer, sector, sector_size / 512) == ESP_OK;

but, this will also require to pass offset from MSC read/write callback and ff_sd_write does not use offset

EDIT i would rather suggest to test this in your code, without editing library

return SD.readRAW( (uint8_t*) buffer, lba) ? 512 : -1 ;

can confirm setting the block size does address incomplete writes/reads