From a544a33131f5c2798315830b79ce3822895dff6e Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Fri, 9 May 2025 12:53:53 +0200 Subject: [PATCH] change(lp-core): Update LP I2C and LP UART drivers to use raw interrupt status This commit updates the LP I2C and LP UART drivers to use the raw interrupt status without enabling the interrupts. --- components/hal/esp32c5/include/hal/i2c_ll.h | 13 ++++++++ components/hal/esp32c6/include/hal/i2c_ll.h | 13 ++++++++ components/hal/esp32p4/include/hal/i2c_ll.h | 13 ++++++++ components/ulp/lp_core/lp_core/lp_core_i2c.c | 16 +++++----- components/ulp/lp_core/lp_core/lp_core_uart.c | 31 +++++++------------ .../main/lp_core/test_main_i2c.c | 10 +++++- .../main/lp_core/test_main_uart.c | 8 +++++ 7 files changed, 75 insertions(+), 29 deletions(-) diff --git a/components/hal/esp32c5/include/hal/i2c_ll.h b/components/hal/esp32c5/include/hal/i2c_ll.h index 3553b98847..f16ddb1676 100644 --- a/components/hal/esp32c5/include/hal/i2c_ll.h +++ b/components/hal/esp32c5/include/hal/i2c_ll.h @@ -291,6 +291,19 @@ static inline void i2c_ll_get_intr_mask(i2c_dev_t *hw, uint32_t *intr_status) *intr_status = hw->int_status.val; } +/** + * @brief Get I2C raw interrupt status + * + * @param hw Beginning address of the peripheral registers + * + * @return I2C raw interrupt status + */ +__attribute__((always_inline)) +static inline void i2c_ll_get_intr_raw_mask(i2c_dev_t *hw, uint32_t *intr_status) +{ + *intr_status = hw->int_raw.val; +} + /** * @brief Configure I2C memory access mode, FIFO mode or non-FIFO mode * diff --git a/components/hal/esp32c6/include/hal/i2c_ll.h b/components/hal/esp32c6/include/hal/i2c_ll.h index 01df5e3489..49da17be0d 100644 --- a/components/hal/esp32c6/include/hal/i2c_ll.h +++ b/components/hal/esp32c6/include/hal/i2c_ll.h @@ -295,6 +295,19 @@ static inline void i2c_ll_get_intr_mask(i2c_dev_t *hw, uint32_t *intr_status) *intr_status = hw->int_status.val; } +/** + * @brief Get I2C raw interrupt status + * + * @param hw Beginning address of the peripheral registers + * + * @return I2C raw interrupt status + */ +__attribute__((always_inline)) +static inline void i2c_ll_get_intr_raw_mask(i2c_dev_t *hw, uint32_t *intr_status) +{ + *intr_status = hw->int_raw.val; +} + /** * @brief Configure I2C memory access mode, FIFO mode or non-FIFO mode * diff --git a/components/hal/esp32p4/include/hal/i2c_ll.h b/components/hal/esp32p4/include/hal/i2c_ll.h index d1bfb68f51..37fb6e1038 100644 --- a/components/hal/esp32p4/include/hal/i2c_ll.h +++ b/components/hal/esp32p4/include/hal/i2c_ll.h @@ -310,6 +310,19 @@ static inline void i2c_ll_get_intr_mask(i2c_dev_t *hw, uint32_t *intr_status) *intr_status = hw->int_status.val; } +/** + * @brief Get I2C raw interrupt status + * + * @param hw Beginning address of the peripheral registers + * + * @return I2C raw interrupt status + */ +__attribute__((always_inline)) +static inline void i2c_ll_get_intr_raw_mask(i2c_dev_t *hw, uint32_t *intr_status) +{ + *intr_status = hw->int_raw.val; +} + /** * @brief Configure I2C memory access mode, FIFO mode or non-FIFO mode * diff --git a/components/ulp/lp_core/lp_core/lp_core_i2c.c b/components/ulp/lp_core/lp_core/lp_core_i2c.c index cc2d0df7be..dbfdc9d6df 100644 --- a/components/ulp/lp_core/lp_core/lp_core_i2c.c +++ b/components/ulp/lp_core/lp_core/lp_core_i2c.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -72,7 +72,7 @@ static inline esp_err_t lp_core_i2c_wait_for_interrupt(uint32_t intr_mask, int32 uint32_t to = 0; while (1) { - i2c_ll_get_intr_mask(dev, &intr_status); + i2c_ll_get_intr_raw_mask(dev, &intr_status); if (intr_status & intr_mask) { if (intr_status & LP_I2C_NACK_INT_ST) { /* The ACK/NACK received during a WRITE operation does not match the expected ACK/NACK level @@ -82,9 +82,8 @@ static inline esp_err_t lp_core_i2c_wait_for_interrupt(uint32_t intr_mask, int32 return ESP_ERR_INVALID_RESPONSE; } else if (intr_status & LP_I2C_TRANS_COMPLETE_INT_ST_M) { /* Transaction complete. - * Disable and clear interrupt bits and break + * Clear interrupt bits and break */ - i2c_ll_disable_intr_mask(dev, intr_mask); i2c_ll_clear_intr_mask(dev, intr_mask); break; } else { @@ -105,8 +104,7 @@ static inline esp_err_t lp_core_i2c_wait_for_interrupt(uint32_t intr_mask, int32 ulp_lp_core_delay_cycles(1); to++; if (to >= ticks_to_wait) { - /* Disable and clear interrupt bits */ - i2c_ll_disable_intr_mask(dev, intr_mask); + /* Timeout. Clear interrupt bits and return an error */ i2c_ll_clear_intr_mask(dev, intr_mask); return ESP_ERR_TIMEOUT; } @@ -169,7 +167,7 @@ esp_err_t lp_core_i2c_master_read_from_device(i2c_port_t lp_i2c_num, uint16_t de /* Enable trans complete interrupt and end detect interrupt for read/write operation */ uint32_t intr_mask = (1 << LP_I2C_TRANS_COMPLETE_INT_ST_S) | (1 << LP_I2C_END_DETECT_INT_ST_S); - i2c_ll_enable_intr_mask(dev, intr_mask); + i2c_ll_clear_intr_mask(dev, intr_mask); /* Read data */ uint32_t fifo_size = 0; @@ -273,7 +271,7 @@ esp_err_t lp_core_i2c_master_write_to_device(i2c_port_t lp_i2c_num, uint16_t dev /* Enable LP_I2C_NACK_INT to check for ACK errors */ intr_mask |= (1 << LP_I2C_NACK_INT_ST_S); } - i2c_ll_enable_intr_mask(dev, intr_mask); + i2c_ll_clear_intr_mask(dev, intr_mask); /* Write data */ uint32_t fifo_available = LP_I2C_FIFO_LEN - addr_len; // Initially, 1 or 2 fifo slots are taken by the device address @@ -358,7 +356,7 @@ esp_err_t lp_core_i2c_master_write_read_device(i2c_port_t lp_i2c_num, uint16_t d /* Enable LP_I2C_NACK_INT to check for ACK errors */ intr_mask |= (1 << LP_I2C_NACK_INT_ST_S); } - i2c_ll_enable_intr_mask(dev, intr_mask); + i2c_ll_clear_intr_mask(dev, intr_mask); /* Execute RSTART command to send the START bit */ lp_core_i2c_format_cmd(cmd_idx++, I2C_LL_CMD_RESTART, 0, 0, 0, 0); diff --git a/components/ulp/lp_core/lp_core/lp_core_uart.c b/components/ulp/lp_core/lp_core/lp_core_uart.c index f1163fba79..a8196bfb48 100644 --- a/components/ulp/lp_core/lp_core/lp_core_uart.c +++ b/components/ulp/lp_core/lp_core/lp_core_uart.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -99,7 +99,6 @@ esp_err_t lp_core_uart_write_bytes(uart_port_t lp_uart_num, const void *src, siz /* Enable the Tx done interrupt */ uint32_t intr_mask = LP_UART_TX_INT_FLAG | LP_UART_ERR_INT_FLAG; uart_hal_clr_intsts_mask(&hal, intr_mask); - uart_hal_ena_intr_mask(&hal, intr_mask); /* Transmit data */ uint32_t tx_len; @@ -118,22 +117,23 @@ esp_err_t lp_core_uart_write_bytes(uart_port_t lp_uart_num, const void *src, siz /* We have managed to write some data to the Tx FIFO. Check Tx interrupt status */ while (1) { /* Fetch the interrupt status */ - intr_status = uart_hal_get_intsts_mask(&hal); + intr_status = uart_hal_get_intraw_mask(&hal); if (intr_status & LP_UART_TX_INT_FLAG) { /* Clear interrupt status and break */ uart_hal_clr_intsts_mask(&hal, intr_mask); break; } else if ((intr_status & LP_UART_ERR_INT_FLAG)) { /* Transaction error. Abort */ + uart_hal_clr_intsts_mask(&hal, intr_mask); return ESP_FAIL; } /* Check for transaction timeout */ ret = lp_core_uart_check_timeout(intr_mask, timeout, &to); if (ret == ESP_ERR_TIMEOUT) { - /* Timeout */ - uart_hal_disable_intr_mask(&hal, intr_mask); - return ret; + /* Timeout. Clear interrupt status and break */ + uart_hal_clr_intsts_mask(&hal, intr_mask); + break; } } @@ -144,16 +144,13 @@ esp_err_t lp_core_uart_write_bytes(uart_port_t lp_uart_num, const void *src, siz /* Tx FIFO does not have empty slots. Check for transaction timeout */ ret = lp_core_uart_check_timeout(intr_mask, timeout, &to); if (ret == ESP_ERR_TIMEOUT) { - /* Timeout */ - uart_hal_disable_intr_mask(&hal, intr_mask); - return ret; + /* Timeout. Clear interrupt status and break */ + uart_hal_clr_intsts_mask(&hal, intr_mask); + break; } } } - /* Disable the Tx done interrupt */ - uart_hal_disable_intr_mask(&hal, intr_mask); - return ret; } @@ -179,8 +176,6 @@ int lp_core_uart_read_bytes(uart_port_t lp_uart_num, void *buf, size_t size, int /* Enable the Rx interrupts */ uint32_t intr_mask = LP_UART_RX_INT_FLAG | LP_UART_ERR_INT_FLAG; uart_hal_clr_intsts_mask(&hal, intr_mask); - uart_hal_ena_intr_mask(&hal, intr_mask); - /* Receive data */ int rx_len = 0; uint32_t bytes_rcvd = 0; @@ -198,7 +193,7 @@ int lp_core_uart_read_bytes(uart_port_t lp_uart_num, void *buf, size_t size, int if (rx_len) { /* We have some data to read from the Rx FIFO. Check Rx interrupt status */ - intr_status = uart_hal_get_intsts_mask(&hal); + intr_status = uart_hal_get_intraw_mask(&hal); if ((intr_status & UART_INTR_RXFIFO_FULL) || (intr_status & UART_INTR_RXFIFO_TOUT)) { /* This is expected. Clear interrupt status and break */ @@ -212,7 +207,6 @@ int lp_core_uart_read_bytes(uart_port_t lp_uart_num, void *buf, size_t size, int } else if ((intr_status & LP_UART_ERR_INT_FLAG)) { /* Transaction error. Abort */ uart_hal_clr_intsts_mask(&hal, intr_mask); - uart_hal_disable_intr_mask(&hal, intr_mask); return -1; } @@ -223,14 +217,13 @@ int lp_core_uart_read_bytes(uart_port_t lp_uart_num, void *buf, size_t size, int /* We have no data to read from the Rx FIFO. Check for transaction timeout */ ret = lp_core_uart_check_timeout(intr_mask, timeout, &to); if (ret == ESP_ERR_TIMEOUT) { + /* Timeout. Clear interrupt status and break */ + uart_hal_clr_intsts_mask(&hal, intr_mask); break; } } } - /* Disable the Rx interrupts */ - uart_hal_disable_intr_mask(&hal, intr_mask); - /* Return the number of bytes received */ return bytes_rcvd; } diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_i2c.c b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_i2c.c index 06657c0de6..d5ed84990b 100644 --- a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_i2c.c +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_i2c.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -8,6 +8,7 @@ #include "test_shared.h" #include "ulp_lp_core_utils.h" #include "ulp_lp_core_i2c.h" +#include "ulp_lp_core_interrupts.h" #define LP_I2C_TRANS_WAIT_FOREVER -1 @@ -19,6 +20,13 @@ uint8_t data_wr[DATA_LENGTH] = {}; int main(void) { + /* Enable interrupts. + * This does not affect how the LP I2C functions + * but it will add extra test coverage to make sure + * the interrupt handler does not cause any issues. + */ + ulp_lp_core_intr_enable(); + lp_core_i2c_master_read_from_device(LP_I2C_NUM_0, I2C_SLAVE_ADDRESS, data_rd, RW_TEST_LENGTH, LP_I2C_TRANS_WAIT_FOREVER); read_test_reply = LP_CORE_COMMAND_OK; diff --git a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_uart.c b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_uart.c index aa7ae65f2e..dbfb69096a 100644 --- a/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_uart.c +++ b/components/ulp/test_apps/lp_core/lp_core_basic_tests/main/lp_core/test_main_uart.c @@ -11,6 +11,7 @@ #include "ulp_lp_core_uart.h" #include "ulp_lp_core_print.h" #include "soc/soc_caps.h" +#include "ulp_lp_core_interrupts.h" #define LP_UART_PORT_NUM LP_UART_NUM_0 #define LP_UART_BUFFER_LEN UART_BUF_SIZE @@ -37,6 +38,13 @@ volatile char test_character; int main(void) { + /* Enable interrupts. + * This does not affect how the LP UART functions + * but it will add extra test coverage to make sure + * the interrupt handler does not cause any issues. + */ + ulp_lp_core_intr_enable(); + while (1) { /* Wait for the HP core to start the test */ while (test_cmd == LP_CORE_NO_COMMAND) {