From 78d49e8747b3f5e8434e57f1116f93602db42965 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Tue, 30 Jun 2026 10:21:34 +0200 Subject: [PATCH 1/2] fix(ulp/lp_spi): fix driver bugs and add bus deinit API Co-authored-by: Cursor --- .../hal/esp32p4/include/hal/lp_spi_ll.h | 70 ++++ .../hal/esp32s31/include/hal/lp_spi_ll.h | 66 +++ components/ulp/lp_core/include/lp_core_spi.h | 18 +- .../lp_core/lp_core/include/ulp_lp_core_spi.h | 56 ++- components/ulp/lp_core/lp_core/lp_core_spi.c | 383 ++++++++++-------- components/ulp/lp_core/lp_core_spi.c | 31 +- .../main/lp_core/test_main_spi_slave.c | 41 +- 7 files changed, 491 insertions(+), 174 deletions(-) diff --git a/components/hal/esp32p4/include/hal/lp_spi_ll.h b/components/hal/esp32p4/include/hal/lp_spi_ll.h index aff04da0ad0..685dcecfad7 100644 --- a/components/hal/esp32p4/include/hal/lp_spi_ll.h +++ b/components/hal/esp32p4/include/hal/lp_spi_ll.h @@ -13,8 +13,10 @@ #include #include +#include #include "soc/lp_spi_struct.h" #include "soc/lpperi_struct.h" +#include "hal/assert.h" #ifdef __cplusplus extern "C" { @@ -51,6 +53,60 @@ static inline uint32_t lp_spi_ll_read_buffer_word(lp_spi_ll_dev_t *hw, int n) return hw->data_buf[n].reg_buf; } +/** + * @brief Write ``len`` bytes into the LP SPI data buffer registers from W0. + * Sub-word safe (no read past ``src``). + * + * @param hw LP SPI hardware + * @param src Source byte buffer + * @param len Number of bytes (<= ``LP_SPI_LL_MAX_BUFFER_SIZE``) + */ +static inline void lp_spi_ll_write_buffer_bytes(lp_spi_ll_dev_t *hw, const uint8_t *src, size_t len) +{ + HAL_ASSERT(len <= LP_SPI_LL_MAX_BUFFER_SIZE); + size_t reg_idx = 0; + size_t remaining = len; + while (remaining >= 4) { + uint32_t word; + memcpy(&word, src, 4); + hw->data_buf[reg_idx].reg_buf = word; + reg_idx++; + src += 4; + remaining -= 4; + } + if (remaining > 0) { + uint32_t word = 0; + memcpy(&word, src, remaining); + hw->data_buf[reg_idx].reg_buf = word; + } +} + +/** + * @brief Read ``len`` bytes from the LP SPI data buffer registers into ``dst``, + * starting at W0. Sub-word safe (no write past ``dst``). + * + * @param hw LP SPI hardware + * @param dst Destination byte buffer + * @param len Number of bytes (<= ``LP_SPI_LL_MAX_BUFFER_SIZE``) + */ +static inline void lp_spi_ll_read_buffer_bytes(lp_spi_ll_dev_t *hw, uint8_t *dst, size_t len) +{ + HAL_ASSERT(len <= LP_SPI_LL_MAX_BUFFER_SIZE); + size_t reg_idx = 0; + size_t remaining = len; + while (remaining >= 4) { + uint32_t word = hw->data_buf[reg_idx].reg_buf; + memcpy(dst, &word, 4); + reg_idx++; + dst += 4; + remaining -= 4; + } + if (remaining > 0) { + uint32_t word = hw->data_buf[reg_idx].reg_buf; + memcpy(dst, &word, remaining); + } +} + /** * @brief Reset RX and TX AFIFOs */ @@ -370,6 +426,20 @@ static inline void lp_spi_ll_reset_cs_timing(lp_spi_ll_dev_t *hw) hw->spi_user1.reg_cs_hold_time = 0; } +/** + * @brief This resets the LP SPI peripheral + */ +static inline void lp_spi_ll_reset(void) +{ + lpperi_dev_t *lp_peri_dev = &LPPERI; + lp_peri_dev->reset_en.rst_en_lp_spi = 1; + /* Read-back fence: ensure the reset assertion propagates through the + * bus before de-asserting. + */ + (void)lp_peri_dev->reset_en.rst_en_lp_spi; + lp_peri_dev->reset_en.rst_en_lp_spi = 0; +} + #ifdef __cplusplus } #endif diff --git a/components/hal/esp32s31/include/hal/lp_spi_ll.h b/components/hal/esp32s31/include/hal/lp_spi_ll.h index 5ad0eadfa63..1c19b7464dd 100644 --- a/components/hal/esp32s31/include/hal/lp_spi_ll.h +++ b/components/hal/esp32s31/include/hal/lp_spi_ll.h @@ -13,8 +13,10 @@ #include #include +#include #include "soc/lp_spi_struct.h" #include "soc/lp_peri_clkrst_struct.h" +#include "hal/assert.h" #ifdef __cplusplus extern "C" { @@ -51,6 +53,60 @@ static inline uint32_t lp_spi_ll_read_buffer_word(lp_spi_ll_dev_t *hw, int n) return hw->data_buf[n].val; } +/** + * @brief Write ``len`` bytes into the LP SPI data buffer registers from W0. + * Sub-word safe (no read past ``src``). + * + * @param hw LP SPI hardware + * @param src Source byte buffer + * @param len Number of bytes (<= ``LP_SPI_LL_MAX_BUFFER_SIZE``) + */ +static inline void lp_spi_ll_write_buffer_bytes(lp_spi_ll_dev_t *hw, const uint8_t *src, size_t len) +{ + HAL_ASSERT(len <= LP_SPI_LL_MAX_BUFFER_SIZE); + size_t reg_idx = 0; + size_t remaining = len; + while (remaining >= 4) { + uint32_t word; + memcpy(&word, src, 4); + hw->data_buf[reg_idx].val = word; + reg_idx++; + src += 4; + remaining -= 4; + } + if (remaining > 0) { + uint32_t word = 0; + memcpy(&word, src, remaining); + hw->data_buf[reg_idx].val = word; + } +} + +/** + * @brief Read ``len`` bytes from the LP SPI data buffer registers into ``dst``, + * starting at W0. Sub-word safe (no write past ``dst``). + * + * @param hw LP SPI hardware + * @param dst Destination byte buffer + * @param len Number of bytes (<= ``LP_SPI_LL_MAX_BUFFER_SIZE``) + */ +static inline void lp_spi_ll_read_buffer_bytes(lp_spi_ll_dev_t *hw, uint8_t *dst, size_t len) +{ + HAL_ASSERT(len <= LP_SPI_LL_MAX_BUFFER_SIZE); + size_t reg_idx = 0; + size_t remaining = len; + while (remaining >= 4) { + uint32_t word = hw->data_buf[reg_idx].val; + memcpy(dst, &word, 4); + reg_idx++; + dst += 4; + remaining -= 4; + } + if (remaining > 0) { + uint32_t word = hw->data_buf[reg_idx].val; + memcpy(dst, &word, remaining); + } +} + /** * @brief Reset RX and TX AFIFOs */ @@ -370,6 +426,16 @@ static inline void lp_spi_ll_reset_cs_timing(lp_spi_ll_dev_t *hw) hw->spi_user1.reg_cs_hold_time = 0; } +/** + * @brief This resets the LP SPI peripheral + */ +static inline void lp_spi_ll_reset(void) +{ + LP_PERI_CLKRST.spi_ctrl.lp_spi_rst_en = 1; + (void)LP_PERI_CLKRST.spi_ctrl.lp_spi_rst_en; + LP_PERI_CLKRST.spi_ctrl.lp_spi_rst_en = 0; +} + #ifdef __cplusplus } #endif diff --git a/components/ulp/lp_core/include/lp_core_spi.h b/components/ulp/lp_core/include/lp_core_spi.h index f2f0ea9f036..a9d836d4bf5 100644 --- a/components/ulp/lp_core/include/lp_core_spi.h +++ b/components/ulp/lp_core/include/lp_core_spi.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -104,6 +104,22 @@ esp_err_t lp_core_lp_spi_bus_add_device(lp_spi_host_t host_id, const lp_spi_devi */ esp_err_t lp_core_lp_spi_slave_initialize(lp_spi_host_t host_id, const lp_spi_slave_config_t *slave_config); +/** + * @brief Deinitialize the LP SPI bus. + * + * Performs a module-level hardware reset of the LP SPI peripheral (all + * registers return to power-on defaults) and deinitializes the LP GPIO + * pins that were configured for SPI signals. + * + * @param host_id LP SPI host ID (currently unused, only one host exists) + * @param bus_config Pointer to the bus configuration that was used during + * initialization, so that the same GPIO pins can be + * deinitialized. May be NULL to skip GPIO deinit. + * + * @return ESP_OK on success + */ +esp_err_t lp_core_lp_spi_bus_deinit(lp_spi_host_t host_id, const lp_spi_bus_config_t *bus_config); + #ifdef __cplusplus } #endif diff --git a/components/ulp/lp_core/lp_core/include/ulp_lp_core_spi.h b/components/ulp/lp_core/lp_core/include/ulp_lp_core_spi.h index 6e4515cdc2d..ce6f28ef2a5 100644 --- a/components/ulp/lp_core/lp_core/include/ulp_lp_core_spi.h +++ b/components/ulp/lp_core/lp_core/include/ulp_lp_core_spi.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -21,10 +21,10 @@ typedef uint32_t lp_spi_bus_t; * This structure describes one SPI transaction. The descriptor should not be modified until the transaction finishes. */ typedef struct { - uint32_t tx_length; /*!< Total data length to transmit in bytes */ - uint32_t rx_length; /*!< Total data length to receive in bytes */ - const void *tx_buffer; /*!< Pointer to the transmit buffer. Must be set for master mode transactions. Can be NULL for slave mode transactions. */ - void *rx_buffer; /*!< Pointer to the receive buffer. Must be set for slave mode transactions. Can be NULL for master mode transactions. */ + uint32_t tx_length; /*!< Number of bytes to transmit. Must be 0 when ``tx_buffer`` is NULL. */ + uint32_t rx_length; /*!< Number of bytes to receive. Must be 0 when ``rx_buffer`` is NULL. */ + const void *tx_buffer; /*!< Pointer to the transmit buffer, or NULL. */ + void *rx_buffer; /*!< Pointer to the receive buffer, or NULL. */ lp_spi_bus_t bus; /*!< The LP SPI bus to transmit the data on */ // The following are only used in master mode transactions int command; /*!< Command data, of which the length is set in the ``command_bits`` field of this structure. */ @@ -35,31 +35,69 @@ typedef struct { } lp_spi_transaction_t; /** - * @brief Initiate an LP SPI transaction in master mode to transmit device to an SPI device and optionally receive data - * from the device. + * @brief Initiate an LP SPI master transaction. * * @param trans_desc LP SPI transaction configuration descriptor * @param cycles_to_wait Operation timeout in CPU cycles. Set to -1 to wait forever. * * @return esp_err_t ESP_OK when successful * ESP_ERR_INVALID_ARG if the configuration is invalid + * ESP_ERR_INVALID_STATE if a previous transaction is still in progress * ESP_ERR_TIMEOUT when the operation times out */ esp_err_t lp_core_lp_spi_master_transfer(lp_spi_transaction_t *trans_desc, int32_t cycles_to_wait); /** - * @brief Initiate an LP SPI transaction in slave mode to receive data from an SPI master and optionally transmit data - * back to the master. + * @brief Initiate an LP SPI slave transaction. * * @param trans_desc LP SPI transaction configuration descriptor * @param cycles_to_wait Operation timeout in CPU cycles. Set to -1 to wait forever. * * @return esp_err_t ESP_OK when successful * ESP_ERR_INVALID_ARG if the configuration is invalid + * ESP_ERR_INVALID_STATE if a previous transaction is still in progress * ESP_ERR_TIMEOUT when the operation times out */ esp_err_t lp_core_lp_spi_slave_transfer(lp_spi_transaction_t *trans_desc, int32_t cycles_to_wait); +/** + * @brief Preload the LP SPI slave's TX data and arm the peripheral, then return. + * + * Loads `trans_desc->tx_buffer` into the LP-SPI W0..W15 data buffer, + * programs the bit length, and starts the slave user phase. The call + * does not block on the master's SCK; the peripheral is left armed and + * will sample/drive the bus as soon as the master starts clocking. + * + * Pair with `lp_core_lp_spi_slave_wait()` to block on completion and + * drain the RX buffer. Calling `lp_core_lp_spi_slave_arm()` again while + * a previous arm has not been waited on returns `ESP_ERR_INVALID_STATE`. + * + * This is the LP-SPI counterpart of `spi_slave_queue_trans()`. + * + * @param trans_desc LP SPI transaction configuration descriptor. + * + * @return esp_err_t ESP_OK when successful + * ESP_ERR_INVALID_ARG if the configuration is invalid + * ESP_ERR_INVALID_STATE if a previous transaction is still in progress + */ +esp_err_t lp_core_lp_spi_slave_arm(lp_spi_transaction_t *trans_desc); + +/** + * @brief Wait for a previously-armed LP SPI slave transaction to complete. + * + * Must be paired with `lp_core_lp_spi_slave_arm()` using the same `trans_desc`. + * + * This is the LP-SPI counterpart of `spi_slave_get_trans_result()`. + * + * @param trans_desc LP SPI transaction configuration descriptor. + * @param cycles_to_wait Operation timeout in CPU cycles. Set to -1 to wait forever. + * + * @return esp_err_t ESP_OK when successful + * ESP_ERR_INVALID_ARG if the configuration is invalid + * ESP_ERR_TIMEOUT when the operation times out + */ +esp_err_t lp_core_lp_spi_slave_wait(lp_spi_transaction_t *trans_desc, int32_t cycles_to_wait); + #ifdef __cplusplus } #endif diff --git a/components/ulp/lp_core/lp_core/lp_core_spi.c b/components/ulp/lp_core/lp_core/lp_core_spi.c index 6a2236e4799..8258ffc3c30 100644 --- a/components/ulp/lp_core/lp_core/lp_core_spi.c +++ b/components/ulp/lp_core/lp_core/lp_core_spi.c @@ -17,6 +17,22 @@ static lp_spi_ll_dev_t *lp_spi_dev = LP_SPI_LL_GET_HW(); +/* Tracks an outstanding lp_core_lp_spi_slave_arm() that has not yet been + * paired with a slave_wait(). The LP_SPI_CMD.reg_usr bit is not a reliable + * "busy" indicator in slave mode (the slave holds it set while merely armed + * and waiting for the master's SCK), so we serialise arm/wait in software. + */ +static volatile bool s_slave_armed = false; + +/* LP SPI data buffer is W0..W15 (16 x 32-bit = 64 B). Per TRM, transfers + * beyond 64 B repeatedly fetch from W15[31:24], so byte 63 is replayed for + * every byte past 64. Skipping W15 (cap at 60 B / W0..W14) avoids that + * aliasing region entirely; longer transfers are split into back-to-back + * 60 B hardware transactions. + */ +#define LP_SPI_MAX_DATA_REG_NUM ((LP_SPI_LL_MAX_BUFFER_SIZE / 4) - 1) /* 15 */ +#define LP_SPI_CHUNK_BYTES (LP_SPI_MAX_DATA_REG_NUM * 4) /* 60 */ + static inline esp_err_t lp_core_spi_wait_for_interrupt(int32_t cycles_to_wait) { uint32_t timeout_start = ulp_lp_core_get_cpu_cycles(); @@ -38,26 +54,37 @@ esp_err_t lp_core_lp_spi_master_transfer(lp_spi_transaction_t *trans_desc, int32 { esp_err_t ret = ESP_OK; - /* Argument sanity check - * Note: The Tx buffer is mandatory for this API. + /* Require at least one of tx_buffer/rx_buffer; length must be 0 when its + * buffer is NULL. */ - if (trans_desc == NULL || trans_desc->tx_buffer == NULL || trans_desc->tx_length == 0) { + if (trans_desc == NULL) { + return ESP_ERR_INVALID_ARG; + } + if (trans_desc->tx_buffer != NULL && trans_desc->tx_length == 0) { + return ESP_ERR_INVALID_ARG; + } + if (trans_desc->rx_buffer != NULL && trans_desc->rx_length == 0) { return ESP_ERR_INVALID_ARG; } - /* Reset the Tx and Rx FIFOs */ - lp_spi_ll_reset_fifos(lp_spi_dev); - - /* Clear any previous interrupts. - * Note: LP SPI does not have any DMA access but the interrupt bit lives in the DMA interrupt register. + /* The peripheral has a single shared bit-length register + * (LP_SPI_MS_DLEN.reg_ms_data_bitlen), so we program it for + * max(tx_length, rx_length) bytes to avoid truncating the longer side. */ - lp_spi_ll_clear_int_trans_done(lp_spi_dev); + uint32_t tx_total = trans_desc->tx_buffer ? trans_desc->tx_length : 0; + uint32_t rx_total = trans_desc->rx_buffer ? trans_desc->rx_length : 0; + uint32_t bus_total = tx_total > rx_total ? tx_total : rx_total; + if (bus_total == 0) { + return ESP_ERR_INVALID_ARG; + } /* Make sure that we do not have any ongoing transactions */ if (lp_spi_ll_is_busy(lp_spi_dev)) { return ESP_ERR_INVALID_STATE; } + lp_spi_ll_clear_int_trans_done(lp_spi_dev); + /* Configure dummy bits */ lp_spi_ll_set_dummy_en(lp_spi_dev, trans_desc->dummy_bits != 0); if (trans_desc->dummy_bits) { @@ -84,167 +111,207 @@ esp_err_t lp_core_lp_spi_master_transfer(lp_spi_transaction_t *trans_desc, int32 : trans_desc->address << (32 - trans_desc->address_bits)); } - /* Set data lines */ - lp_spi_ll_set_mosi_en(lp_spi_dev, true); + /* MOSI gated by tx_buffer to avoid clocking stale W0..W15 on read-only. */ + lp_spi_ll_set_mosi_en(lp_spi_dev, trans_desc->tx_buffer != NULL); lp_spi_ll_set_miso_en(lp_spi_dev, trans_desc->rx_buffer != NULL); - /* Configure the transaction bit length */ - int tx_bitlen = trans_desc->tx_length * 8; - lp_spi_ll_set_data_bitlen(lp_spi_dev, tx_bitlen - 1); - - /* Prepare the data to be transmitted */ - uint32_t tx_idx = 0; - uint32_t rx_idx = 0; - - /* The TRM suggests that the data is sent from and received in the LP_SPI_W0_REG ~ LP_SPI_W15_REG registers. - * The following rules apply: - * 1. The first 64 bytes are sent from/received in LP_SPI_W0_REG ~ LP_SPI_W15_REG - * 2. Bytes 64 - 255 are repeatedly sent from or received in LP_SPI_W15_REG[31:24] - * 3. Subsequent blocks of 256 bytes of data continue to follow the above rules - * - * This driver, however, avoids using the LP_SPI_W15_REG altogether. In other words, - * this driver sends or receives data in chunks of 60 bytes (LP_SPI_W0_REG ~ LP_SPI_W14_REG) - * and does not handle the repeated use of the high-byte of LP_SPI_W15_REG. This design approach - * has been chosen to simplify the data handling logic. + /* Drive the bus one hardware transaction at a time. Each iteration: + * 1. clip ``chunk`` to the remaining bytes, capped at LP_SPI_CHUNK_BYTES; + * 2. preload ``tx_chunk`` TX bytes into W0.. (only if the caller still + * has TX bytes left for this chunk -- TX may end before RX); + * 3. program the shared bit-length register for ``chunk * 8`` SCKs; + * 4. AFIFO reset + apply config + kick (ordering as per the TRM). + * 5. block on TRANS_DONE. + * 6. drain ``rx_chunk`` RX bytes from W0.. (only if the caller still + * wants RX bytes for this chunk -- RX may end before TX). */ - uint8_t max_data_reg_num = (LP_SPI_LL_MAX_BUFFER_SIZE / 4) - 1; // 15 - uint8_t max_data_chunk_size = max_data_reg_num * 4; // 60 - while (tx_idx < trans_desc->tx_length) { - /* Store 4 bytes of data in the data buffer registers serially. */ - lp_spi_ll_write_buffer_word(lp_spi_dev, (tx_idx / 4) & max_data_reg_num, *(uint32_t *)(trans_desc->tx_buffer + tx_idx)); - tx_idx += 4; - - /* Begin transmission of the data if we have pushed all the data or if we have reached the maximum data chunk size */ - if ((tx_idx >= trans_desc->tx_length) || (tx_idx % max_data_chunk_size) == 0) { - /* Apply the configuration */ - lp_spi_ll_apply_config(lp_spi_dev); - - /* Start the transaction */ - lp_spi_ll_start_user_transaction(lp_spi_dev); - - /* Wait for the transaction to complete */ - ret = lp_core_spi_wait_for_interrupt(cycles_to_wait); - if (ret != ESP_OK) { - return ret; - } - - /* Clear the transaction done interrupt */ - lp_spi_ll_clear_int_trans_done(lp_spi_dev); - - /* Fetch the received data if an Rx buffer is provided */ - if (trans_desc->rx_buffer != NULL) { - while (rx_idx < tx_idx) { - *(uint32_t *)(trans_desc->rx_buffer + rx_idx) = lp_spi_ll_read_buffer_word(lp_spi_dev, (rx_idx / 4) & max_data_reg_num); - rx_idx += 4; - // This loop would exit even if we haven't received all the data. - } - } + uint32_t bus_done = 0; + while (bus_done < bus_total) { + uint32_t chunk = bus_total - bus_done; + if (chunk > LP_SPI_CHUNK_BYTES) { + chunk = LP_SPI_CHUNK_BYTES; } + + if (trans_desc->tx_buffer != NULL && bus_done < tx_total) { + uint32_t tx_chunk = tx_total - bus_done; + if (tx_chunk > chunk) { + tx_chunk = chunk; + } + lp_spi_ll_write_buffer_bytes(lp_spi_dev, + (const uint8_t *)trans_desc->tx_buffer + bus_done, + tx_chunk); + } + + lp_spi_ll_set_data_bitlen(lp_spi_dev, chunk * 8 - 1); + + lp_spi_ll_reset_fifos(lp_spi_dev); + + lp_spi_ll_apply_config(lp_spi_dev); + lp_spi_ll_start_user_transaction(lp_spi_dev); + + ret = lp_core_spi_wait_for_interrupt(cycles_to_wait); + if (ret != ESP_OK) { + return ret; + } + + if (trans_desc->rx_buffer != NULL && bus_done < rx_total) { + uint32_t rx_chunk = rx_total - bus_done; + if (rx_chunk > chunk) { + rx_chunk = chunk; + } + lp_spi_ll_read_buffer_bytes(lp_spi_dev, + (uint8_t *)trans_desc->rx_buffer + bus_done, + rx_chunk); + } + + lp_spi_ll_clear_int_trans_done(lp_spi_dev); + + bus_done += chunk; } return ret; } +/* Arm = preload TX + start user phase, return immediately. Pair with + * lp_core_lp_spi_slave_wait(). Splitting arm/wait lets the caller signal + * the master only after the slave is actually listening for SCK. + */ +esp_err_t lp_core_lp_spi_slave_arm(lp_spi_transaction_t *trans_desc) +{ + /* Require at least one of tx_buffer/rx_buffer; length must be 0 when its + * buffer is NULL. + */ + if (trans_desc == NULL || + (trans_desc->rx_buffer == NULL && trans_desc->tx_buffer == NULL)) { + return ESP_ERR_INVALID_ARG; + } + if (trans_desc->rx_buffer != NULL && trans_desc->rx_length == 0) { + return ESP_ERR_INVALID_ARG; + } + if (trans_desc->tx_buffer != NULL && trans_desc->tx_length == 0) { + return ESP_ERR_INVALID_ARG; + } + + /* Refuse to re-arm while a previous arm has not been waited on, + * otherwise the preload below would clobber its W0..W15 mid-transfer. + */ + if (s_slave_armed) { + return ESP_ERR_INVALID_STATE; + } + + /* Clear stale TRANS_DONE so the paired wait sees only this arm. */ + lp_spi_ll_clear_int_trans_done(lp_spi_dev); + + /* Slave direction is reversed vs. master: MOSI carries master->slave + * (caller RX), MISO carries slave->master (caller TX). + */ + lp_spi_ll_set_mosi_en(lp_spi_dev, trans_desc->rx_buffer != NULL); + lp_spi_ll_set_miso_en(lp_spi_dev, trans_desc->tx_buffer != NULL); + + /* Same single shared bit-length register as master + * (LP_SPI_MS_DLEN.reg_ms_data_bitlen). The slave runs a single hardware + * shot capped at LP_SPI_CHUNK_BYTES (60 B, W0..W14, W15 reserved per + * TRM); longer transfers must be split by the caller into successive + * arm/wait pairs. + */ + uint32_t rx_total = trans_desc->rx_buffer ? trans_desc->rx_length : 0; + uint32_t tx_total = trans_desc->tx_buffer ? trans_desc->tx_length : 0; + uint32_t arm_bytes = rx_total > tx_total ? rx_total : tx_total; + if (arm_bytes > LP_SPI_CHUNK_BYTES) { + arm_bytes = LP_SPI_CHUNK_BYTES; + } + lp_spi_ll_set_data_bitlen(lp_spi_dev, arm_bytes * 8 - 1); + + /* Preload TX into W0.. for the slave to drive on MISO when the master + * starts clocking. + */ + if (trans_desc->tx_buffer != NULL) { + uint32_t tx_preload = tx_total > LP_SPI_CHUNK_BYTES + ? LP_SPI_CHUNK_BYTES + : tx_total; + lp_spi_ll_write_buffer_bytes(lp_spi_dev, + (const uint8_t *)trans_desc->tx_buffer, + tx_preload); + } + + /* Reset AFIFOs after preload, before start. */ + lp_spi_ll_reset_fifos(lp_spi_dev); + + /* Skip apply_config() in slave mode: reg_update is master-only and + * re-triggering it here was observed to clock out the previous + * transaction's data. + */ + lp_spi_ll_start_user_transaction(lp_spi_dev); + + s_slave_armed = true; + return ESP_OK; +} + +/* Block on TRANS_DONE from the matching arm, then drain whatever the master + * actually clocked into W0..W15. Pair with lp_core_lp_spi_slave_arm(). + */ +esp_err_t lp_core_lp_spi_slave_wait(lp_spi_transaction_t *trans_desc, int32_t cycles_to_wait) +{ + if (trans_desc == NULL) { + return ESP_ERR_INVALID_ARG; + } + /* Reject ``wait()`` without a preceding ``arm()`` -- otherwise we would + * block on whatever stale TRANS_DONE happens to be latched. + */ + if (!s_slave_armed) { + return ESP_ERR_INVALID_STATE; + } + + /* Block until TRANS_DONE or timeout (cycles_to_wait is in LP CPU cycles). */ + esp_err_t ret = lp_core_spi_wait_for_interrupt(cycles_to_wait); + if (ret != ESP_OK) { + /* Clear the armed latch on the timeout path too so the caller can + * recover by issuing a fresh ``arm()``; otherwise the next + * ``arm()`` would return ESP_ERR_INVALID_STATE forever. + * ``lp_core_spi_wait_for_interrupt()`` already cleared + * TRANS_DONE on its timeout exit, so no extra latch clear here. + */ + s_slave_armed = false; + return ret; + } + + s_slave_armed = false; + + /* Clear the latch so the next arm starts from a clean state. */ + lp_spi_ll_clear_int_trans_done(lp_spi_dev); + + /* The master, not the slave, drives SCK, so the actually-received length + * is decided by the master and only known after TRANS_DONE. Query the + * hardware bit counter (LP_SPI_SLAVE1.reg_slv_data_bitlen), clamp it + * against the caller's rx_length, round up to whole bytes, then drain + * that many bytes from W0.. into rx_buffer. + */ + if (trans_desc->rx_buffer != NULL) { + uint32_t rx_total = trans_desc->rx_length; + uint32_t slave_bitlen = lp_spi_ll_get_slave_rcv_bitlen(lp_spi_dev); + uint32_t req_bitlen = rx_total * 8; + uint32_t valid_bitlen = slave_bitlen > req_bitlen ? req_bitlen : slave_bitlen; + uint32_t valid_bytes = (valid_bitlen + 7) / 8; + if (valid_bytes > rx_total) { + valid_bytes = rx_total; + } + if (valid_bytes > 0) { + lp_spi_ll_read_buffer_bytes(lp_spi_dev, + (uint8_t *)trans_desc->rx_buffer, + valid_bytes); + } + } + + return ESP_OK; +} + esp_err_t lp_core_lp_spi_slave_transfer(lp_spi_transaction_t *trans_desc, int32_t cycles_to_wait) { - esp_err_t ret = ESP_OK; - - /* Argument sanity check - * Note: The Rx buffer is mandatory for this API. - */ - if (trans_desc == NULL || trans_desc->rx_buffer == NULL || trans_desc->rx_length == 0) { - return ESP_ERR_INVALID_ARG; + esp_err_t ret = lp_core_lp_spi_slave_arm(trans_desc); + if (ret != ESP_OK) { + return ret; } - - /* Reset the Tx and Rx FIFOs */ - lp_spi_ll_reset_fifos(lp_spi_dev); - - /* Clear any previous interrupts. - * Note: LP SPI does not have any DMA access but the interrupt bit lives in the DMA interrupt register. - */ - lp_spi_ll_clear_int_trans_done(lp_spi_dev); - - /* Set data lines */ - lp_spi_ll_set_mosi_en(lp_spi_dev, true); - lp_spi_ll_set_miso_en(lp_spi_dev, true); - - /* Configure the transaction bit length */ - int rx_bitlen = trans_desc->rx_length * 8; - lp_spi_ll_set_data_bitlen(lp_spi_dev, rx_bitlen - 1); - - /* Prepare the data to be received */ - uint32_t rx_idx = 0; - uint32_t rcvd_bitlen = 0; - uint32_t rcvd_length_in_bytes = 0; - - /* The LP SPI slave receives data in the LP_SPI_W0_REG ~ LP_SPI_W15_REG registers. - * The following rules apply: - * 1. The first 64 bytes are received in LP_SPI_W0_REG ~ LP_SPI_W15_REG - * 2. The next 64 bytes are overwritten in LP_SPI_W0_REG ~ LP_SPI_W15_REG - * - * Since the peripheral has no protection against overwriting the data, we restrict the - * driver to receive up to 64 bytes of data at a time. - */ - uint32_t length_in_bytes = trans_desc->rx_length; - if (trans_desc->rx_length > LP_SPI_LL_MAX_BUFFER_SIZE) { - /* Truncate the length to the maximum buffer size */ - length_in_bytes = LP_SPI_LL_MAX_BUFFER_SIZE; - } - - while (rx_idx < length_in_bytes) { - /* Wait for the transmission to complete */ - ret = lp_core_spi_wait_for_interrupt(cycles_to_wait); - if (ret != ESP_OK) { - return ret; - } - - /* Fetch the received bit length */ - uint32_t slave_bitlen = lp_spi_ll_get_slave_rcv_bitlen(lp_spi_dev); - rcvd_bitlen = slave_bitlen > (trans_desc->rx_length * 8) ? (trans_desc->rx_length * 8) : slave_bitlen; - rcvd_length_in_bytes = (rcvd_bitlen + 7) / 8; - - /* Read the received data */ - while (rx_idx < rcvd_length_in_bytes) { - *(uint32_t *)(trans_desc->rx_buffer + rx_idx) = lp_spi_ll_read_buffer_word(lp_spi_dev, rx_idx / 4); - rx_idx += 4; - } - - /* Clear the transaction done interrupt */ - lp_spi_ll_clear_int_trans_done(lp_spi_dev); - } - - /* Prepare data for transmission if a Tx buffer is provided */ - if (trans_desc->tx_buffer != NULL) { - uint32_t tx_idx = 0; - uint32_t length_in_bytes = trans_desc->tx_length; - if (length_in_bytes > LP_SPI_LL_MAX_BUFFER_SIZE) { - /* Truncate the length to the maximum buffer size */ - length_in_bytes = LP_SPI_LL_MAX_BUFFER_SIZE; - } - - while (tx_idx < length_in_bytes) { - /* Store 4 bytes of data in the data buffer registers serially. */ - lp_spi_ll_write_buffer_word(lp_spi_dev, tx_idx / 4, *(uint32_t *)(trans_desc->tx_buffer + tx_idx)); - tx_idx += 4; - } - - /* Apply the configuration */ - lp_spi_ll_apply_config(lp_spi_dev); - - /* Start the transaction */ - lp_spi_ll_start_user_transaction(lp_spi_dev); - - /* Wait for the transaction to complete */ - ret = lp_core_spi_wait_for_interrupt(cycles_to_wait); - if (ret != ESP_OK) { - return ret; - } - - /* Clear the transaction done interrupt */ - lp_spi_ll_clear_int_trans_done(lp_spi_dev); - } - - return ret; + return lp_core_lp_spi_slave_wait(trans_desc, cycles_to_wait); } #endif /* SOC_LP_SPI_SUPPORTED */ diff --git a/components/ulp/lp_core/lp_core_spi.c b/components/ulp/lp_core/lp_core_spi.c index acc9225c81f..1c75d6ec2d6 100644 --- a/components/ulp/lp_core/lp_core_spi.c +++ b/components/ulp/lp_core/lp_core_spi.c @@ -29,9 +29,15 @@ static esp_err_t lp_spi_config_io(gpio_num_t pin, rtc_gpio_mode_t direction, uin return ESP_OK; } + /* Disconnect any previous LP GPIO matrix routing */ + lp_gpio_matrix_output(pin, LP_SIG_GPIO_OUT_IDX, false, false); + /* Initialize LP_IO */ ESP_RETURN_ON_ERROR(rtc_gpio_init(pin), LP_SPI_TAG, "LP IO Init failed for GPIO %d", pin); + /* Set direction: INPUT_OUTPUT for SPI pins that may both drive and sense */ + ESP_RETURN_ON_ERROR(rtc_gpio_set_direction(pin, direction), LP_SPI_TAG, "LP IO set direction failed for GPIO %d", pin); + /* Connect this LP_IO to the LP SPI pad-out and pad-in indices on the LP IO Matrix. */ ESP_RETURN_ON_ERROR(lp_gpio_matrix_output(pin, out_pad_idx, false, false), LP_SPI_TAG, "LP IO matrix output failed for %d", pin); ESP_RETURN_ON_ERROR(lp_gpio_matrix_input(pin, in_pad_idx, false), LP_SPI_TAG, "LP IO matrix input failed for %d", pin); @@ -216,7 +222,7 @@ static void lp_spi_slave_setup_device(const lp_spi_slave_config_t *slave_config) } lp_spi_ll_set_full_duplex(lp_spi_dev, true); - /* Configure 3-Wire half-duplex mode */ + /* Configure 3-Wire half-duplex (SIO) mode */ lp_spi_ll_set_sio_mode(lp_spi_dev, (slave_config->flags & LP_SPI_DEVICE_3WIRE) != 0); /* Select the CS pin */ @@ -236,6 +242,9 @@ esp_err_t lp_core_lp_spi_bus_initialize(lp_spi_host_t host_id, const lp_spi_bus_ return ESP_ERR_INVALID_ARG; } + /* Reset LP-SPI peripheral to a known state */ + lp_spi_ll_reset(); + /* Connect the LP SPI peripheral to a "bus", i.e. a set of * GPIO pins defined in the bus_config structure. */ @@ -288,3 +297,23 @@ esp_err_t lp_core_lp_spi_slave_initialize(lp_spi_host_t host_id, const lp_spi_sl return ret; } + +esp_err_t lp_core_lp_spi_bus_deinit(lp_spi_host_t host_id, const lp_spi_bus_config_t *bus_config) +{ + (void)host_id; + + /* Disconnect and deinit LP GPIO pins that were used for SPI signals */ + if (bus_config != NULL) { + if (bus_config->miso_io_num != -1) { + rtc_gpio_deinit(bus_config->miso_io_num); + } + if (bus_config->mosi_io_num != -1) { + rtc_gpio_deinit(bus_config->mosi_io_num); + } + if (bus_config->sclk_io_num != -1) { + rtc_gpio_deinit(bus_config->sclk_io_num); + } + } + + return ESP_OK; +} diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_spi_slave.c b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_spi_slave.c index 81b208295e5..18ceb2b38bc 100644 --- a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_spi_slave.c +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_spi_slave.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -12,18 +12,49 @@ volatile lp_core_test_command_reply_t spi_test_cmd_reply = LP_CORE_COMMAND_NOK; volatile uint8_t spi_slave_tx_buf[100] = {0}; volatile uint8_t spi_slave_rx_buf[100] = {0}; volatile uint32_t spi_rx_len = 0; +volatile uint32_t spi_slave_tx_len = 0; + +/* Set by the LP slave once the hardware is armed (W0..W15 preloaded, + * reg_usr written). The HP slave-side test polls this before sending the + * "LP SPI slave ready" signal that releases the master, so the master + * cannot clock SCK while the slave is still in its arm prologue. + */ +volatile uint32_t spi_slave_armed = 0; int main(void) { - /* Setup SPI transaction */ + /* Wait for the HP core to finish writing spi_rx_len, spi_slave_tx_len, + * and spi_slave_tx_buf before we read them. The HP side sets + * spi_test_cmd_reply to LP_CORE_COMMAND_INVALID as a "go" signal + * after filling the shared-memory buffers. + */ + while (spi_test_cmd_reply == LP_CORE_COMMAND_NOK) { + } + spi_test_cmd_reply = LP_CORE_COMMAND_NOK; + + /* Setup SPI transaction. + * When spi_slave_tx_len > 0 the HP side has preloaded spi_slave_tx_buf + * with echo data that the slave should drive on MISO. + */ lp_spi_transaction_t trans_desc = { .rx_length = spi_rx_len, .rx_buffer = (uint8_t *)spi_slave_rx_buf, - .tx_buffer = NULL, + .tx_length = spi_slave_tx_len, + .tx_buffer = spi_slave_tx_len > 0 ? (uint8_t *)spi_slave_tx_buf : NULL, }; - /* Receive data */ - lp_core_lp_spi_slave_transfer(&trans_desc, -1); + /* Arm the slave hardware, then publish the armed flag so the HP test + * can release the master only after the slave is ready to clock. + */ + if (lp_core_lp_spi_slave_arm(&trans_desc) != ESP_OK) { + spi_test_cmd_reply = LP_CORE_COMMAND_NOK; + return 0; + } + spi_slave_armed = 1; + + /* Block until TRANS_DONE, then drain whatever the master clocked in. */ + lp_core_lp_spi_slave_wait(&trans_desc, -1); + spi_slave_armed = 0; /* Synchronize with the HP core running the test */ spi_test_cmd_reply = LP_CORE_COMMAND_OK; From 8bffc8f603c33888c0b3815cb75f9f49e52f4ce4 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Tue, 30 Jun 2026 10:21:56 +0200 Subject: [PATCH 2/2] test(ulp/lp_spi): improve test stability and add cleanup Co-authored-by: Cursor --- .../lp_core_basic_tests/main/CMakeLists.txt | 1 + .../main/test_lp_core_i2c.c | 2 +- .../main/test_lp_core_spi.c | 273 ++++++++++++++---- .../pytest_lp_core_basic.py | 34 +-- 4 files changed, 234 insertions(+), 76 deletions(-) diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt index 62b10447283..b4c9283040c 100644 --- a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/CMakeLists.txt @@ -71,6 +71,7 @@ idf_component_register(SRCS ${app_sources} INCLUDE_DIRS "lp_core" REQUIRES ulp unity esp_timer test_utils PRIV_REQUIRES esp_driver_gptimer esp_driver_i2c esp_driver_tsens esp_hal_rtc_timer + esp_driver_gpio hal WHOLE_ARCHIVE EMBED_FILES "test_vad_8k.pcm") diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_i2c.c b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_i2c.c index a2fb8dae98c..dfc12a2eac5 100644 --- a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_i2c.c +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_i2c.c @@ -195,4 +195,4 @@ static void i2c_slave_read_write_test(void) TEST_ESP_OK(i2c_del_slave_device(slave_handle)); } -TEST_CASE_MULTIPLE_DEVICES("LP-Core I2C read and write test", "[lp_core][test_env=generic_multi_device][timeout=150]", i2c_master_write_read_test, i2c_slave_read_write_test); +TEST_CASE_MULTIPLE_DEVICES("LP-Core I2C read and write test", "[lp_core_i2c][test_env=generic_multi_device][timeout=150]", i2c_master_write_read_test, i2c_slave_read_write_test); diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_spi.c b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_spi.c index ba593237479..49820fbbb81 100644 --- a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_spi.c +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/test_lp_core_spi.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -9,6 +9,8 @@ #include "lp_core_test_app_spi_slave.h" #include "ulp_lp_core.h" #include "lp_core_spi.h" +#include "driver/rtc_io.h" +#include "hal/lp_spi_ll.h" #include "unity.h" #include "test_utils.h" #include "esp_log.h" @@ -35,6 +37,45 @@ static const char* TAG = "lp_core_spi_test"; #define TEST_DATA_LEN_BYTES 42 uint8_t expected_data[100] = {0}; +/* ------------------------------------------------------------------ */ +/* Cleanup: stop LP core + module-reset LP SPI + deinit LP GPIOs */ +/* ------------------------------------------------------------------ */ + +/* Base LP SPI bus settings */ +lp_spi_host_t host_id = 0; +lp_spi_bus_config_t bus_config = { + .miso_io_num = TEST_GPIO_PIN_MISO, + .mosi_io_num = TEST_GPIO_PIN_MOSI, + .sclk_io_num = TEST_GPIO_PIN_CLK, +}; + +/** + * @brief Reset LP SPI peripheral and GPIO state to a known-clean baseline. + * + * Called at the start AND end of every SPI test so the test is + * self-contained and resilient to whatever ran before it. + */ +static void lp_spi_test_cleanup(void) +{ + ulp_lp_core_stop(); + lp_core_lp_spi_bus_deinit(host_id, &bus_config); + rtc_gpio_deinit(TEST_GPIO_PIN_CS); + + /* After the module reset, explicitly clear TRANS_DONE to prevent + * a stale interrupt from being latched before the next slave_arm. + */ + lp_spi_ll_clear_int_trans_done(LP_SPI_LL_GET_HW()); + + /* Brief settle time for the LP peripheral reset to propagate. */ + vTaskDelay(pdMS_TO_TICKS(5)); + + ESP_LOGI(TAG, "LP SPI cleanup done"); +} + +/* ------------------------------------------------------------------ */ +/* Helpers */ +/* ------------------------------------------------------------------ */ + static void load_and_start_lp_core_firmware(ulp_lp_core_cfg_t* cfg, const uint8_t* firmware_start, const uint8_t* firmware_end) { TEST_ASSERT(ulp_lp_core_load_binary(firmware_start, (firmware_end - firmware_start)) == ESP_OK); @@ -63,19 +104,25 @@ static void setup_expected_data(void) } } -/* Base LP SPI bus settings */ -lp_spi_host_t host_id = 0; -lp_spi_bus_config_t bus_config = { - .miso_io_num = TEST_GPIO_PIN_MISO, - .mosi_io_num = TEST_GPIO_PIN_MOSI, - .sclk_io_num = TEST_GPIO_PIN_CLK, -}; +/** + * @brief Preload the slave's TX buffer so it echoes the same pattern + * the master sends on MOSI back on MISO. + */ +static void setup_slave_echo_data(void) +{ + uint8_t *tx_data = (uint8_t *)&ulp_spi_slave_tx_buf; + ulp_spi_slave_tx_len = TEST_DATA_LEN_BYTES; + + for (int i = 0; i < TEST_DATA_LEN_BYTES; i++) { + tx_data[i] = (i + 1) % 256; + } +} /* Base LP SPI device settings */ lp_spi_device_config_t device = { .cs_io_num = TEST_GPIO_PIN_CS, .spi_mode = 0, - .clock_speed_hz = 10 * 1000, // 10 MHz + .clock_speed_hz = 10 * 1000, // 10 kHz .duty_cycle = 128, // 50% duty cycle }; @@ -87,8 +134,12 @@ lp_spi_slave_config_t slv_device = { static void lp_spi_master_init(int spi_flags, bool setup_master_loop_back) { - /* Initialize LP SPI bus */ - /* Setup loop back for tests which do not use an LP SPI slave for looping back the data. */ + /* Ensure a clean peripheral state before init */ + lp_spi_test_cleanup(); + + /* Initialize LP SPI bus. + * Setup loop back for tests which do not use an LP SPI slave for looping back the data. + */ bus_config.miso_io_num = setup_master_loop_back ? TEST_GPIO_PIN_MOSI : TEST_GPIO_PIN_MISO; TEST_ASSERT(lp_core_lp_spi_bus_initialize(host_id, &bus_config) == ESP_OK); @@ -99,17 +150,21 @@ static void lp_spi_master_init(int spi_flags, bool setup_master_loop_back) static void lp_spi_slave_init(int spi_flags) { + lp_spi_test_cleanup(); + /* Initialize LP SPI bus */ TEST_ASSERT(lp_core_lp_spi_bus_initialize(host_id, &bus_config) == ESP_OK); /* Add LP SPI slave device */ - if (spi_flags != 0) { - slv_device.flags = spi_flags; - } + slv_device.flags = spi_flags; TEST_ASSERT(lp_core_lp_spi_slave_initialize(host_id, &slv_device) == ESP_OK); } -static void lp_spi_master_execute_test(bool wait_for_slave_ready) +/* ------------------------------------------------------------------ */ +/* Master-side test execution */ +/* ------------------------------------------------------------------ */ + +static void lp_spi_master_execute_test(bool wait_for_slave_ready, bool verify_rx) { /* Load and run the LP core firmware */ ulp_lp_core_cfg_t lp_cfg = { @@ -118,6 +173,12 @@ static void lp_spi_master_execute_test(bool wait_for_slave_ready) load_and_start_lp_core_firmware(&lp_cfg, lp_core_main_spi_master_bin_start, lp_core_main_spi_master_bin_end); if (wait_for_slave_ready) { + /* Tell the slave that the master's SPI bus and GPIOs are stable. + * The slave only arms after receiving this signal to avoid + * spurious TRANS_DONE from SCLK glitches during the master's + * boot / GPIO init. + */ + unity_send_signal("LP SPI master initialized"); /* Wait for the HP SPI device to be initialized */ unity_wait_for_signal("LP SPI slave ready"); } @@ -128,105 +189,202 @@ static void lp_spi_master_execute_test(bool wait_for_slave_ready) /* Start the test */ ulp_spi_test_cmd = LP_CORE_LP_SPI_WRITE_READ_TEST; + /* Wait for the test to complete */ while (ulp_spi_test_cmd != LP_CORE_NO_COMMAND) { - /* Wait for the test to complete */ vTaskDelay(1); } /* Verify the received data if we expect the data to be looped back from the LP SPI slave */ uint8_t *rx_data = (uint8_t *)&ulp_spi_master_rx_buf; - for (int i = 0; i < TEST_DATA_LEN_BYTES; i++) { - ESP_LOGI(TAG, "LP SPI master received data: 0x%02x", rx_data[i]); + + if (verify_rx) { + bool mismatch = false; + for (int i = 0; i < TEST_DATA_LEN_BYTES; i++) { + if (rx_data[i] != expected_data[i]) { + ESP_LOGE(TAG, "Master RX mismatch [%d]: expected 0x%02x got 0x%02x", + i, expected_data[i], rx_data[i]); + mismatch = true; + } + } + if (!mismatch) { + ESP_LOGI(TAG, "Master RX: all %d bytes match", TEST_DATA_LEN_BYTES); + } + TEST_ASSERT_EQUAL_HEX8_ARRAY(expected_data, rx_data, ulp_spi_tx_len); + } else { + ESP_LOGI(TAG, "Master TX-only test completed (%d bytes)", TEST_DATA_LEN_BYTES); } - TEST_ASSERT_EQUAL_HEX8_ARRAY(expected_data, rx_data, ulp_spi_tx_len); + lp_spi_test_cleanup(); } -static void lp_spi_slave_execute_test(void) +/* ------------------------------------------------------------------ */ +/* Slave-side test execution */ +/* ------------------------------------------------------------------ */ + +static void lp_spi_slave_execute_test(bool provide_echo) { - /* Load and run the LP core firmware */ + /* Wait until the master's SPI bus and GPIOs are fully initialized + * and stable before arming the slave. This prevents spurious + * TRANS_DONE triggers from SCLK glitches during the master's + * boot / GPIO init sequence (both boards are reset between tests). + */ + unity_wait_for_signal("LP SPI master initialized"); + + /* Ensure shared-memory handshake variables are in the expected + * initial state *before* loading the binary. LP RAM survives HP + * resets, so stale values from a previous test can fool the + * handshake if we don't clear them here. + */ + ulp_spi_slave_armed = 0; + ulp_spi_test_cmd_reply = LP_CORE_COMMAND_NOK; + + /* Load and run the LP core firmware. The LP core spins on + * spi_test_cmd_reply == LP_CORE_COMMAND_NOK until we release it. + */ ulp_lp_core_cfg_t lp_cfg = { .wakeup_source = ULP_LP_CORE_WAKEUP_SOURCE_HP_CPU, }; load_and_start_lp_core_firmware(&lp_cfg, lp_core_main_spi_slave_bin_start, lp_core_main_spi_slave_bin_end); - /* Setup expected test data */ + /* Give the LP core a moment to boot and enter its handshake spin + * loop before we write shared-memory buffers. + */ + vTaskDelay(pdMS_TO_TICKS(10)); + + /* Now that the binary is loaded (and the LP core is spinning), fill + * the shared-memory buffers with test data. + */ setup_expected_data(); + if (provide_echo) { + setup_slave_echo_data(); + } else { + ulp_spi_slave_tx_len = 0; + } + + /* Release the LP core: it will read the lengths, build its + * transaction descriptor, and arm the hardware. + */ + ulp_spi_test_cmd_reply = LP_CORE_COMMAND_INVALID; + + /* Wait for the slave hardware to be armed before releasing the + * master. Bounded wait to avoid hanging the whole test suite if the + * LP core fails for any reason. + */ + int armed_wait_ms = 0; + const int armed_timeout_ms = 5000; + while (ulp_spi_slave_armed == 0) { + vTaskDelay(pdMS_TO_TICKS(10)); + armed_wait_ms += 10; + if (armed_wait_ms >= armed_timeout_ms) { + ESP_LOGE(TAG, "LP SPI slave arm timed out after %d ms", armed_timeout_ms); + TEST_FAIL_MESSAGE("LP SPI slave did not arm in time"); + } + } + ESP_LOGI(TAG, "LP SPI slave armed after ~%d ms", armed_wait_ms); + /* Send signal to LP SPI master */ unity_send_signal("LP SPI slave ready"); /* Wait for the test to complete */ + int done_wait_ms = 0; + const int done_timeout_ms = 10000; while (ulp_spi_test_cmd_reply != LP_CORE_COMMAND_OK) { - vTaskDelay(1); + vTaskDelay(pdMS_TO_TICKS(10)); + done_wait_ms += 10; + if (done_wait_ms >= done_timeout_ms) { + ESP_LOGE(TAG, "LP SPI slave transfer timed out after %d ms", done_timeout_ms); + TEST_FAIL_MESSAGE("LP SPI slave transfer did not complete in time"); + } } /* Verify the received data */ uint8_t *rx_data = (uint8_t *)&ulp_spi_slave_rx_buf; + bool mismatch = false; for (int i = 0; i < TEST_DATA_LEN_BYTES; i++) { - ESP_LOGI(TAG, "LP SPI slave received data: 0x%02x", rx_data[i]); + if (rx_data[i] != expected_data[i]) { + ESP_LOGE(TAG, "Slave RX mismatch [%d]: expected 0x%02x got 0x%02x", + i, expected_data[i], rx_data[i]); + mismatch = true; + } + } + if (!mismatch) { + ESP_LOGI(TAG, "Slave RX: all %d bytes match", TEST_DATA_LEN_BYTES); } TEST_ASSERT_EQUAL_HEX8_ARRAY(expected_data, rx_data, TEST_DATA_LEN_BYTES); + + lp_spi_test_cleanup(); } +/* ================================================================== */ +/* Individual test-case wrappers (master side) */ +/* ================================================================== */ + void test_lp_spi_master(void) { /* Initialize LP SPI in master mode */ lp_spi_master_init(0, false); /* Start the LP SPI master test */ - lp_spi_master_execute_test(true); + lp_spi_master_execute_test(true, true); } +void test_lp_spi_master_3wire(void) +{ + /* Initialize LP SPI in master mode */ + lp_spi_master_init(LP_SPI_DEVICE_3WIRE, false); + + /* In 3-Wire SIO mode the slave does not echo, so the master + * cannot verify RX data — only the slave side verifies RX. + */ + lp_spi_master_execute_test(true, false); +} + +void test_lp_spi_master_lsbfirst(void) +{ + /* Initialize LP SPI in master mode */ + lp_spi_master_init(LP_SPI_DEVICE_BIT_LSBFIRST, false); + + /* Start the LP SPI master test */ + lp_spi_master_execute_test(true, true); +} + +/* ================================================================== */ +/* Individual test-case wrappers (slave side) */ +/* ================================================================== */ + void test_lp_spi_slave(void) { /* Initialize LP SPI in slave mode */ lp_spi_slave_init(0); /* Start the LP SPI slave test */ - lp_spi_slave_execute_test(); -} -void test_lp_spi_master_3wire(void) -{ - /* Initialize LP SPI in master mode */ - int spi_flags = LP_SPI_DEVICE_3WIRE; - lp_spi_master_init(spi_flags, false); - - /* Start the LP SPI master test */ - lp_spi_master_execute_test(true); + lp_spi_slave_execute_test(true); } void test_lp_spi_slave_3wire(void) { /* Initialize LP SPI in slave mode */ - int spi_flags = LP_SPI_DEVICE_3WIRE; - lp_spi_slave_init(spi_flags); + lp_spi_slave_init(LP_SPI_DEVICE_3WIRE); /* Start the LP SPI slave test */ - lp_spi_slave_execute_test(); -} - -void test_lp_spi_master_lsbfirst(void) -{ - /* Initialize LP SPI in master mode */ - int spi_flags = LP_SPI_DEVICE_BIT_LSBFIRST; - lp_spi_master_init(spi_flags, false); - - /* Start the LP SPI master test */ - lp_spi_master_execute_test(true); + lp_spi_slave_execute_test(false); } void test_lp_spi_slave_lsbfirst(void) { /* Initialize LP SPI in slave mode */ - int spi_flags = LP_SPI_DEVICE_BIT_LSBFIRST; - lp_spi_slave_init(spi_flags); + lp_spi_slave_init(LP_SPI_DEVICE_BIT_LSBFIRST); /* Start the LP SPI slave test */ - lp_spi_slave_execute_test(); + lp_spi_slave_execute_test(true); } +/* ================================================================== */ +/* Loopback tests (single-device, no slave needed) */ +/* ================================================================== */ + /* Test LP-SPI master loopback */ TEST_CASE("LP-Core LP-SPI master loopback test", "[lp_core]") { @@ -234,20 +392,23 @@ TEST_CASE("LP-Core LP-SPI master loopback test", "[lp_core]") lp_spi_master_init(0, true); /* Start the LP SPI master test */ - lp_spi_master_execute_test(false); + lp_spi_master_execute_test(false, true); } -/* Test LP-SPI master loopback with active low CS line */ +/* Test LP-SPI master loopback with active high CS line */ TEST_CASE("LP-Core LP-SPI master loopback test with active high CS line", "[lp_core]") { /* Initialize LP SPI in master mode */ - int spi_flags = LP_SPI_DEVICE_CS_ACTIVE_HIGH; - lp_spi_master_init(spi_flags, true); + lp_spi_master_init(LP_SPI_DEVICE_CS_ACTIVE_HIGH, true); /* Start the LP SPI master test */ - lp_spi_master_execute_test(false); + lp_spi_master_execute_test(false, true); } +/* ================================================================== */ +/* Multi-device tests */ +/* ================================================================== */ + /* Test LP-SPI master and LP-SPI slave communication */ TEST_CASE_MULTIPLE_DEVICES("LP-Core LP-SPI master and LP-SPI slave read write test", "[lp_core_spi][test_env=generic_multi_device][timeout=150]", test_lp_spi_master, test_lp_spi_slave); diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/pytest_lp_core_basic.py b/components/ulp/test_apps/lp_core/lp_core_basic_tests/pytest_lp_core_basic.py index 0bb7325637e..2de2aa690ce 100644 --- a/components/ulp/test_apps/lp_core/lp_core_basic_tests/pytest_lp_core_basic.py +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/pytest_lp_core_basic.py @@ -53,31 +53,27 @@ def test_lp_vad(dut: Dut) -> None: @pytest.mark.generic_multi_device @pytest.mark.temp_skip_ci(targets=['esp32s31'], reason='TODO IDF-15572 Enable ULP multi device tests for ESP32-S31') @pytest.mark.parametrize('count', [2], indirect=True) -@pytest.mark.parametrize( - 'config', - [ - 'defaults', - ], - indirect=True, -) +@pytest.mark.parametrize('config', ['defaults'], indirect=True) @idf_parametrize('target', soc_filtered_targets('SOC_LP_I2C_SUPPORTED == 1'), indirect=['target']) -def test_lp_core_multi_device(case_tester: CaseTester) -> None: - # Run only non-UART multi-device cases (e.g. LP I2C); LP UART is covered - # by test_lp_uart_multi_device which targets all LP_CORE_SUPPORTED chips. - non_uart_cases = [case for case in case_tester.test_menu if 'uart' not in case.groups] - for case in non_uart_cases: +def test_lp_i2c_multi_device(case_tester: CaseTester) -> None: + i2c_cases = [case for case in case_tester.test_menu if 'lp_core_i2c' in case.groups] + for case in i2c_cases: case_tester.run_multi_dev_case(case=case, reset=True) @pytest.mark.generic_multi_device @pytest.mark.parametrize('count', [2], indirect=True) -@pytest.mark.parametrize( - 'config', - [ - 'defaults', - ], - indirect=True, -) +@pytest.mark.parametrize('config', ['defaults'], indirect=True) +@idf_parametrize('target', soc_filtered_targets('SOC_LP_SPI_SUPPORTED == 1'), indirect=['target']) +def test_lp_spi_multi_device(case_tester: CaseTester) -> None: + spi_cases = [case for case in case_tester.test_menu if 'lp_core_spi' in case.groups] + for case in spi_cases: + case_tester.run_multi_dev_case(case=case, reset=True) + + +@pytest.mark.generic_multi_device +@pytest.mark.parametrize('count', [2], indirect=True) +@pytest.mark.parametrize('config', ['defaults'], indirect=True) @idf_parametrize('target', soc_filtered_targets('SOC_ULP_LP_UART_SUPPORTED == 1'), indirect=['target']) @pytest.mark.temp_skip_ci(targets=['esp32s31'], reason='TODO IDF-15572 Enable ULP multi device tests for ESP32-S31') def test_lp_uart_multi_device(case_tester: CaseTester) -> None: