From ba940baa0aff05ad26d265f32f1d185a1f410373 Mon Sep 17 00:00:00 2001
From: jacqueline <me@jacqueline.id.au>
Date: Fri, 29 Sep 2023 15:17:10 +1000
Subject: [PATCH] Add a lock around the SPI bus

This seems to have been the cause of recurring deadlocks that have been
difficult to repo.
---
 src/audio/fatfs_audio_input.cpp |  5 -----
 src/database/tag_parser.cpp     | 22 ++++++++++++++++------
 src/drivers/display.cpp         |  9 +++++++--
 src/drivers/include/spi.hpp     |  2 ++
 src/drivers/spi.cpp             |  6 ++++++
 5 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/src/audio/fatfs_audio_input.cpp b/src/audio/fatfs_audio_input.cpp
index 6039ff9d..54630a98 100644
--- a/src/audio/fatfs_audio_input.cpp
+++ b/src/audio/fatfs_audio_input.cpp
@@ -91,12 +91,7 @@ auto FatfsAudioInput::NextStream() -> std::shared_ptr<codecs::IStream> {
     {
       std::lock_guard<std::mutex> guard{new_stream_mutex_};
       // If the path is a future, then wait for it to complete.
-      // TODO(jacqueline): We should really make some kind of
-      // FreeRTOS-integrated way to block a task whilst awaiting a future.
       if (pending_path_) {
-        while (!pending_path_->Finished()) {
-          vTaskDelay(pdMS_TO_TICKS(100));
-        }
         auto res = pending_path_->Result();
         pending_path_.reset();
 
diff --git a/src/database/tag_parser.cpp b/src/database/tag_parser.cpp
index fe71089d..bec41507 100644
--- a/src/database/tag_parser.cpp
+++ b/src/database/tag_parser.cpp
@@ -14,6 +14,7 @@
 #include "esp_log.h"
 #include "ff.h"
 #include "opusfile.h"
+#include "spi.hpp"
 #include "tags.h"
 
 #include "memory_resource.hpp"
@@ -184,10 +185,14 @@ auto GenericTagParser::ReadAndParseTags(const std::pmr::string& path)
   libtags::Aux aux;
   auto out = std::make_shared<TrackTags>();
   aux.tags = out.get();
-  if (f_stat(path.c_str(), &aux.info) != FR_OK ||
-      f_open(&aux.file, path.c_str(), FA_READ) != FR_OK) {
-    ESP_LOGW(kTag, "failed to open file %s", path.c_str());
-    return {};
+  {
+    auto lock = drivers::acquire_spi();
+
+    if (f_stat(path.c_str(), &aux.info) != FR_OK ||
+        f_open(&aux.file, path.c_str(), FA_READ) != FR_OK) {
+      ESP_LOGW(kTag, "failed to open file %s", path.c_str());
+      return {};
+    }
   }
   // Fine to have this on the stack; this is only called on tasks with large
   // stacks anyway, due to all the string handling.
@@ -200,8 +205,13 @@ auto GenericTagParser::ReadAndParseTags(const std::pmr::string& path)
   ctx.aux = &aux;
   ctx.buf = buf;
   ctx.bufsz = kBufSize;
-  int res = tagsget(&ctx);
-  f_close(&aux.file);
+
+  int res;
+  {
+    auto lock = drivers::acquire_spi();
+    res = tagsget(&ctx);
+    f_close(&aux.file);
+  }
 
   if (res != 0) {
     // Parsing failed.
diff --git a/src/drivers/display.cpp b/src/drivers/display.cpp
index 48310074..a06320f4 100644
--- a/src/drivers/display.cpp
+++ b/src/drivers/display.cpp
@@ -34,6 +34,7 @@
 #include "gpios.hpp"
 #include "misc/lv_color.h"
 #include "soc/soc.h"
+#include "spi.hpp"
 #include "tasks.hpp"
 
 static const char* kTag = "DISPLAY";
@@ -267,8 +268,12 @@ void Display::SendTransaction(TransactionType type,
 
   gpio_set_level(kDisplayDr, type);
 
-  // TODO(jacqueline): Handle these errors.
-  esp_err_t ret = spi_device_transmit(handle_, &sTransaction);
+  esp_err_t ret;
+  {
+    auto lock = drivers::acquire_spi();
+    // TODO(jacqueline): Handle these errors.
+    ret = spi_device_transmit(handle_, &sTransaction);
+  }
   ESP_ERROR_CHECK(ret);
 }
 
diff --git a/src/drivers/include/spi.hpp b/src/drivers/include/spi.hpp
index 7dbc2aae..60638f71 100644
--- a/src/drivers/include/spi.hpp
+++ b/src/drivers/include/spi.hpp
@@ -6,11 +6,13 @@
 
 #pragma once
 
+#include <mutex>
 #include "esp_err.h"
 
 namespace drivers {
 
 esp_err_t init_spi(void);
 esp_err_t deinit_spi(void);
+std::lock_guard<std::mutex> acquire_spi(void);
 
 }  // namespace drivers
diff --git a/src/drivers/spi.cpp b/src/drivers/spi.cpp
index 93bf9cde..ba461b67 100644
--- a/src/drivers/spi.cpp
+++ b/src/drivers/spi.cpp
@@ -20,6 +20,8 @@ static const gpio_num_t kSpiSdoPin = GPIO_NUM_23;
 static const gpio_num_t kSpiSdiPin = GPIO_NUM_19;
 static const gpio_num_t kSpiSclkPin = GPIO_NUM_18;
 
+static std::mutex sSpiMutex{};
+
 esp_err_t init_spi(void) {
   spi_bus_config_t config = {
       .mosi_io_num = kSpiSdoPin,
@@ -52,4 +54,8 @@ esp_err_t deinit_spi(void) {
   return spi_bus_free(kSpiHost);
 }
 
+std::lock_guard<std::mutex> acquire_spi(void) {
+  return std::lock_guard<std::mutex>{sSpiMutex};
+}
+
 }  // namespace drivers