From a93c1d3c06171092a7f7c91f7fe0afb18ac85eed Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Wed, 29 Jul 2026 07:46:02 -0500 Subject: [PATCH] feat: take the SPI bus lock around our own transfers Boards that share the display's SPI bus with other peripherals - an SD card, a LoRa radio - serialize access with a host-side lock. Meshtastic's firmware held that lock across the *entire* UI cycle, because it has no finer-grained way to know when we touch the bus. Most of a cycle is LVGL timer work and rendering into the draw buffer, which issues no SPI at all, so on T-Deck every radio operation on the host's main loop waited out a whole render+flush - tens to hundreds of milliseconds while the UI animates, felt as mesh RX/TX latency. Invert the ownership. ISpiLock is passed to DeviceScreen::create(); we take it only around real transfers, so the bus stays free during the CPU-only majority of a redraw and the host can stop holding it at all. Guarded: the flush (pushImage), SPI touch reads, panel init/fillScreen, printConfig's readCommand, touch calibration, the sleep/wake/power-save panel commands, every SD entry point (SdFsCard, the SD_MMC SDCard, both lv_fs tile services, TFTView's backup/restore), and the LittleFS paths in FileLoader and LogRotate - those last two share the filesystem object the host also serializes with this lock, and were only covered before by the coarse hold this removes. The interface is also held in a static, because two classes of call site cannot reach an instance member: the LVGL flush and touch callbacks are static and have no 'this', and the SD layer (sdCard, SDFs) is file-scope globals with no route to a DeviceGUI. No guard nests inside another: usedBytes() computes from cluster counts rather than calling cardSize()/freeBytes(), and LogRotate guards only its public entry points. Implementations should still be reentrant - the header says so - but nothing here depends on it. Nothing is guarded across DisplayDriver::view->sleep(), which suspends the SoC in light sleep, nor across controller->sendConfig() or messageAlert() in backup/restore - the former hands work to the radio, which needs this same bus from another task. Tested on T-Deck: flush, touch, panel init, SD detect, powersave sleep/wake, FileLoader's boot image and LogRotate's log directory all exercised; LoRa RX decoding with a live UI, no deadlocks, no watchdog resets. --- include/graphics/DeviceScreen.h | 15 +++- include/graphics/driver/LGFXDriver.h | 73 ++++++++++++++----- include/util/ISpiLock.h | 71 ++++++++++++++++++ source/graphics/DeviceScreen.cpp | 13 +++- source/graphics/TFT/TFTView_320x240.cpp | 95 +++++++++++++++---------- source/graphics/common/SdCard.cpp | 27 ++++++- source/graphics/map/SDCardService.cpp | 8 +++ source/graphics/map/SdFatService.cpp | 8 +++ source/util/FileLoader.cpp | 9 +++ source/util/ISpiLock.cpp | 26 +++++++ source/util/LogRotate.cpp | 5 ++ 11 files changed, 290 insertions(+), 60 deletions(-) create mode 100644 include/util/ISpiLock.h create mode 100644 source/util/ISpiLock.cpp diff --git a/include/graphics/DeviceScreen.h b/include/graphics/DeviceScreen.h index c55afe01..51692953 100644 --- a/include/graphics/DeviceScreen.h +++ b/include/graphics/DeviceScreen.h @@ -3,6 +3,7 @@ #include "comms/IClientBase.h" #include "graphics/DeviceGUI.h" #include "graphics/driver/DisplayDriverConfig.h" +#include "util/ISpiLock.h" #if defined(ARDUINO_ARCH_ESP32) #include "esp_sleep.h" @@ -16,8 +17,18 @@ class DeviceScreen { public: static DeviceScreen &create(void); - static DeviceScreen &create(const DisplayDriverConfig *cfg); - static DeviceScreen &create(DisplayDriverConfig &&cfg); + + /** + * @param spiLock the host's bus lock, for boards where the display or SD card shares + * an SPI bus with peripherals the host drives itself. See ISpiLock - note the + * reentrancy requirement. + * + * Taken by reference rather than pointer so that create(nullptr) stays unambiguous + * against the config-taking overloads below. + */ + static DeviceScreen &create(ISpiLock &spiLock); + static DeviceScreen &create(const DisplayDriverConfig *cfg, ISpiLock *spiLock = nullptr); + static DeviceScreen &create(DisplayDriverConfig &&cfg, ISpiLock *spiLock = nullptr); void init(IClientBase *client); void task_handler(void); diff --git a/include/graphics/driver/LGFXDriver.h b/include/graphics/driver/LGFXDriver.h index fb56e6ca..2255d2f9 100644 --- a/include/graphics/driver/LGFXDriver.h +++ b/include/graphics/driver/LGFXDriver.h @@ -6,6 +6,7 @@ #include "input/InputDriver.h" #include "lvgl_private.h" #include "util/ILog.h" +#include "util/ISpiLock.h" #include constexpr uint32_t defaultLongPressTime = 700; // ms until long press is detected (lvgl default is 400) @@ -103,8 +104,11 @@ template void LGFXDriver::task_handler(void) lv_indev_enable(DisplayDriver::touch, false); lv_indev_enable(InputDriver::instance()->getButton(), true); } - lgfx->sleep(); - lgfx->powerSaveOn(); + { + ISpiLock::Guard bus; + lgfx->sleep(); + lgfx->powerSaveOn(); + } powerSaving = true; } } @@ -129,9 +133,14 @@ template void LGFXDriver::task_handler(void) ILOG_INFO("leaving powersave"); powerSaving = false; DisplayDriver::view->triggerHeartbeat(); - lgfx->powerSaveOff(); - lgfx->wakeup(); - lgfx->setBrightness(lastBrightness); + { + // Scoped so the bus is not held across view->sleep() above, + // which suspends the whole SoC in light sleep. + ISpiLock::Guard bus; + lgfx->powerSaveOff(); + lgfx->wakeup(); + lgfx->setBrightness(lastBrightness); + } DisplayDriver::view->screenSaving(false); if (hasTouch() && hasButton()) { ILOG_DEBUG("enable touch, disable button input"); @@ -149,14 +158,20 @@ template void LGFXDriver::task_handler(void) else { if (!powerSaving) { DisplayDriver::view->blankScreen(true); - lgfx->sleep(); - lgfx->powerSaveOn(); + { + ISpiLock::Guard bus; + lgfx->sleep(); + lgfx->powerSaveOn(); + } powerSaving = true; } if (screenTimeout > lv_display_get_inactive_time(NULL)) { DisplayDriver::view->blankScreen(false); - lgfx->powerSaveOff(); - lgfx->wakeup(); + { + ISpiLock::Guard bus; + lgfx->powerSaveOff(); + lgfx->wakeup(); + } powerSaving = false; lv_disp_trig_activity(NULL); } @@ -178,8 +193,11 @@ template void LGFXDriver::display_flush(lv_display_t *disp, c { uint32_t w = lv_area_get_width(area); uint32_t h = lv_area_get_height(area); - lv_draw_sw_rgb565_swap(px_map, w * h); - lgfx->pushImage(area->x1, area->y1, w, h, (uint16_t *)px_map); + lv_draw_sw_rgb565_swap(px_map, w * h); // CPU only - deliberately outside the guard + { + ISpiLock::Guard bus; + lgfx->pushImage(area->x1, area->y1, w, h, (uint16_t *)px_map); + } lv_display_flush_ready(disp); } #else @@ -220,9 +238,14 @@ template void LGFXDriver::touchpad_read(lv_indev_t *indev_dri { uint16_t touchX = 0, touchY = 0; #ifdef CUSTOM_TOUCH_DRIVER - bool touched = lgfx->getTouchXY(&touchX, &touchY); + bool touched = lgfx->getTouchXY(&touchX, &touchY); // I2C, no bus guard needed #else - bool touched = lgfx->getTouch(&touchX, &touchY); + // XPT2046/STMPE610 sit on SPI, and on several boards on the panel's own host. + bool touched; + { + ISpiLock::Guard bus; + touched = lgfx->getTouch(&touchX, &touchY); + } #endif if (!touched) { data->state = LV_INDEV_STATE_REL; @@ -343,9 +366,12 @@ template void LGFXDriver::init_lgfx(void) { // Initialize LovyanGFX ILOG_DEBUG("LGFX init..."); - lgfx->init(); - lgfx->setBrightness(defaultBrightness); - lgfx->fillScreen(LGFX::color565(0x3D, 0xDA, 0x83)); + { + ISpiLock::Guard bus; + lgfx->init(); + lgfx->setBrightness(defaultBrightness); + lgfx->fillScreen(LGFX::color565(0x3D, 0xDA, 0x83)); + } if (hasTouch()) { #ifndef CUSTOM_TOUCH_DRIVER @@ -393,6 +419,15 @@ template bool LGFXDriver::calibrate(uint16_t parameters[8]) calibrating = true; std::uint16_t fg = TFT_BLUE; std::uint16_t bg = LGFX::color565(0x67, 0xEA, 0x94); + // calibrateTouch() blocks until the user has tapped every marker, and the bus + // stays held throughout. Deliberate: LovyanGFX runs its own draw/read loop with + // no hook to release in the middle, and dropping the guard would leave this the + // one unprotected SPI path now that the host no longer holds a coarse lock. It + // is also not a regression - the host used to hold that lock across this same + // call. Note the branch is only reached when a board ships no stored calibration + // parameters; every board in-tree supplies them, so in practice we take the + // non-blocking setTouchCalibrate() path above. + ISpiLock::Guard bus; lgfx->clearDisplay(); lgfx->fillScreen(LGFX::color565(0x67, 0xEA, 0x94)); lgfx->setTextSize(1); @@ -422,7 +457,11 @@ template void LGFXDriver::printConfig(void) if (lgfx->panel()) { auto p = lgfx->panel(); auto cfg = p->config(); - uint32_t id = p->readCommand(0x04, 0, 4); + uint32_t id; + { + ISpiLock::Guard bus; // readCommand is a real panel transaction + id = p->readCommand(0x04, 0, 4); + } ILOG_DEBUG("Panel id=0x%08x (%dx%d): rst:%d, busy:%d, offX:%d, offY:%d invert:%d, RGB:%d, rotation:%d, offR:%d, read:%d, " "readP:%d, readB:%d, dlen:%d, colordepth:%d", id, p->width(), p->height(), cfg.pin_rst, cfg.pin_busy, cfg.offset_x, cfg.offset_y, p->getInvert(), diff --git a/include/util/ISpiLock.h b/include/util/ISpiLock.h new file mode 100644 index 00000000..c1dfa7a1 --- /dev/null +++ b/include/util/ISpiLock.h @@ -0,0 +1,71 @@ +#pragma once + +/** + * @brief Host-provided mutual exclusion for an SPI bus shared with other peripherals. + * + * On boards where the display and/or SD card sit on the same SPI bus as something the + * host drives itself - a LoRa radio, say - the host owns the arbitration. Installing an + * implementation here lets the UI take that lock around its own bus traffic, so the host + * does not have to serialize whole UI cycles to stay safe. + * + * Hold time is what matters: the UI takes the lock only around actual transfers, never + * across rendering, so the bus stays free during the CPU-only majority of a redraw. + * + * @note Implementations MUST be reentrant. UI call sites nest (a tile draw takes the + * lock, and the SD read that feeds it takes it again), so a plain non-recursive mutex + * will self-deadlock. Wrap one if that is all the host has - see the firmware's + * tftSetup.cpp for a task-tracking example. + * + * When no implementation is installed the guards below are no-ops, which is correct for + * hosts that do not share the bus. + */ +class ISpiLock +{ + public: + virtual ~ISpiLock() = default; + virtual void lock(void) = 0; + virtual void unlock(void) = 0; + + /** + * Let a waiter in without giving up the enclosing critical section. + * + * Provided for an async-DMA flush: once the display driver can signal completion + * asynchronously, the UI can release the bus while the transfer runs. Nothing calls + * it yet, so the default - release and immediately re-take - is sufficient. + */ + virtual void yield(void) + { + unlock(); + lock(); + } + + /** + * Normally set through DeviceScreen::create(). Kept as a static holder because two + * classes of call site cannot reach an instance member: the LVGL flush and touch + * callbacks are static and have no `this`, and the SD layer (`sdCard`, `SDFs`) is + * file-scope globals with no route to a DeviceGUI. + */ + static void install(ISpiLock *lock); + static ISpiLock *installed(void); + + /// RAII: hold the bus for the enclosing scope. Safe when nothing is installed. + class Guard + { + public: + Guard(void) : held(ISpiLock::installed()) + { + if (held) + held->lock(); + } + ~Guard() + { + if (held) + held->unlock(); + } + Guard(const Guard &) = delete; + Guard &operator=(const Guard &) = delete; + + private: + ISpiLock *held; + }; +}; diff --git a/source/graphics/DeviceScreen.cpp b/source/graphics/DeviceScreen.cpp index d71c4a58..2dadf351 100644 --- a/source/graphics/DeviceScreen.cpp +++ b/source/graphics/DeviceScreen.cpp @@ -15,14 +15,23 @@ DeviceScreen &DeviceScreen::create(void) return *new DeviceScreen(nullptr); } -DeviceScreen &DeviceScreen::create(const DisplayDriverConfig *cfg) +DeviceScreen &DeviceScreen::create(ISpiLock &spiLock) +{ + // Installed before anything is constructed, so even panel init is guarded. + ISpiLock::install(&spiLock); + return *new DeviceScreen(nullptr); +} + +DeviceScreen &DeviceScreen::create(const DisplayDriverConfig *cfg, ISpiLock *spiLock) { ILOG_DEBUG("creating DeviceScreen %dx%d ...", cfg ? cfg->width() : 0, cfg ? cfg->height() : 0); + ISpiLock::install(spiLock); return *new DeviceScreen(cfg); } -DeviceScreen &DeviceScreen::create(DisplayDriverConfig &&cfg) +DeviceScreen &DeviceScreen::create(DisplayDriverConfig &&cfg, ISpiLock *spiLock) { + ISpiLock::install(spiLock); return *new DeviceScreen(std::move(cfg)); } diff --git a/source/graphics/TFT/TFTView_320x240.cpp b/source/graphics/TFT/TFTView_320x240.cpp index c0fff609..e572a936 100644 --- a/source/graphics/TFT/TFTView_320x240.cpp +++ b/source/graphics/TFT/TFTView_320x240.cpp @@ -21,6 +21,7 @@ #include "ui.h" #include "util/FileLoader.h" #include "util/ILog.h" +#include "util/ISpiLock.h" #include #include #include @@ -6160,26 +6161,36 @@ void TFTView_320x240::backup(uint32_t option) std::stringstream path; path << "/keys/" << std::hex << std::setw(8) << std::setfill('0') << ownNode << ".yml"; + + // The bus is held for the card access only - messageAlert() below is LVGL work. + bool written = false; + { + ISpiLock::Guard bus; #if defined(ARCH_PORTDUINO) || defined(HAS_SD_MMC) - SDFs.mkdir("/keys"); - File sd = SDFs.open(path.str().c_str(), FILE_WRITE); + SDFs.mkdir("/keys"); + File sd = SDFs.open(path.str().c_str(), FILE_WRITE); #else - SDFs.mkdir("/keys"); - FsFile sd = SDFs.open(path.str().c_str(), O_RDWR | O_CREAT); + SDFs.mkdir("/keys"); + FsFile sd = SDFs.open(path.str().c_str(), O_RDWR | O_CREAT); #endif - if (sd) { - sd.println("config:"); - sd.println(" security:"); - sd.print(" privateKey: base64:"); - sd.println(pskToBase64(privkey.bytes, privkey.size).c_str()); - sd.print(" publicKey: base64:"); - sd.println(pskToBase64(pubkey.bytes, pubkey.size).c_str()); + if (sd) { + sd.println("config:"); + sd.println(" security:"); + sd.print(" privateKey: base64:"); + sd.println(pskToBase64(privkey.bytes, privkey.size).c_str()); + sd.print(" publicKey: base64:"); + sd.println(pskToBase64(pubkey.bytes, pubkey.size).c_str()); + written = true; + } + sd.close(); + } + + if (written) { ILOG_INFO("backup pub/priv keys done."); } else { ILOG_ERROR("open file %s for backup failed", path.str().c_str()); messageAlert(_("Failed to write keys!"), true); } - sd.close(); #endif } @@ -6192,39 +6203,47 @@ void TFTView_320x240::restore(uint32_t option) std::stringstream path; path << "/keys/" << std::hex << std::setw(8) << std::setfill('0') << ownNode << ".yml"; + // Read the file out under the bus guard, then release it: sendConfig() goes to the + // radio - which needs this same bus from another task - and messageAlert() is LVGL. + bool opened = false; + String privKey, pubKey; + { + ISpiLock::Guard bus; #if defined(ARCH_PORTDUINO) || defined(HAS_SD_MMC) - File sd = SDFs.open(path.str().c_str(), FILE_READ); + File sd = SDFs.open(path.str().c_str(), FILE_READ); #else - FsFile sd = SDFs.open(path.str().c_str(), O_RDONLY); + FsFile sd = SDFs.open(path.str().c_str(), O_RDONLY); #endif - if (sd) { - // TODO: improve parsing file contents - sd.readStringUntil('\n'); // config: - sd.readStringUntil('\n'); // security: - String privKey = sd.readStringUntil('\n'); // privateKey: base64: - String pubKey = sd.readStringUntil('\n'); // publicKey: base64: - if (privKey.indexOf("privateKey:") > 0 && pubKey.indexOf("publicKey:") > 0) { - String b64priv = privKey.substring(privKey.lastIndexOf(":") + 1); - String b64pub = pubKey.substring(pubKey.lastIndexOf(":") + 1); - b64priv.trim(); - b64pub.trim(); - if (base64ToPsk(b64priv.c_str(), privkey.bytes, privkey.size) && - base64ToPsk(b64pub.c_str(), pubkey.bytes, pubkey.size) && - controller->sendConfig(meshtastic_Config_SecurityConfig{db.config.security})) { - ILOG_INFO("restore pub/priv keys sent to radio"); - } else { - ILOG_ERROR("decoding keys failed"); - messageAlert(_("Failed to restore keys!"), true); - } - } else { - ILOG_ERROR("file %s contents don't match backup", path.str().c_str()); - messageAlert(_("Failed to parse keys!"), true); + if (sd) { + opened = true; + // TODO: improve parsing file contents + sd.readStringUntil('\n'); // config: + sd.readStringUntil('\n'); // security: + privKey = sd.readStringUntil('\n'); // privateKey: base64: + pubKey = sd.readStringUntil('\n'); // publicKey: base64: } - } else { + sd.close(); + } + + if (!opened) { ILOG_ERROR("open file %s failed", path.str().c_str()); messageAlert(_("Failed to retrieve keys!"), true); + } else if (privKey.indexOf("privateKey:") > 0 && pubKey.indexOf("publicKey:") > 0) { + String b64priv = privKey.substring(privKey.lastIndexOf(":") + 1); + String b64pub = pubKey.substring(pubKey.lastIndexOf(":") + 1); + b64priv.trim(); + b64pub.trim(); + if (base64ToPsk(b64priv.c_str(), privkey.bytes, privkey.size) && base64ToPsk(b64pub.c_str(), pubkey.bytes, pubkey.size) && + controller->sendConfig(meshtastic_Config_SecurityConfig{db.config.security})) { + ILOG_INFO("restore pub/priv keys sent to radio"); + } else { + ILOG_ERROR("decoding keys failed"); + messageAlert(_("Failed to restore keys!"), true); + } + } else { + ILOG_ERROR("file %s contents don't match backup", path.str().c_str()); + messageAlert(_("Failed to parse keys!"), true); } - sd.close(); #endif } diff --git a/source/graphics/common/SdCard.cpp b/source/graphics/common/SdCard.cpp index fab14566..e0143247 100644 --- a/source/graphics/common/SdCard.cpp +++ b/source/graphics/common/SdCard.cpp @@ -1,5 +1,6 @@ #include "graphics/common/SdCard.h" #include "util/ILog.h" +#include "util/ISpiLock.h" #ifndef SD_SPI_FREQUENCY #define SD_SPI_FREQUENCY 50000000 @@ -67,6 +68,7 @@ SDCard::~SDCard(void) {} bool SDCard::init(void) { + ISpiLock::Guard bus; // #ifndef BOARD_HAS_1BIT_SDMMC // SDFs.setPins(SDMMC_CLK, SDMMC_CMD, SDMMC_D0, SDMMC_D1, SDMMC_D2, SDMMC_D3); // return SDFs.begin("/sdcard", false); @@ -78,6 +80,7 @@ bool SDCard::init(void) ISdCard::CardType SDCard::cardType(void) { + ISpiLock::Guard bus; switch (SDFs.cardType()) { case CARD_NONE: return CardType::eNone; @@ -96,11 +99,13 @@ ISdCard::CardType SDCard::cardType(void) ISdCard::FatType SDCard::fatType(void) { + ISpiLock::Guard bus; return SDFs.cardSize() > 4Ull * 1024Ull * 1024Ull * 1024Ull ? FatType::eFat32 : FatType::eFat16; } ISdCard::ErrorType SDCard::errorType(void) { + ISpiLock::Guard bus; switch (SDFs.cardType()) { case CARD_NONE: return ErrorType::eSlotEmpty; @@ -113,21 +118,25 @@ ISdCard::ErrorType SDCard::errorType(void) uint64_t SDCard::usedBytes(void) { + ISpiLock::Guard bus; return SDFs.usedBytes(); } uint64_t SDCard::freeBytes(void) { + ISpiLock::Guard bus; return SDFs.totalBytes() - SDFs.usedBytes(); } uint64_t SDCard::cardSize(void) { + ISpiLock::Guard bus; return SDFs.totalBytes(); } SDCard::~SDCard(void) { + ISpiLock::Guard bus; SDFs.end(); } #endif @@ -135,6 +144,7 @@ SDCard::~SDCard(void) #if defined(ARCH_PORTDUINO) || defined(HAS_SD_MMC) std::set SDCard::loadMapStyles(const char *folder) { + ISpiLock::Guard bus; std::set styles; File maps = SDFs.open(folder); if (maps) { @@ -169,6 +179,7 @@ std::set SDCard::loadMapStyles(const char *folder) std::string SDCard::getUrlProvider(const char *folder, const char *style) { + ISpiLock::Guard bus; String filename = String(folder) + "/" + String(style) + "/.url"; File file = SDFs.open(filename.c_str(), FILE_READ); if (file) { @@ -181,6 +192,7 @@ std::string SDCard::getUrlProvider(const char *folder, const char *style) #elif defined(HAS_SDCARD) bool SdFsCard::init(void) { + ISpiLock::Guard bus; // TODO: allow specification of SPI bus // TODO: use begin(SdioConfig(FIFO_SDIO)) for SDIO (T-HMI) // Note: this can also be done via #define BUILTIN_SDCARD SDCARD_CS using begin(SDCARD_CS) @@ -202,6 +214,7 @@ bool SdFsCard::init(void) ISdCard::CardType SdFsCard::cardType(void) { + ISpiLock::Guard bus; uint8_t card = SDFs.card()->type(); // 0 - SD V1, 1 - SD V2, or 3 - SDHC/SDXC uint8_t fsType = SDFs.fatType(); // FAT_TYPE_EXFAT, FAT_TYPE_FAT32, FAT_TYPE_FAT16, or zero for error if (card == 3) @@ -213,6 +226,7 @@ ISdCard::CardType SdFsCard::cardType(void) ISdCard::FatType SdFsCard::fatType(void) { + ISpiLock::Guard bus; uint8_t type = SDFs.fatType(); return type == FAT_TYPE_EXFAT ? FatType::eExFat : type == FAT_TYPE_FAT32 ? FatType::eFat32 @@ -227,6 +241,7 @@ ISdCard::FatType SdFsCard::fatType(void) */ ISdCard::ErrorType SdFsCard::errorType(void) { + ISpiLock::Guard bus; ILOG_ERROR("SD card error code: %d", SDFs.sdErrorCode()); if (SDFs.sdErrorCode() == SD_CARD_ERROR_CMD0) return ErrorType::eSlotEmpty; @@ -258,26 +273,35 @@ ISdCard::ErrorType SdFsCard::errorType(void) uint64_t SdFsCard::usedBytes(void) { - return cardSize() - freeBytes(); + ISpiLock::Guard bus; + // Computed here rather than as cardSize() - freeBytes(): those take the guard + // themselves, and nesting would lean on the host's lock being reentrant. Not + // nesting is cheaper than depending on every host getting that right. + uint64_t bytesPerCluster = uint64_t(SDFs.bytesPerCluster()); + return (uint64_t(SDFs.clusterCount()) - uint64_t(SDFs.freeClusterCount())) * bytesPerCluster; } uint64_t SdFsCard::freeBytes(void) { + ISpiLock::Guard bus; return uint64_t(SDFs.freeClusterCount()) * uint64_t(SDFs.bytesPerCluster()); } uint64_t SdFsCard::cardSize(void) { + ISpiLock::Guard bus; return uint64_t(SDFs.clusterCount()) * uint64_t(SDFs.bytesPerCluster()); } bool SdFsCard::format(void) { + ISpiLock::Guard bus; return SDFs.format(); } std::set SdFsCard::loadMapStyles(const char *folder) { + ISpiLock::Guard bus; std::set styles; File maps = SDFs.open(folder); if (maps) { @@ -314,6 +338,7 @@ std::set SdFsCard::loadMapStyles(const char *folder) std::string SdFsCard::getUrlProvider(const char *folder, const char *style) { + ISpiLock::Guard bus; String filename = String(folder) + "/" + String(style) + "/.url"; File file = SDFs.open(filename.c_str(), FILE_READ); if (file) { diff --git a/source/graphics/map/SDCardService.cpp b/source/graphics/map/SDCardService.cpp index 8a180fd0..a79301c9 100644 --- a/source/graphics/map/SDCardService.cpp +++ b/source/graphics/map/SDCardService.cpp @@ -1,4 +1,5 @@ #include "lvgl.h" +#include "util/ISpiLock.h" #include "graphics/map/MapTileSettings.h" #include "graphics/map/SDCardService.h" @@ -70,6 +71,7 @@ SDCardService::SDCardService() : ITileService(DRIVE_LETTER ":") SDCardService::~SDCardService() { + ISpiLock::Guard bus; #ifndef ARCH_PORTDUINO SD.end(); #endif @@ -168,6 +170,7 @@ bool SDCardService::save(const char *name, void *img, size_t len) void *SDCardService::fs_open(lv_fs_drv_t *drv, const char *path, lv_fs_mode_t mode) { + ISpiLock::Guard bus; String s(path); File file = SD.open(path, mode == LV_FS_MODE_RD ? FILE_READ : FILE_WRITE); if (!file) { @@ -182,6 +185,7 @@ void *SDCardService::fs_open(lv_fs_drv_t *drv, const char *path, lv_fs_mode_t mo lv_fs_res_t SDCardService::fs_close(lv_fs_drv_t *drv, void *file_p) { + ISpiLock::Guard bus; // ILOG_DEBUG("SD.close()"); SdFile *lf = static_cast(file_p); lf->file.close(); @@ -191,6 +195,7 @@ lv_fs_res_t SDCardService::fs_close(lv_fs_drv_t *drv, void *file_p) lv_fs_res_t SDCardService::fs_read(lv_fs_drv_t *drv, void *file_p, void *buf, uint32_t btr, uint32_t *br) { + ISpiLock::Guard bus; *br = static_cast(file_p)->file.read((uint8_t *)buf, btr); // ILOG_DEBUG("SD.read(): %d/%d bytes", *br, btr); return (*br <= 0) ? LV_FS_RES_UNKNOWN : LV_FS_RES_OK; @@ -198,6 +203,7 @@ lv_fs_res_t SDCardService::fs_read(lv_fs_drv_t *drv, void *file_p, void *buf, ui lv_fs_res_t SDCardService::fs_write(lv_fs_drv_t *drv, void *file_p, const void *buf, uint32_t btw, uint32_t *bw) { + ISpiLock::Guard bus; *bw = static_cast(file_p)->file.write((uint8_t *)buf, btw); // ILOG_DEBUG("SD.write(): %d/btw bytes", *bw, btw); return (*bw <= 0) ? LV_FS_RES_UNKNOWN : LV_FS_RES_OK; @@ -205,12 +211,14 @@ lv_fs_res_t SDCardService::fs_write(lv_fs_drv_t *drv, void *file_p, const void * lv_fs_res_t SDCardService::fs_seek(lv_fs_drv_t *drv, void *file_p, uint32_t pos, lv_fs_whence_t whence) { + ISpiLock::Guard bus; // ILOG_DEBUG("SD.seek(): pos %d", pos); return static_cast(file_p)->file.seek(pos, (SeekMode)whence) ? LV_FS_RES_OK : LV_FS_RES_UNKNOWN; } lv_fs_res_t SDCardService::fs_tell(lv_fs_drv_t *drv, void *file_p, uint32_t *pos_p) { + ISpiLock::Guard bus; *pos_p = static_cast(file_p)->file.position(); // ILOG_DEBUG("SD.tell(): pos %d", *pos_p); return (int32_t)(*pos_p) < 0 ? LV_FS_RES_UNKNOWN : LV_FS_RES_OK; diff --git a/source/graphics/map/SdFatService.cpp b/source/graphics/map/SdFatService.cpp index cf742c0a..3d51bca4 100644 --- a/source/graphics/map/SdFatService.cpp +++ b/source/graphics/map/SdFatService.cpp @@ -1,6 +1,7 @@ #if defined(HAS_SDCARD) && not defined(HAS_SD_MMC) && not defined(ARCH_PORTDUINO) #include "lvgl.h" +#include "util/ISpiLock.h" #include "graphics/common/SdCard.h" #include "graphics/map/MapTileSettings.h" @@ -38,6 +39,7 @@ SdFatService::SdFatService() : ITileService(DRIVE_LETTER ":") SdFatService::~SdFatService() { + ISpiLock::Guard bus; SDFs.end(); } @@ -135,6 +137,7 @@ bool SdFatService::save(const char *name, void *img, size_t len) void *SdFatService::fs_open(lv_fs_drv_t *drv, const char *path, lv_fs_mode_t mode) { + ISpiLock::Guard bus; String s(path); SdFile *lf = new SdFile; lf->file = SDFs.open(path, mode == LV_FS_MODE_RD ? O_RDONLY : O_WRONLY); // NOTE: O_RDWR @@ -149,6 +152,7 @@ void *SdFatService::fs_open(lv_fs_drv_t *drv, const char *path, lv_fs_mode_t mod lv_fs_res_t SdFatService::fs_close(lv_fs_drv_t *drv, void *file_p) { + ISpiLock::Guard bus; // ILOG_DEBUG("FsSD.close()"); SdFile *lf = static_cast(file_p); lf->file.close(); @@ -158,6 +162,7 @@ lv_fs_res_t SdFatService::fs_close(lv_fs_drv_t *drv, void *file_p) lv_fs_res_t SdFatService::fs_read(lv_fs_drv_t *drv, void *file_p, void *buf, uint32_t btr, uint32_t *br) { + ISpiLock::Guard bus; *br = static_cast(file_p)->file.read((uint8_t *)buf, btr); // ILOG_DEBUG("FsSD.read(): %d/%d bytes", *br, btr); return (*br <= 0) ? LV_FS_RES_UNKNOWN : LV_FS_RES_OK; @@ -165,6 +170,7 @@ lv_fs_res_t SdFatService::fs_read(lv_fs_drv_t *drv, void *file_p, void *buf, uin lv_fs_res_t SdFatService::fs_write(lv_fs_drv_t *drv, void *file_p, const void *buf, uint32_t btw, uint32_t *bw) { + ISpiLock::Guard bus; *bw = static_cast(file_p)->file.write((uint8_t *)buf, btw); // ILOG_DEBUG("FsSD.write(): %d/btw bytes", *bw, btw); return (*bw <= 0) ? LV_FS_RES_UNKNOWN : LV_FS_RES_OK; @@ -172,6 +178,7 @@ lv_fs_res_t SdFatService::fs_write(lv_fs_drv_t *drv, void *file_p, const void *b lv_fs_res_t SdFatService::fs_seek(lv_fs_drv_t *drv, void *file_p, uint32_t pos, lv_fs_whence_t whence) { + ISpiLock::Guard bus; // ILOG_DEBUG("FsSD.seek(): pos %d", pos); if (whence == LV_FS_SEEK_SET) { return static_cast(file_p)->file.seekSet(pos) ? LV_FS_RES_OK : LV_FS_RES_UNKNOWN; @@ -184,6 +191,7 @@ lv_fs_res_t SdFatService::fs_seek(lv_fs_drv_t *drv, void *file_p, uint32_t pos, lv_fs_res_t SdFatService::fs_tell(lv_fs_drv_t *drv, void *file_p, uint32_t *pos_p) { + ISpiLock::Guard bus; *pos_p = static_cast(file_p)->file.position(); // ILOG_DEBUG("FsSD.tell(): pos %d", *pos_p); return (int32_t)(*pos_p) < 0 ? LV_FS_RES_UNKNOWN : LV_FS_RES_OK; diff --git a/source/util/FileLoader.cpp b/source/util/FileLoader.cpp index 9e2f771d..9b0434cd 100644 --- a/source/util/FileLoader.cpp +++ b/source/util/FileLoader.cpp @@ -1,11 +1,13 @@ #include "util/FileLoader.h" #include "lvgl_private.h" #include "util/ILog.h" +#include "util/ISpiLock.h" fs::FS *FileLoader::_fs = nullptr; void FileLoader::init(fs::FS *fs) { + ISpiLock::Guard bus; _fs = fs; static lv_fs_drv_t drv; lv_fs_drv_init(&drv); @@ -27,6 +29,7 @@ void FileLoader::init(fs::FS *fs) void *FileLoader::fs_open(lv_fs_drv_t *drv, const char *path, lv_fs_mode_t mode) { + ISpiLock::Guard bus; File file = _fs->open(path, mode == LV_FS_MODE_RD ? FILE_READ : FILE_WRITE); if (!file) { return nullptr; @@ -38,6 +41,7 @@ void *FileLoader::fs_open(lv_fs_drv_t *drv, const char *path, lv_fs_mode_t mode) lv_fs_res_t FileLoader::fs_close(lv_fs_drv_t *drv, void *file_p) { + ISpiLock::Guard bus; if (file_p == nullptr) { return LV_FS_RES_INV_PARAM; } @@ -51,6 +55,7 @@ lv_fs_res_t FileLoader::fs_close(lv_fs_drv_t *drv, void *file_p) lv_fs_res_t FileLoader::fs_read(lv_fs_drv_t *drv, void *file_p, void *buf, uint32_t btr, uint32_t *br) { + ISpiLock::Guard bus; if (file_p == nullptr) { return LV_FS_RES_INV_PARAM; } @@ -60,6 +65,7 @@ lv_fs_res_t FileLoader::fs_read(lv_fs_drv_t *drv, void *file_p, void *buf, uint3 lv_fs_res_t FileLoader::fs_write(lv_fs_drv_t *drv, void *file_p, const void *buf, uint32_t btw, uint32_t *bw) { + ISpiLock::Guard bus; if (file_p == nullptr) { return LV_FS_RES_INV_PARAM; } @@ -69,6 +75,7 @@ lv_fs_res_t FileLoader::fs_write(lv_fs_drv_t *drv, void *file_p, const void *buf lv_fs_res_t FileLoader::fs_seek(lv_fs_drv_t *drv, void *file_p, uint32_t pos, lv_fs_whence_t whence) { + ISpiLock::Guard bus; if (file_p == nullptr) { return LV_FS_RES_INV_PARAM; } @@ -77,6 +84,7 @@ lv_fs_res_t FileLoader::fs_seek(lv_fs_drv_t *drv, void *file_p, uint32_t pos, lv lv_fs_res_t FileLoader::fs_size(lv_fs_drv_t *drv, void *file_p, uint32_t *size_p) { + ISpiLock::Guard bus; if (file_p == nullptr) { return LV_FS_RES_INV_PARAM; } @@ -86,6 +94,7 @@ lv_fs_res_t FileLoader::fs_size(lv_fs_drv_t *drv, void *file_p, uint32_t *size_p lv_fs_res_t FileLoader::fs_tell(lv_fs_drv_t *drv, void *file_p, uint32_t *pos_p) { + ISpiLock::Guard bus; if (file_p == nullptr) { return LV_FS_RES_INV_PARAM; } diff --git a/source/util/ISpiLock.cpp b/source/util/ISpiLock.cpp new file mode 100644 index 00000000..b279027f --- /dev/null +++ b/source/util/ISpiLock.cpp @@ -0,0 +1,26 @@ +#include "util/ISpiLock.h" +#include "util/ILog.h" + +namespace +{ +ISpiLock *hostLock = nullptr; +} + +void ISpiLock::install(ISpiLock *lock) +{ + // A create() overload that was given no lock must not clear one an earlier call + // installed: drivers and SD code constructed by that earlier call keep guarding + // against it, so silently dropping it here would un-serialize the bus. + if (!lock) + return; + + if (hostLock && hostLock != lock) + ILOG_WARN("ISpiLock: replacing the installed bus lock"); + + hostLock = lock; +} + +ISpiLock *ISpiLock::installed(void) +{ + return hostLock; +} diff --git a/source/util/LogRotate.cpp b/source/util/LogRotate.cpp index 42765745..e4203aa1 100644 --- a/source/util/LogRotate.cpp +++ b/source/util/LogRotate.cpp @@ -1,5 +1,6 @@ #include "util/LogRotate.h" #include "util/ILog.h" +#include "util/ISpiLock.h" #include #define FILE_PREFIX "log_" @@ -13,6 +14,7 @@ LogRotate::LogRotate(fs::FS &fs, const char *logDir, uint32_t maxLen, uint32_t m void LogRotate::init(void) { + ISpiLock::Guard bus; if (!_fs.exists(rootDirName)) { _fs.mkdir(rootDirName); ILOG_INFO("LogRotate: no log files found."); @@ -49,6 +51,7 @@ void LogRotate::init(void) bool LogRotate::readNext(ILogEntry &entry) { + ISpiLock::Guard bus; if (!rootDir) { rootDir = _fs.open(rootDirName); if (!rootDir) @@ -83,6 +86,7 @@ bool LogRotate::readNext(ILogEntry &entry) bool LogRotate::write(const ILogEntry &entry) { + ISpiLock::Guard bus; time_t start = millis(); if (currentSize + entry.size() >= c_maxFileSize || totalSize + entry.size() >= c_maxSize) { // log rotation @@ -114,6 +118,7 @@ bool LogRotate::write(const ILogEntry &entry) */ bool LogRotate::clear(void) { + ISpiLock::Guard bus; time_t start = millis(); File root = _fs.open(rootDirName);