From 758defdbed8fe2963fdfb2affc5575d819e3d2be Mon Sep 17 00:00:00 2001 From: Yajat Narayan Date: Wed, 3 Jun 2026 19:49:47 -0500 Subject: [PATCH 1/2] fix(sx1262): make HAL re-creatable and tolerate shared SPI3 bus Adds sx1262_hal_destroy() and sx1262_deinit() so the driver can be torn down and rebuilt within a single boot. Previously, a second call to sx1262_hal_create() early-returned with the stale SPI device handle, mutex, and GPIO state from the first run, causing subsequent status reads to return corrupted values (e.g. 0x15 / chip_mode 1, an impossible value) even though the chip itself was healthy. Also fixes a pre-existing bus-ownership bug: kernel_init() initializes SPI3 first via spi_init() because ST7789 shares the bus, so the sx1262_hal_create() call to spi_bus_initialize() was returning ESP_ERR_INVALID_STATE and treating it as fatal. Both error-cleanup paths also called spi_bus_free(SPI3_HOST), which would have torn down the bus ST7789 was actively using. Changes: - sx1262_hal_create(): treat ESP_ERR_INVALID_STATE from spi_bus_initialize as success; drop spi_bus_free() from error paths. - sx1262_hal_destroy(): new function that removes the SX1262 SPI device, deletes the mutex, and clears is_initialized. SPI3 bus left intact. - sx1262_deinit(): new function that stops the IRQ task (if running), calls sx1262_hal_destroy(), and clears s_is_initialized. Safe no-op when nothing is initialized. - meshtastic_app_start(): calls sx1262_deinit() before sx1262_hal_create() so re-entry rebuilds the SPI device from scratch. Co-Authored-By: Claude Opus 4.7 --- .../LoRa/meshtastic/meshtastic_app.c | 2 ++ .../Drivers/sx1262/include/sx1262.h | 13 +++++++++ .../Drivers/sx1262/include/sx1262_hal.h | 13 +++++++++ .../components/Drivers/sx1262/sx1262.c | 7 +++++ .../components/Drivers/sx1262/sx1262_hal.c | 28 +++++++++++++++++-- 5 files changed, 61 insertions(+), 2 deletions(-) diff --git a/firmware_p4/components/Applications/LoRa/meshtastic/meshtastic_app.c b/firmware_p4/components/Applications/LoRa/meshtastic/meshtastic_app.c index 32dbc5c7..c2b2bc19 100644 --- a/firmware_p4/components/Applications/LoRa/meshtastic/meshtastic_app.c +++ b/firmware_p4/components/Applications/LoRa/meshtastic/meshtastic_app.c @@ -66,6 +66,8 @@ esp_err_t meshtastic_app_start(void) { mt_region_t region = mt_region_current(); uint32_t freq_hz = mt_region_freq_for_channel(region, MT_PRIMARY_CHANNEL, p_info->bw_hz); + sx1262_deinit(); + sx1262_config_t cfg = {0}; ret = sx1262_hal_create(&cfg.hal); if (ret != ESP_OK) { diff --git a/firmware_p4/components/Drivers/sx1262/include/sx1262.h b/firmware_p4/components/Drivers/sx1262/include/sx1262.h index ff535832..d8324659 100644 --- a/firmware_p4/components/Drivers/sx1262/include/sx1262.h +++ b/firmware_p4/components/Drivers/sx1262/include/sx1262.h @@ -35,6 +35,19 @@ extern "C" { */ esp_err_t sx1262_init(const sx1262_config_t *config); +/** + * @brief Tear down the SX1262 driver so the next sx1262_init() starts clean. + * + * Stops the IRQ task (if running), destroys the HAL (removes the SPI device + * and mutex; SPI3 bus is preserved for shared peripherals), and clears + * driver state. + * + * Safe to call when nothing is initialized — no-op. + * + * @return ESP_OK on success. + */ +esp_err_t sx1262_deinit(void); + /** * @brief Start the IRQ processing task. * diff --git a/firmware_p4/components/Drivers/sx1262/include/sx1262_hal.h b/firmware_p4/components/Drivers/sx1262/include/sx1262_hal.h index d76d8c5c..2887d325 100644 --- a/firmware_p4/components/Drivers/sx1262/include/sx1262_hal.h +++ b/firmware_p4/components/Drivers/sx1262/include/sx1262_hal.h @@ -132,6 +132,19 @@ typedef struct sx1262_hal { */ esp_err_t sx1262_hal_create(sx1262_hal_t *out_hal); +/** + * @brief Tear down the HAL so the next sx1262_hal_create() rebuilds it. + * + * Removes the SX1262 SPI device, deletes the SPI mutex, and clears the + * internal initialized flag. The SPI3 bus itself is left initialized + * because it is shared with the ST7789 display. + * + * Safe to call when the HAL is not initialized — returns ESP_OK as a no-op. + * + * @return ESP_OK. + */ +esp_err_t sx1262_hal_destroy(void); + #ifdef __cplusplus } #endif diff --git a/firmware_p4/components/Drivers/sx1262/sx1262.c b/firmware_p4/components/Drivers/sx1262/sx1262.c index 32789b6a..9e8db25b 100644 --- a/firmware_p4/components/Drivers/sx1262/sx1262.c +++ b/firmware_p4/components/Drivers/sx1262/sx1262.c @@ -402,6 +402,13 @@ void sx1262_stop(void) { ESP_LOGI(TAG, "Stopped"); } +esp_err_t sx1262_deinit(void) { + sx1262_stop(); + esp_err_t ret = sx1262_hal_destroy(); + s_is_initialized = false; + return ret; +} + esp_err_t sx1262_set_callbacks(const sx1262_callbacks_t *cbs) { if (cbs == NULL) { return ESP_ERR_INVALID_ARG; diff --git a/firmware_p4/components/Drivers/sx1262/sx1262_hal.c b/firmware_p4/components/Drivers/sx1262/sx1262_hal.c index afd73ef9..d541a43e 100644 --- a/firmware_p4/components/Drivers/sx1262/sx1262_hal.c +++ b/firmware_p4/components/Drivers/sx1262/sx1262_hal.c @@ -202,6 +202,10 @@ esp_err_t sx1262_hal_create(sx1262_hal_t *out_hal) { }; ret = spi_bus_initialize(SPI_HOST_ID, &bus_cfg, SPI_DMA_CH_AUTO); + if (ret == ESP_ERR_INVALID_STATE) { + /* Bus already initialized by another driver (e.g. ST7789 via kernel spi_init). */ + ret = ESP_OK; + } if (ret != ESP_OK) { ESP_LOGE(TAG, "SPI bus init failed: %s", esp_err_to_name(ret)); return ret; @@ -217,7 +221,6 @@ esp_err_t sx1262_hal_create(sx1262_hal_t *out_hal) { ret = spi_bus_add_device(SPI_HOST_ID, &dev_cfg, &s_ctx.spi); if (ret != ESP_OK) { ESP_LOGE(TAG, "SPI add device failed: %s", esp_err_to_name(ret)); - spi_bus_free(SPI_HOST_ID); return ret; } @@ -226,7 +229,7 @@ esp_err_t sx1262_hal_create(sx1262_hal_t *out_hal) { if (s_ctx.spi_mutex == NULL) { ESP_LOGE(TAG, "Failed to create SPI mutex"); spi_bus_remove_device(s_ctx.spi); - spi_bus_free(SPI_HOST_ID); + s_ctx.spi = NULL; return ESP_ERR_NO_MEM; } @@ -254,3 +257,24 @@ esp_err_t sx1262_hal_create(sx1262_hal_t *out_hal) { return ESP_OK; } + +esp_err_t sx1262_hal_destroy(void) { + if (!s_ctx.is_initialized) { + return ESP_OK; + } + + if (s_ctx.spi != NULL) { + spi_bus_remove_device(s_ctx.spi); + s_ctx.spi = NULL; + } + if (s_ctx.spi_mutex != NULL) { + vSemaphoreDelete(s_ctx.spi_mutex); + s_ctx.spi_mutex = NULL; + } + + /* SPI3 bus is shared with ST7789 display; do not free it here. */ + + s_ctx.is_initialized = false; + ESP_LOGI(TAG, "HAL destroyed (SPI3 bus left intact for shared peripherals)"); + return ESP_OK; +} From cb2e03072383ca4d610932a57bbefba1fa8390da Mon Sep 17 00:00:00 2001 From: Yajat Narayan Date: Wed, 3 Jun 2026 22:10:22 -0500 Subject: [PATCH 2/2] style(format): apply clang-format to c5_flasher and spi_bridge Pre-existing clang-format violations on dev that fail the CI format check (tools/format.sh --check runs over the whole tree). Reformatted with the CI-pinned clang-format 22.1.3; cosmetic only, no logic changes. Co-Authored-By: Claude Opus 4.8 --- .../components/Service/spi_bridge/spi_bridge.c | 9 +++------ .../components/Service/c5_flasher/c5_flasher.c | 16 ++++++++++------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/firmware_c5/components/Service/spi_bridge/spi_bridge.c b/firmware_c5/components/Service/spi_bridge/spi_bridge.c index 7a1f6ad5..80f47af1 100644 --- a/firmware_c5/components/Service/spi_bridge/spi_bridge.c +++ b/firmware_c5/components/Service/spi_bridge/spi_bridge.c @@ -226,7 +226,7 @@ static void bridge_task(void *pvParameters) { spi_status_t status = SPI_STATUS_OK; uint8_t resp_payload[SPI_MAX_PAYLOAD]; uint8_t resp_len = 0; - bool tx_ready = false; // set when the case already built a complete tx_buf frame + bool tx_ready = false; // set when the case already built a complete tx_buf frame size_t tx_size = SPI_FRAME_SIZE; // bytes the master will clock for the response uint16_t cmd = spi_header_cmd(header); @@ -295,11 +295,8 @@ static void bridge_task(void *pvParameters) { while ((w = stream_pop_into(recs, batch_len, cap)) > 0) batch_len += w; - spi_header_t stream_header = {.sync = SPI_SYNC_BYTE, - .type = SPI_TYPE_STREAM, - .category = 0, - .op = 0, - .length = 0}; + spi_header_t stream_header = { + .sync = SPI_SYNC_BYTE, .type = SPI_TYPE_STREAM, .category = 0, .op = 0, .length = 0}; memcpy(tx_buf, &stream_header, sizeof(stream_header)); tx_buf[sizeof(spi_header_t)] = (uint8_t)(batch_len & 0xFF); tx_buf[sizeof(spi_header_t) + 1] = (uint8_t)((batch_len >> 8) & 0xFF); diff --git a/firmware_p4/components/Service/c5_flasher/c5_flasher.c b/firmware_p4/components/Service/c5_flasher/c5_flasher.c index ad961087..64762bc4 100644 --- a/firmware_p4/components/Service/c5_flasher/c5_flasher.c +++ b/firmware_p4/components/Service/c5_flasher/c5_flasher.c @@ -26,10 +26,10 @@ static const char *TAG = "C5_FLASHER"; -#define FLASHER_UART UART_NUM_1 -#define FLASHER_INIT_BAUD 115200 -#define FLASHER_FAST_BAUD 921600 -#define FLASH_BLOCK_SIZE 1024 +#define FLASHER_UART UART_NUM_1 +#define FLASHER_INIT_BAUD 115200 +#define FLASHER_FAST_BAUD 921600 +#define FLASH_BLOCK_SIZE 1024 // C5 flash layout (matches firmware_c5 partition table / flash_args). #define C5_BOOTLOADER_OFFSET 0x2000 @@ -106,9 +106,13 @@ esp_err_t c5_flasher_update(const uint8_t *bin_data, uint32_t bin_size) { } else { #if C5_FIRMWARE_EMBEDDED const c5_image_t images[] = { - {"bootloader", C5_BOOTLOADER_OFFSET, c5_bootloader_start, + {"bootloader", + C5_BOOTLOADER_OFFSET, + c5_bootloader_start, (uint32_t)(c5_bootloader_end - c5_bootloader_start)}, - {"partition-table", C5_PARTITION_OFFSET, c5_partition_start, + {"partition-table", + C5_PARTITION_OFFSET, + c5_partition_start, (uint32_t)(c5_partition_end - c5_partition_start)}, {"app", C5_APP_OFFSET, c5_app_start, (uint32_t)(c5_app_end - c5_app_start)}, };