From 0c699642b05cc7289131db280a6f2c6faa6ab9a4 Mon Sep 17 00:00:00 2001 From: Eric Wang Date: Tue, 28 Jul 2026 13:31:41 -0700 Subject: [PATCH] shims: report SPI init failures instead of ignoring them Problem: wlan_hal_spi_init() logged errors without their code and then continued regardless. A failed spi_bus_initialize() was still followed by spi_bus_add_device() on an unusable bus; a failed add_device left spi_handle indeterminate yet still passed it to spi_device_get_actual_freq(). wlan_hal_spi_deinit() reported a spi_bus_free() error as "spi_bus_initialize failed". Reproduce: 1. Claim SPI2_HOST before mmhalow_init() (or let mmwlan_boot() retry after a failure -- the failing path never runs wlan_hal_spi_deinit(), so the host stays initialized). 2. Boot. Observed: "spi_bus_initialize failed" with no error code, then a causeless "Transport init failed" from mmdrv_init(). Expected: the actual esp_err_t, and no work done on a dead bus. Fix: return esp_err_t and stop at the first error, logging esp_err_to_name(). ESP_ERR_INVALID_STATE is benign (host already up -- the retry case) so it reuses the bus, but now drops any device the previous attempt added instead of leaking its handle and stacking a second device. spi_handle is cleared on every failure path so it is never used indeterminate. mmhal_wlan_init() logs the failure, since the HAL signature is void and cannot report upward. Verified: no behaviour change on the success path; the retry path no longer accumulates devices. wlan_hal_spi_init() is static, so this is not an API change. Note: this makes the failure attributable. It does not change how often it occurs. Signed-off-by: Eric Wang --- halow/components/shims/mmhal_wlan.c | 58 ++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/halow/components/shims/mmhal_wlan.c b/halow/components/shims/mmhal_wlan.c index a25783c..91d49a4 100644 --- a/halow/components/shims/mmhal_wlan.c +++ b/halow/components/shims/mmhal_wlan.c @@ -73,7 +73,13 @@ static void wlan_hal_gpio_init(void) gpio_config(&io_conf); } -static void wlan_hal_spi_init(void) +/** + * Bring up the SPI bus and add the WLAN transceiver device. + * + * @returns ESP_OK on success, otherwise the first error encountered. On failure the transceiver is + * unreachable, so the caller must not treat it as recoverable. + */ +static esp_err_t wlan_hal_spi_init(void) { esp_err_t ret; @@ -89,9 +95,24 @@ static void wlan_hal_spi_init(void) .flags = SPICOMMON_BUSFLAG_MASTER, }; ret = spi_bus_initialize(SPI2_HOST, &buscfg, SPI_DMA_CH_AUTO); - if (ret != ESP_OK) + if (ret == ESP_ERR_INVALID_STATE) { - ESP_LOGE(TAG, "spi_bus_initialize failed\n"); + /* The host is already initialized. This is reachable because mmwlan_boot() may be retried + * after a failed attempt and the failing path does not run wlan_hal_spi_deinit(), so reuse + * the bus rather than failing. Drop any device the previous attempt added: otherwise its + * handle is leaked and a second device is stacked on the same bus. */ + ESP_LOGW(TAG, "SPI host already initialized, reusing it"); + if (spi_handle != NULL) + { + (void)spi_bus_remove_device(spi_handle); + spi_handle = NULL; + } + } + else if (ret != ESP_OK) + { + ESP_LOGE(TAG, "spi_bus_initialize failed: %s", esp_err_to_name(ret)); + spi_handle = NULL; + return ret; } /* Selected the highest available SPI clock speed that is still below the MM6108's maximum of @@ -105,7 +126,11 @@ static void wlan_hal_spi_init(void) ret = spi_bus_add_device(SPI2_HOST, &dev_cfg, &spi_handle); if (ret != ESP_OK) { - ESP_LOGE(TAG, "spi_bus_add_device failed\n"); + /* A failed add_device leaves spi_handle indeterminate, so clear it and return rather than + * passing it to spi_device_get_actual_freq() below. */ + ESP_LOGE(TAG, "spi_bus_add_device failed: %s", esp_err_to_name(ret)); + spi_handle = NULL; + return ret; } /* The actual clock frequency may not be the one that was set as it is re-calculated by the @@ -114,20 +139,29 @@ static void wlan_hal_spi_init(void) int actual_freq_khz = 0; spi_device_get_actual_freq(spi_handle, &actual_freq_khz); ESP_LOGD(TAG, "Actual SPI CLK %dkHz\n", actual_freq_khz); + + return ESP_OK; } static void wlan_hal_spi_deinit(void) { - esp_err_t ret = spi_bus_remove_device(spi_handle); - if (ret != ESP_OK) + esp_err_t ret; + + /* spi_handle is NULL when wlan_hal_spi_init() failed before adding the device. */ + if (spi_handle != NULL) { - ESP_LOGE(TAG, "spi_bus_remove_device failed\n"); + ret = spi_bus_remove_device(spi_handle); + if (ret != ESP_OK) + { + ESP_LOGE(TAG, "spi_bus_remove_device failed: %s", esp_err_to_name(ret)); + } + spi_handle = NULL; } ret = spi_bus_free(SPI2_HOST); if (ret != ESP_OK) { - ESP_LOGE(TAG, "spi_bus_initialize failed\n"); + ESP_LOGE(TAG, "spi_bus_free failed: %s", esp_err_to_name(ret)); } } @@ -245,7 +279,13 @@ void mmhal_wlan_set_spi_irq_enabled(bool enabled) void mmhal_wlan_init(void) { wlan_hal_gpio_init(); - wlan_hal_spi_init(); + esp_err_t ret = wlan_hal_spi_init(); + if (ret != ESP_OK) + { + /* This HAL entry point cannot report failure upward, so record it here. Without this the + * only symptom is a later, causeless "Transport init failed" from mmdrv_init(). */ + ESP_LOGE(TAG, "SPI init failed (%s), transceiver will be unreachable", esp_err_to_name(ret)); + } /* Raise the RESET_N line to enable the WLAN transceiver. */ gpio_set_level(CONFIG_MM_RESET_N, 1); }