From d1d95b32557b4d72dae3704ae93d9da8b489970e Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Wed, 29 Jul 2026 15:34:54 +0200 Subject: [PATCH] test(ulp/lp_spi): improve test stability and add cleanup Reset the LP SPI peripheral and GPIOs before and after every SPI case so each test starts from a known-clean baseline, and gate the master on an explicit slave-armed handshake so the master cannot clock the bus while the slave is still in its arm prologue. Verify received data against the expected pattern with bounded waits instead of merely logging it. Add a dedicated LP SPI multi-device pytest for esp32p4 that runs the retagged lp_core_spi cases, and tag the LP I2C case accordingly. --- .../main/lp_core/test_main_spi_slave.c | 41 ++- .../main/test_lp_core_i2c.c | 2 +- .../main/test_lp_core_spi.c | 273 ++++++++++++++---- .../pytest_lp_core_basic.py | 9 + 4 files changed, 263 insertions(+), 62 deletions(-) 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; 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 a2885ecf768..d68cc9d01dc 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 @@ -144,4 +144,4 @@ static void i2c_slave_read_write_test(void) i2c_driver_delete(I2C_SLAVE_NUM); } -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 b23e99bd099..472e1d123a9 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 "soc/lp_spi_struct.h" #include "unity.h" #include "test_utils.h" #include "esp_log.h" @@ -31,6 +33,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); + + /* Explicitly clear TRANS_DONE to prevent a stale interrupt from being + * latched before the next slave_arm. + */ + LP_SPI.spi_dma_int_clr.reg_trans_done_int_clr = 1; + + /* 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); @@ -59,19 +100,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 }; @@ -83,8 +130,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); @@ -95,17 +146,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 = { @@ -114,6 +169,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"); } @@ -124,105 +185,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]") { @@ -230,20 +388,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 7c26a0b6021..8a405f00e69 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 @@ -19,3 +19,12 @@ def test_lp_core_multi_device(case_tester) -> None: # type: ignore for case in case_tester.test_menu: if case.attributes.get('test_env', 'generic_multi_device') == 'generic_multi_device': case_tester.run_multi_dev_case(case=case, reset=True) + + +@pytest.mark.generic_multi_device +@pytest.mark.parametrize('count', [2], indirect=True) +@idf_parametrize('target', ['esp32p4'], indirect=['target']) +def test_lp_spi_multi_device(case_tester) -> None: # type: ignore + for case in case_tester.test_menu: + if 'lp_core_spi' in case.groups: + case_tester.run_multi_dev_case(case=case, reset=True)