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);