adafruit / Adafruit_TinyUSB_Arduino

Arduino library for TinyUSB
MIT License
470 stars 124 forks source link

ESP32-S2 msc_sdfat example crashes #178

Closed skobkars closed 2 years ago

skobkars commented 2 years ago

Operating System

MacOS

IDE version

1.8.19

Board

Adafruit ESP32-S2 Feather with BME280 Sensor - STEMMA QT (4MB Flash + 2 MB PSRAM) + Adafruit Micro SD SPI or SDIO Card Breakout Board connected as SPI device with CS = 15

ArduinoCore version

2.0.3

TinyUSB Library version

1.12.0

Sketch (attached txt file)

examples/MassStorage/msc_sdfat with two minor changes:

const int chipSelect = 15;
...
while ( !Serial ) delay(10);   // wait for native usb

What happened ?

If during loop() directory printing process one of the usb_msc.setReadWriteCallback callbacks is called, or in other words when USB driver is working on reading or writing SD card (SPI interface, CS = 15) the ESP32-S2 Feather crashes and reboots.

If I disable root. and file. calls in loop() the device is working normally as a USB storage device.

If I leave usb_msc.setUnitReady in FALSE state and keep fs_changed flag as TRUE, loop is printing directory listing periodically without any issues.

If both usb_msc.setUnitReady is TRUE, and access to SD card file system is enabled in loop(), the device crashes and restarts constantly.

How to reproduce ?

  1. Open Adafruit msc.sdfat example
  2. Change CS pin to 15
  3. Uncomment while(!Serial) line to enable serial monitor outputs
  4. Compile and upload the example

Debug Log

output.txt

Screenshots

No response

hathach commented 2 years ago

I run feather esp32s2 with adalogger, and has no issue with both Serial and MSC, therefore couldn't reproduce this issue.

However, from what you said, this is probably the conflict when both loop() and msc callback try to access SD card at the same time (especially when writing files), which cause conflict in SPI driver and cause an data corruption which lead to hardfault (or something like that). It is not an bug with tinyusb, but rather the example sketch's.

Maybe you should try to introduce a semaphore (or busy flag variable) to skip reading file in loop() when msc writing/reading is in the way.

skobkars commented 2 years ago

Are you saying that msc_sdfat example works on adalogger? For my project I need WiFi, MSC and access to the FAT filesystem, thus the choice of the board(s). But if adalogger works with MSC/FAT combination I may try that board with a WiFi breakout instead maybe.

BTW, Serial is not an issue here, as disabling it makes no difference - the sketch is still crashing the board.

As for using semaphore, how do I make sure that callbacks are not called while I am already checked, found it not in use and called file or directory methods in loop?

I could probably set another flag before calling those methods and wait until they finish before calling read/writeBlocks, but the callbacks are very time sensitive as I learned, so that may break the MSC anyway.

hathach commented 2 years ago

Are you saying that msc_sdfat example works on adalogger?

yes. The conflict is just my guess based on your input. You cannot prevent callback to be invoked, but you could either delay() within callback or simply set the unit to not ready when wanting to access the SD card. Let me know if adalogger work with stock example for you with Feather ESP32s2 (which are my testing hardware).

skobkars commented 2 years ago

What are the exact boards that work for you? Which ESP32-SP2 and which adalogger?

skobkars commented 2 years ago

As Adalogger also uses SPI for SD it should not be any different from my setup. The only minor difference is the CS pin used.

I am puzzled.

skobkars commented 2 years ago

Okay, I used millis() with 300 ms delay to fix the sketch to work for me. Attaching the working version here if you want to test it with your setup.

It does not print out directory list every second, but only after changes have been made to the file system.

100ms was the minimum that more or less reliably worked on my computer, 150ms was almost ideal, but failed a couple of times when I copied/deleted 100+ files. 300ms seems to be solid for now.

/*********************************************************************
 Adafruit invests time and resources providing this open source code,
 please support Adafruit and open-source hardware by purchasing
 products from Adafruit!

 MIT license, check LICENSE for more information
 Copyright (c) 2019 Ha Thach for Adafruit Industries
 All text above, and the splash screen below must be included in
 any redistribution
*********************************************************************/

