Automatically acquire the CS lock when talking to the SD card

3 files changed, 35 insertions, 15 deletions

diff --git a/main/gay-ipod-fw.cpp b/main/gay-ipod-fw.cpp
index f9ae00b0..7d91d5b2 100644
--- a/main/gay-ipod-fw.cpp
+++ b/main/gay-ipod-fw.cpp
@@ -123,19 +123,16 @@ extern "C" void app_main(void)
   ESP_LOGI(TAG, "Looks okay? Let's list some files!");
   vTaskDelay(pdMS_TO_TICKS(1000));
 
-  {
-    auto lock = expander.AcquireSpiBus(gay_ipod::GpioExpander::SD_CARD);
-    DIR *d;
-    struct dirent *dir;
-    d = opendir(gay_ipod::kStoragePath);
-    if (d) {
-      while ((dir = readdir(d)) != NULL) {
-	ESP_LOGI(TAG, "file! %s", dir->d_name);
-      }
-      closedir(d);
-    } else {
-      ESP_LOGI(TAG, "nope!");
+  DIR *d;
+  struct dirent *dir;
+  d = opendir(gay_ipod::kStoragePath);
+  if (d) {
+    while ((dir = readdir(d)) != NULL) {
+      ESP_LOGI(TAG, "file! %s", dir->d_name);
     }
+    closedir(d);
+  } else {
+    ESP_LOGI(TAG, "nope!");
   }
 
   vTaskDelay(pdMS_TO_TICKS(1000));
diff --git a/main/storage.cpp b/main/storage.cpp
index e1f6f386..d2f88894 100644
--- a/main/storage.cpp
+++ b/main/storage.cpp
@@ -28,7 +28,6 @@ SdStorage::~SdStorage() {}
 SdStorage::Error SdStorage::Acquire(void) {
   // Acquiring the bus will also flush the mux switch change.
   gpio_->set_pin(GpioExpander::SD_MUX_SWITCH, GpioExpander::SD_MUX_ESP);
-  auto lock = gpio_->AcquireSpiBus(GpioExpander::SD_CARD);
 
   // Now we can init the driver and set up the SD card into SPI mode.
   sdspi_host_init();
@@ -44,6 +43,18 @@ SdStorage::Error SdStorage::Acquire(void) {
   ESP_ERROR_CHECK(sdspi_host_init_device(&config, &handle_));
 
   host_ = sdmmc_host_t SDSPI_HOST_DEFAULT();
+
+  // We manage the CS pin ourselves via the GPIO expander. To do this safely in
+  // a multithreaded environment, we wrap the ESP IDF do_transaction function
+  // with our own that acquires the CS mutex for the duration of the SPI
+  // transaction.
+  auto src = host_.do_transaction;
+  sdspi::do_transaction_wrapper = [=](sdspi_dev_handle_t handle, sdmmc_command_t *cmd) -> esp_err_t {
+    auto lock = gpio_->AcquireSpiBus(GpioExpander::SD_CARD);
+    return src(handle, cmd);
+  };
+  host_.do_transaction = &sdspi::do_transaction;
+
   host_.slot = handle_;
   // Will return ESP_ERR_INVALID_RESPONSE if there is no card
   esp_err_t err = sdmmc_card_init(&host_, &card_);
@@ -66,8 +77,6 @@ SdStorage::Error SdStorage::Acquire(void) {
 }
 
 void SdStorage::Release(void) {
-  auto lock = gpio_->AcquireSpiBus(GpioExpander::SD_CARD);
-
   // Unmount and unregister the filesystem
   f_unmount("");
   ff_diskio_register(fs_->pdrv, NULL);
diff --git a/main/storage.h b/main/storage.h
index 5db3685f..fc1f8f5d 100644
--- a/main/storage.h
+++ b/main/storage.h
@@ -8,6 +8,20 @@
 
 namespace gay_ipod {
 
+// Static functions for interrop with the ESP IDF API, which requires a function
+// pointer.
+namespace sdspi {
+  // Holds a lambda created by SdStorage.
+  static std::function<esp_err_t(sdspi_dev_handle_t,sdmmc_command_t*)> do_transaction_wrapper;
+
+  // Fits the required function pointer signature, but just delegates to the
+  // wrapper function. Does that make this the wrapper? Who knows.
+  __attribute__ ((unused)) // (gcc incorrectly thinks this is unused)
+  static esp_err_t do_transaction(sdspi_dev_handle_t handle, sdmmc_command_t *cmdinfo) {
+    return do_transaction_wrapper(handle, cmdinfo);
+  }
+} // namespace sdspi
+
 static const char *kStoragePath = "/sd";
 static const uint8_t kMaxOpenFiles = 8;