Merge branch 'fix/rcp_spi_crash_issue' into 'master'

fix(openthread): fix multiple SPI communication bugs that caused the host to...

Closes IDFCI-9931

See merge request espressif/esp-idf!47660
This commit is contained in:
Zhang Wen Xu
2026-04-29 10:57:04 +00:00
4 changed files with 80 additions and 32 deletions

View File

@@ -168,6 +168,7 @@ private:
esp_openthread_spi_host_config_t m_spi_config;
uint8_t m_tx_buffer[kSPIFrameSize];
uint8_t *m_rx_dma_buf; ///< DMA-aligned RX buffer; avoids cache-coherency issues with unaligned frame buffer
int m_event_fd;
volatile uint16_t m_pending_data_len;

View File

@@ -3,15 +3,6 @@
*
* SPDX-License-Identifier: Apache-2.0
*/
/* SPI Slave example, receiver (uses SPI Slave driver to communicate with sender)
This example code is in the Public Domain (or CC0 licensed, at your option.)
Unless required by applicable law or agreed to in writing, this
software is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
CONDITIONS OF ANY KIND, either express or implied.
*/
#include <openthread/platform/spi-slave.h>
#include "esp_attr.h"
@@ -42,6 +33,15 @@ typedef struct {
uint16_t input_buf_len;
} pending_transaction_t;
// DMA bounce buffer for RX — always sized to max(input, output) so MISO is
// driven for the full output even when NcpSpi passes a small input buffer.
#define SPI_SLAVE_RX_DMA_BUF_SIZE OPENTHREAD_CONFIG_NCP_SPI_BUFFER_SIZE
static DRAM_ATTR uint8_t *s_rx_dma_buf = NULL;
// Guards the BUSY path: only return OT_ERROR_BUSY when a transaction is truly
// queued in the driver, so post_trans_cb is guaranteed to fire and re-queue.
static volatile DRAM_ATTR bool s_transaction_in_flight = false;
static otPlatSpiSlaveTransactionProcessCallback s_process_callback = NULL;
static otPlatSpiSlaveTransactionCompleteCallback s_complete_callback = NULL;
@@ -59,12 +59,25 @@ static void IRAM_ATTR handle_spi_setup_done(spi_slave_transaction_t *trans)
static void IRAM_ATTR handle_spi_transaction_done(spi_slave_transaction_t *trans)
{
gpio_set_level(s_spi_config->intr_pin, 1);
s_transaction_in_flight = false;
pending_transaction_t *pending_transaction = (pending_transaction_t *)(trans->user);
trans->trans_len /= CHAR_BIT;
// Cap trans_len: HW reports actual clock count which may exceed slave buffer.
uint16_t max_buf_len = pending_transaction->output_buf_len > pending_transaction->input_buf_len
? pending_transaction->output_buf_len : pending_transaction->input_buf_len;
if (trans->trans_len > max_buf_len) {
trans->trans_len = max_buf_len;
}
// Copy RX bounce buffer back to the actual NcpSpi input buffer.
if (s_input_buf && s_rx_dma_buf && s_rx_dma_buf != s_input_buf) {
memcpy(s_input_buf, s_rx_dma_buf, pending_transaction->input_buf_len);
}
if (s_complete_callback &&
s_complete_callback(s_context, (void*)trans->tx_buffer, pending_transaction->output_buf_len,
trans->rx_buffer, pending_transaction->input_buf_len, trans->trans_len)) {
s_input_buf, pending_transaction->input_buf_len, trans->trans_len)) {
esp_openthread_task_queue_post(s_process_callback, s_context);
}
}
@@ -83,22 +96,23 @@ esp_err_t esp_openthread_host_rcp_spi_init(const esp_openthread_platform_config_
.pin_bit_mask = (1ULL << s_spi_config->intr_pin),
};
ESP_GOTO_ON_ERROR(gpio_config(&io_conf), err, OT_PLAT_LOG_TAG, "fail to configure SPI gpio");
// Deassert INT: GPIO latch resets to LOW after chip reset, which would
// cause a spurious NEGEDGE before the slave is ready.
gpio_set_level(s_spi_config->intr_pin, 1);
gpio_set_pull_mode(s_spi_config->bus_config.mosi_io_num, GPIO_PULLUP_ONLY);
gpio_set_pull_mode(s_spi_config->bus_config.sclk_io_num, GPIO_PULLUP_ONLY);
gpio_set_pull_mode(s_spi_config->slave_config.spics_io_num, GPIO_PULLUP_ONLY);
s_spi_transaction = heap_caps_malloc(sizeof(spi_slave_transaction_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
ESP_GOTO_ON_FALSE(s_spi_transaction != NULL, ESP_ERR_NO_MEM, err, OT_PLAT_LOG_TAG, "failed to allocate memory for SPI transaction on internal heap");
s_pending_transaction = heap_caps_malloc(sizeof(pending_transaction_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
if (s_spi_transaction == NULL || s_pending_transaction == NULL) {
ESP_LOGE(OT_PLAT_LOG_TAG, "failed to allocate memory for SPI transaction on internal heap");
ret = ESP_ERR_NO_MEM;
goto err;
}
ESP_GOTO_ON_FALSE(s_pending_transaction != NULL, ESP_ERR_NO_MEM, err, OT_PLAT_LOG_TAG, "failed to allocate memory for pending transaction on internal heap");
s_rx_dma_buf = heap_caps_malloc(SPI_SLAVE_RX_DMA_BUF_SIZE, MALLOC_CAP_DMA | MALLOC_CAP_INTERNAL);
ESP_GOTO_ON_FALSE(s_rx_dma_buf != NULL, ESP_ERR_NO_MEM, err, OT_PLAT_LOG_TAG, "failed to allocate memory for RX DMA buffer on internal heap");
s_spi_transaction->user = (void *)s_pending_transaction;
/* Initialize SPI slave interface */
s_spi_config->slave_config.post_setup_cb = handle_spi_setup_done;
s_spi_config->slave_config.post_trans_cb = handle_spi_transaction_done;
ESP_GOTO_ON_ERROR(spi_slave_initialize(s_spi_config->host_device, &s_spi_config->bus_config,
@@ -114,6 +128,8 @@ err:
s_spi_transaction = NULL;
heap_caps_free(s_pending_transaction);
s_pending_transaction = NULL;
heap_caps_free(s_rx_dma_buf);
s_rx_dma_buf = NULL;
return ret;
}
@@ -125,9 +141,11 @@ void esp_openthread_spi_slave_deinit(void)
heap_caps_free(s_spi_config);
heap_caps_free(s_spi_transaction);
heap_caps_free(s_pending_transaction);
heap_caps_free(s_rx_dma_buf);
s_spi_config = NULL;
s_spi_transaction = NULL;
s_pending_transaction = NULL;
s_rx_dma_buf = NULL;
return;
}
@@ -155,14 +173,21 @@ otError IRAM_ATTR otPlatSpiSlavePrepareTransaction(uint8_t *aOutputBuf, uint16_t
s_input_len = aInputBufLen;
}
trans_length = s_output_len > s_input_len ? s_output_len : s_input_len;
trans_length *= CHAR_BIT;
if ((gpio_get_level(s_spi_config->slave_config.spics_io_num) == 0)) {
// Use max(input, output) so MISO is driven for the full output frame;
// s_rx_dma_buf absorbs extra RX bytes to avoid overflowing the NcpSpi buffer.
uint16_t trans_data_len = (s_input_len > s_output_len) ? s_input_len : s_output_len;
trans_length = trans_data_len * CHAR_BIT;
// In task context, return BUSY only when a transaction is already in flight
// AND CS is asserted — ensures post_trans_cb will fire to re-queue.
// In ISR context (post_trans_cb) we always queue unconditionally.
if (xPortCanYield() && s_transaction_in_flight &&
(gpio_get_level(s_spi_config->slave_config.spics_io_num) == 0)) {
ESP_EARLY_LOGE(SPI_SLAVE_TAG, "SPI busy");
return OT_ERROR_BUSY;
}
s_spi_transaction->length = trans_length;
s_spi_transaction->rx_buffer = s_input_buf;
s_spi_transaction->rx_buffer = s_rx_dma_buf;
s_spi_transaction->tx_buffer = s_output_buf;
pending_transaction_t *pending_transaction = (pending_transaction_t *)s_spi_transaction->user;
@@ -179,6 +204,7 @@ otError IRAM_ATTR otPlatSpiSlavePrepareTransaction(uint8_t *aOutputBuf, uint16_t
}
if (trans_state == ESP_OK) {
s_transaction_in_flight = true;
return OT_ERROR_NONE;
} else {
return OT_ERROR_FAILED;

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -8,6 +8,7 @@
#include "openthread/error.h"
#include "esp_check.h"
#include "esp_heap_caps.h"
#include "esp_openthread_common_macro.h"
#include "esp_rom_sys.h"
#include "esp_vfs.h"
@@ -17,6 +18,7 @@
#include "driver/gpio.h"
#include "driver/spi_master.h"
#include "hal/gpio_types.h"
#include "soc/soc_caps.h"
#include "ncp/ncp_spi.hpp"
using ot::Spinel::SpiFrame;
@@ -26,7 +28,8 @@ namespace esp {
namespace openthread {
SpiSpinelInterface::SpiSpinelInterface(void)
: m_event_fd(-1)
: m_rx_dma_buf(nullptr)
, m_event_fd(-1)
, m_receiver_frame_callback(nullptr)
, m_receiver_frame_context(nullptr)
, m_receive_frame_buffer(nullptr)
@@ -73,7 +76,7 @@ esp_err_t SpiSpinelInterface::Enable(const esp_openthread_spi_host_config_t &spi
io_conf.mode = GPIO_MODE_INPUT;
io_conf.pull_up_en = GPIO_PULLUP_ENABLE;
ESP_RETURN_ON_ERROR(gpio_config(&io_conf), OT_PLAT_LOG_TAG, "fail to config spi gpio");
gpio_install_isr_service(0); // The gpio isr service may has been installed.
gpio_install_isr_service(0);
ESP_RETURN_ON_ERROR(gpio_isr_handler_add(spi_config.intr_pin, GpioIntrHandler, this), OT_PLAT_LOG_TAG,
"fail to add gpio isr handler");
m_has_pending_device_frame = false;
@@ -82,6 +85,9 @@ esp_err_t SpiSpinelInterface::Enable(const esp_openthread_spi_host_config_t &spi
ESP_RETURN_ON_FALSE(m_event_fd >= 0, ESP_FAIL, OT_PLAT_LOG_TAG, "fail to get event fd");
m_rx_dma_buf = (uint8_t *)heap_caps_malloc(kSPIFrameSize, MALLOC_CAP_DMA | MALLOC_CAP_INTERNAL);
ESP_RETURN_ON_FALSE(m_rx_dma_buf != nullptr, ESP_ERR_NO_MEM, OT_PLAT_LOG_TAG, "fail to alloc SPI RX DMA buffer");
ESP_LOGI(OT_PLAT_LOG_TAG, "spinel SPI interface initialization completed");
return ESP_OK;
@@ -105,6 +111,8 @@ esp_err_t SpiSpinelInterface::Disable(void)
ESP_RETURN_ON_ERROR(spi_bus_free(m_spi_config.host_device), OT_PLAT_LOG_TAG, "fail to free spi bus");
gpio_uninstall_isr_service();
}
heap_caps_free(m_rx_dma_buf);
m_rx_dma_buf = nullptr;
return ESP_OK;
}
@@ -115,8 +123,7 @@ otError SpiSpinelInterface::SendFrame(const uint8_t *frame, uint16_t length)
"send frame is too long");
memcpy(&m_tx_buffer[kSPIFrameHeaderSize], frame, length);
uint16_t rx_data_size =
length < kSmallPacketSize ? kSmallPacketSize : length; // We'll use tx_size to receive small packets piggybacked
uint16_t rx_data_size = length < kSmallPacketSize ? kSmallPacketSize : length;
if (ConductSPITransaction(false, length, rx_data_size) == ESP_OK) {
return OT_ERROR_NONE;
} else {
@@ -152,9 +159,12 @@ esp_err_t SpiSpinelInterface::ConductSPITransaction(bool reset, uint16_t tx_data
transaction.length = data_size * CHAR_BIT;
transaction.rxlength = (rx_data_size + kSPIFrameHeaderSize) * CHAR_BIT;
transaction.tx_buffer = m_tx_buffer;
transaction.rx_buffer = rx_buffer;
// Use aligned DMA buffer: MultiFrameBuffer's internal array is not
// cache-line-aligned, causing stale D-cache reads after DMA completes.
transaction.rx_buffer = m_rx_dma_buf;
ESP_RETURN_ON_ERROR(spi_device_polling_transmit(m_device, &transaction), OT_PLAT_LOG_TAG, "SPI transaction failed");
memcpy(rx_buffer, m_rx_dma_buf, data_size);
SpiFrame rx_frame(rx_buffer);
if (!rx_frame.IsValid() || rx_frame.GetHeaderAcceptLen() > kSPIFrameSize ||
@@ -162,17 +172,14 @@ esp_err_t SpiSpinelInterface::ConductSPITransaction(bool reset, uint16_t tx_data
vTaskDelay(pdMS_TO_TICKS(15));
ESP_RETURN_ON_ERROR(spi_device_polling_transmit(m_device, &transaction), OT_PLAT_LOG_TAG,
"fail to retry SPI invalid transaction");
memcpy(rx_buffer, m_rx_dma_buf, data_size);
}
if (rx_frame.IsResetFlagSet()) {
ESP_LOGW(OT_PLAT_LOG_TAG, "RCP Reset");
m_receive_frame_buffer->DiscardFrame();
return ESP_OK;
}
if (rx_frame.GetHeaderDataLen() == 0 && rx_frame.GetHeaderAcceptLen() == 0) {
vTaskDelay(pdMS_TO_TICKS(15));
ESP_RETURN_ON_ERROR(spi_device_polling_transmit(m_device, &transaction), OT_PLAT_LOG_TAG,
"fail to retry SPI empty transaction");
memcpy(rx_buffer, m_rx_dma_buf, data_size);
}
if (rx_frame.GetHeaderDataLen() > 0 && rx_frame.GetHeaderDataLen() < tx_frame.GetHeaderAcceptLen()) {
@@ -267,7 +274,20 @@ otError SpiSpinelInterface::HardwareReset(void)
{
if (mRcpFailureHandler) {
mRcpFailureHandler();
ConductSPITransaction(true, 0, 0); // clear
ConductSPITransaction(true, 0, 0);
// Drain stale GPIO interrupt events accumulated before reset, otherwise
// WaitForFrame() would fire immediately on a stale event and fail.
uint64_t event;
struct timeval zero_timeout = {0, 0};
fd_set read_fds;
FD_ZERO(&read_fds);
FD_SET(m_event_fd, &read_fds);
if (select(m_event_fd + 1, &read_fds, NULL, NULL, &zero_timeout) > 0 &&
FD_ISSET(m_event_fd, &read_fds)) {
read(m_event_fd, &event, sizeof(event));
}
m_pending_data_len = 0;
}
return OT_ERROR_NONE;
}

View File

@@ -97,6 +97,7 @@ void app_main(void)
ESP_ERROR_CHECK(mdns_hostname_set("esp-ot-br"));
#if CONFIG_OPENTHREAD_SUPPORT_HW_RESET_RCP
esp_openthread_register_rcp_failure_handler(rcp_failure_hardware_reset_handler);
esp_openthread_set_coprocessor_reset_failure_callback(rcp_failure_hardware_reset_handler);
#endif
#if CONFIG_OPENTHREAD_CLI