/* This example expose SD card as mass storage using
 * SdFat Library
 */

#include "SPI.h"
#include "SdFat.h"
#include "Adafruit_TinyUSB.h"

const int chipSelect = 15;

// File system on SD Card
SdFat sd;

SdFile root;
SdFile file;

// USB Mass Storage object
Adafruit_USBD_MSC usb_msc;

#define MSC_DELAY 300
#define MSC_FREE msc_accessed_at && millis()>(msc_accessed_at+MSC_DELAY)
#define MSC_WAIT while(millis()<=(msc_accessed_at+MSC_DELAY) ) yield()
#define MSC_WAIT_RESET msc_accessed_at=millis()
unsigned long msc_accessed_at = 0L;

// Set to true when PC write to flash
bool fs_changed;

// the setup function runs once when you press reset or power the board
void setup()
{
  pinMode(LED_BUILTIN, OUTPUT);

  // Set disk vendor id, product id and revision with string up to 8, 16, 4 characters respectively
  usb_msc.setID("Adafruit", "SD Card", "1.0");

  // Set read write callback
  usb_msc.setReadWriteCallback(msc_read_cb, msc_write_cb, msc_flush_cb);

  // Still initialize MSC but tell usb stack that MSC is not ready to read/write
  // If we don't initialize, board will be enumerated as CDC only
  usb_msc.setUnitReady(false);
  usb_msc.begin();

  Serial.begin(115200);
  while ( !Serial ) delay(10);   // wait for native usb

  Serial.println("Adafruit TinyUSB Mass Storage SD Card example");

  Serial.print("\nInitializing SD card ... ");
  Serial.print("CS = "); Serial.println(chipSelect);

  if ( !sd.begin(chipSelect, SD_SCK_MHZ(50)) )
  {
    Serial.println("initialization failed. Things to check:");
    Serial.println("* is a card inserted?");
    Serial.println("* is your wiring correct?");
    Serial.println("* did you change the chipSelect pin to match your shield or module?");
    while (1) delay(1);
  }

  // Size in blocks (512 bytes)
  uint32_t block_count = sd.card()->cardSize();

  Serial.print("Volume size (MB):  ");
  Serial.println((block_count/2) / 1024);

  // Set disk size, SD block size is always 512
  usb_msc.setCapacity(block_count, 512);

  // MSC is ready for read/write
  usb_msc.setUnitReady(true);

  fs_changed = true; // to print contents initially
}

void loop()
{
  if ( fs_changed && MSC_FREE )
  {
    root.open("/");
    Serial.println("SD contents:");

    // Open next file in root.
    // Warning, openNext starts at the current directory position
    // so a rewind of the directory may be required.
    MSC_WAIT;
    while ( file.openNext(&root, O_RDONLY) )
    {
      MSC_WAIT;
      file.printFileSize(&Serial);
      Serial.write(' ');

      MSC_WAIT;
      file.printName(&Serial);
      if ( file.isDir() )
      {
        // Indicate a directory.
        Serial.write('/');
      }
      Serial.println();
      file.close();
    }

    root.close();

    Serial.println();

    MSC_WAIT_RESET;
    fs_changed = false;
  }
}

// Callback invoked when received READ10 command.
// Copy disk's data to buffer (up to bufsize) and
// return number of copied bytes (must be multiple of block size)
int32_t msc_read_cb (uint32_t lba, void* buffer, uint32_t bufsize)
{
  MSC_WAIT_RESET;
  return sd.card()->readBlocks(lba, (uint8_t*) buffer, bufsize/512) ? bufsize : -1;
}

// Callback invoked when received WRITE10 command.
// Process data in buffer to disk's storage and 
// return number of written bytes (must be multiple of block size)
int32_t msc_write_cb (uint32_t lba, uint8_t* buffer, uint32_t bufsize)
{
  MSC_WAIT_RESET;
  digitalWrite(LED_BUILTIN, HIGH);

  return sd.card()->writeBlocks(lba, buffer, bufsize/512) ? bufsize : -1;
}

// Callback invoked when WRITE10 command is completed (status received and accepted by host).
// used to flush any pending cache.
void msc_flush_cb (void)
{
  MSC_WAIT_RESET;

  sd.card()->syncBlocks();

  // clear file system's cache to force refresh
  sd.cacheClear();

  fs_changed = true;

  digitalWrite(LED_BUILTIN, LOW);
}
hathach commented 2 years ago

What are the exact boards that work for you? Which ESP32-SP2 and which adalogger?

I used feather esp32s2 + adalogger

when I copied/deleted 100+ files. 300ms seems to be solid for now.

I don't copy files, just stop and running the sketch as reproducing step said.

Attaching the working version here if you want to test it with your setup.

adding time delay may not fix the issue, it probably just make the issue harder to reproduce. I won't try to fix this since this is off-USB and purely the SPI/SD peripheral usage conflict between the loop() and the callback. The loop print out directory is for visual demonstration only, you can just remove it entirely.

skobkars commented 2 years ago

I connected debug pin to serial port and ran the crashing code again. This is what I get:

assert failed: xQueueGenericSend queue.c:832 (pxQueue->pcHead != ((void *)0) || pxQueue->u.xSemaphore.xMutexHolder == ((void *)0) || pxQueue->u.xSemaphore.xMutexHolder == xTaskGetCurrentTaskHandle())

Backtrace:0x40026ff6:0x3ffd3d700x4002c75d:0x3ffd3d90 0x4003193d:0x3ffd3db0 0x4002d01e:0x3ffd3ee0 0x40095e25:0x3ffd3f20 0x40083dd6:0x3ffd3f40 0x400951b1:0x3ffd3f60 0x40094b21:0x3ffd3f80 0x40094c81:0x3ffd3fa0 0x40095135:0x3ffd3fc0 0x40082df2:0x3ffd3fe0 0x4008423b:0x3ffd4000 0x400854a5:0x3ffd4020 0x400cba71:0x3ffd4090 0x400962ab:0x3ffd40e0 

ELF file SHA256: 0000000000000000

Rebooting...

I tracked that assert to esp32/2.0.3/tools/sdk/esp32s2/qspi_qspi/libfreertos.a so it has to do with using FreeRTOS.

It will take me forever to figure this on out, so I thought I'd share it and see if it tells you something right away.

Thanks

hathach commented 2 years ago

the trace literally prove my mentioned point above. This looks more to the spi/sd conflict than usb issue, maybe you should find an way to access SD on multiple thread safely. This is out of my knowledge and I couldn't help any further. Issue is closed since it is off-USB

skobkars commented 2 years ago

Just wanted to share here the (temporary?) solution to the issue. Apparently, it was not a problem before ESP32 Core version prior to 2.0.0 as per https://github.com/espressif/arduino-esp32/issues/6079

Unfortunately, I cannot downgrade the Core because my board (Adafruit ESP32-S2 Feather with BME280 Sensor - STEMMA QT) is not supported in any version earlier than 2.0.3.

However, following the hint in the linked above discussion about disabling HAL locks I set CONFIG_DISABLE_HAL_LOCKS to 1 and bingo! My sketch is working fine now.

As ESP32-S2 only has one core, HAL locks appear to be not important anyway.

hathach commented 2 years ago

thank you for sharing the solution, I am sure other with the same issue will appreciate your finding. Too bad this is out of my knowledge to help with.

bassjansson commented 1 year ago

For anyone stumbling on this issue, on my esp32-s2 with the latest Arduino ESP32 Core this example sketch still crashes. Must admit, it's too bad the example doesn't work at the moment. It indeed seems that it's related to the SPI class or how SdFat is accessing it, so nothing to do with TinyUSB. Even when using SdFat together with another MSC library in just the same way, the program will hang.

I ended up leaving TinyUSB and using the USBMSC class which comes with the ESP32 Core. After digging into the libraries and testing some different setups I discovered that I could make the code working by ending SdFat together with the SPI class by calling sd.end(); in the start/stop callback in case of a disk eject. In this way, the esp is being a mass storage until the point the user ejects, after that it will continue with the rest of the program. Not too great, but okay for my purposes now.