From 6cd7f7487718f02dcac4d416d27d3cdd67819853 Mon Sep 17 00:00:00 2001 From: Meet Patel Date: Mon, 18 Aug 2025 14:21:29 +0530 Subject: [PATCH 1/2] refactor(ulp_riscv): Modify ESP_EARLY_LOG to ESP_LOG and move it outside critical section Moved the error logs outside critical section for i2c communication errors like READ fail, WRITE fail etc. in the ulp_riscv_i2c component Also changed the error log API from ESP_EARLY_LOG to ESP_LOG, so we can support tag based filtering and enabling/disabling of logs Closes https://github.com/espressif/esp-idf/issues/17425 --- components/ulp/ulp_riscv/ulp_riscv_i2c.c | 27 ++++++++++++++++-------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/components/ulp/ulp_riscv/ulp_riscv_i2c.c b/components/ulp/ulp_riscv/ulp_riscv_i2c.c index 138d479642..95aea4aeb3 100644 --- a/components/ulp/ulp_riscv/ulp_riscv_i2c.c +++ b/components/ulp/ulp_riscv/ulp_riscv_i2c.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -321,6 +321,7 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) uint32_t i = 0; uint32_t cmd_idx = 0; esp_err_t ret = ESP_OK; + uint32_t status = 0; if (size == 0) { // Quietly return @@ -379,10 +380,7 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) /* Clear the Rx data interrupt bit */ SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, RTC_I2C_RX_DATA_INT_CLR); } else { - ESP_EARLY_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: Read Failed!"); - uint32_t status = READ_PERI_REG(RTC_I2C_INT_RAW_REG); - ESP_EARLY_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: RTC I2C Interrupt Raw Reg 0x%"PRIx32"", status); - ESP_EARLY_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: RTC I2C Status Reg 0x%"PRIx32"", READ_PERI_REG(RTC_I2C_STATUS_REG)); + status = READ_PERI_REG(RTC_I2C_INT_RAW_REG); SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, status); break; } @@ -390,6 +388,12 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) portEXIT_CRITICAL(&rtc_i2c_lock); + if (ret != ESP_OK) { + ESP_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: Read Failed!"); + ESP_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: RTC I2C Interrupt Raw Reg 0x%"PRIx32"", status); + ESP_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: RTC I2C Status Reg 0x%"PRIx32"", READ_PERI_REG(RTC_I2C_STATUS_REG)); + } + /* Clear the RTC I2C transmission bits */ CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START_FORCE); CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START); @@ -417,6 +421,7 @@ void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) uint32_t i = 0; uint32_t cmd_idx = 0; esp_err_t ret = ESP_OK; + uint32_t status = 0; if (size == 0) { // Quietly return @@ -455,10 +460,7 @@ void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) /* Clear the Tx data interrupt bit */ SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, RTC_I2C_TX_DATA_INT_CLR); } else { - ESP_EARLY_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: Write Failed!"); - uint32_t status = READ_PERI_REG(RTC_I2C_INT_RAW_REG); - ESP_EARLY_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: RTC I2C Interrupt Raw Reg 0x%"PRIx32"", status); - ESP_EARLY_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: RTC I2C Status Reg 0x%"PRIx32"", READ_PERI_REG(RTC_I2C_STATUS_REG)); + status = READ_PERI_REG(RTC_I2C_INT_RAW_REG); SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, status); break; } @@ -466,6 +468,13 @@ void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) portEXIT_CRITICAL(&rtc_i2c_lock); + /* In case of error, print the status after critical section */ + if (ret != ESP_OK) { + ESP_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: Write Failed!"); + ESP_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: RTC I2C Interrupt Raw Reg 0x%"PRIx32"", status); + ESP_LOGE(RTCI2C_TAG, "ulp_riscv_i2c: RTC I2C Status Reg 0x%"PRIx32"", READ_PERI_REG(RTC_I2C_STATUS_REG)); + } + /* Clear the RTC I2C transmission bits */ CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START_FORCE); CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START); From 6e2db679da3704d9870c9f25a1543b68ae7dd1ea Mon Sep 17 00:00:00 2001 From: Meet Patel Date: Wed, 20 Aug 2025 11:55:39 +0530 Subject: [PATCH 2/2] refactor(ulp_riscv): Modify ulp i2c read/write functions to return error code Updated the i2c read/write APIs ulp_riscv_i2c_master_read_from_device and ulp_riscv_i2c_master_write_to_device in ulp_riscv component to return error codes back to the application Closes https://github.com/espressif/esp-idf/issues/15904 --- .../ulp/ulp_riscv/include/ulp_riscv_i2c.h | 8 +++++--- .../ulp_core/include/ulp_riscv_i2c_ulp_core.h | 9 ++++++--- .../ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c | 19 ++++++++++++++----- components/ulp/ulp_riscv/ulp_riscv_i2c.c | 14 ++++++++++---- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/components/ulp/ulp_riscv/include/ulp_riscv_i2c.h b/components/ulp/ulp_riscv/include/ulp_riscv_i2c.h index 503fa7b20c..907e62c1ec 100644 --- a/components/ulp/ulp_riscv/include/ulp_riscv_i2c.h +++ b/components/ulp/ulp_riscv/include/ulp_riscv_i2c.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -114,8 +114,9 @@ void ulp_riscv_i2c_master_set_slave_reg_addr(uint8_t slave_reg_addr); * * @param data_rd Buffer to hold data to be read * @param size Size of data to be read in bytes + * @return esp_err_t ESP_OK when successful */ -void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size); +esp_err_t ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size); /** * @brief Write to I2C slave device @@ -124,8 +125,9 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size); * * @param data_wr Buffer which holds the data to be written * @param size Size of data to be written in bytes + * @return esp_err_t ESP_OK when successful */ -void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size); +esp_err_t ulp_riscv_i2c_master_write_to_device(const uint8_t *data_wr, size_t size); /** * @brief Initialize and configure the RTC I2C for use by ULP RISC-V diff --git a/components/ulp/ulp_riscv/ulp_core/include/ulp_riscv_i2c_ulp_core.h b/components/ulp/ulp_riscv/ulp_core/include/ulp_riscv_i2c_ulp_core.h index 41383245ea..1202a21059 100644 --- a/components/ulp/ulp_riscv/ulp_core/include/ulp_riscv_i2c_ulp_core.h +++ b/components/ulp/ulp_riscv/ulp_core/include/ulp_riscv_i2c_ulp_core.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -12,6 +12,7 @@ extern "C" { #include #include +#include "esp_err.h" /** * @brief Set the I2C slave device address @@ -34,8 +35,9 @@ void ulp_riscv_i2c_master_set_slave_reg_addr(uint8_t slave_reg_addr); * * @param data_rd Buffer to hold data to be read * @param size Size of data to be read in bytes + * @return esp_err_t ESP_OK when successful */ -void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size); +esp_err_t ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size); /** * @brief Write to I2C slave device @@ -44,8 +46,9 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size); * * @param data_wr Buffer which holds the data to be written * @param size Size of data to be written in bytes + * @return esp_err_t ESP_OK when successful */ -void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size); +esp_err_t ulp_riscv_i2c_master_write_to_device(const uint8_t *data_wr, size_t size); #ifdef __cplusplus } diff --git a/components/ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c b/components/ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c index 75c39faf46..65611628ee 100644 --- a/components/ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c +++ b/components/ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c @@ -1,8 +1,9 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ +#include "esp_err.h" #include "ulp_riscv_i2c_ulp_core.h" #include "ulp_riscv_utils.h" #include "soc/rtc_i2c_reg.h" @@ -131,14 +132,15 @@ void ulp_riscv_i2c_master_set_slave_reg_addr(uint8_t slave_reg_addr) * | Slave | | | ACK | | ACK | | | ACK | DATA | | DATA | | | * |--------|--------|---------|--------|--------|--------|--------|---------|--------|--------|--------|--------|--------|--------| */ -void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) +esp_err_t ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) { uint32_t i = 0; uint32_t cmd_idx = 0; + esp_err_t ret = ESP_OK; if (size == 0) { // Quietly return - return; + return ESP_ERR_INVALID_ARG; } // Workaround for IDF-9145 @@ -197,6 +199,7 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) } else { /* Error in transaction */ CLEAR_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, READ_PERI_REG(RTC_I2C_INT_ST_REG)); + ret = ESP_ERR_INVALID_RESPONSE; break; } } @@ -207,6 +210,8 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) // Workaround for IDF-9145 ULP_RISCV_EXIT_CRITICAL(); + + return ret; } /* @@ -226,14 +231,15 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) * | Slave | | | ACK | | ACK | | ACK | | ACK | | * |--------|--------|---------|--------|--------|--------|--------|--------|--------|--------|--------| */ -void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) +esp_err_t ulp_riscv_i2c_master_write_to_device(const uint8_t *data_wr, size_t size) { uint32_t i = 0; uint32_t cmd_idx = 0; + esp_err_t ret = ESP_OK; if (size == 0) { // Quietly return - return; + return ESP_ERR_INVALID_ARG; } // Workaround for IDF-9145 @@ -271,6 +277,7 @@ void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, RTC_I2C_TX_DATA_INT_CLR); } else { SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, READ_PERI_REG(RTC_I2C_INT_ST_REG)); + ret = ESP_ERR_INVALID_RESPONSE; break; } } @@ -281,4 +288,6 @@ void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) // Workaround for IDF-9145 ULP_RISCV_EXIT_CRITICAL(); + + return ret; } diff --git a/components/ulp/ulp_riscv/ulp_riscv_i2c.c b/components/ulp/ulp_riscv/ulp_riscv_i2c.c index 95aea4aeb3..12881ae57a 100644 --- a/components/ulp/ulp_riscv/ulp_riscv_i2c.c +++ b/components/ulp/ulp_riscv/ulp_riscv_i2c.c @@ -316,7 +316,7 @@ void ulp_riscv_i2c_master_set_slave_reg_addr(uint8_t slave_reg_addr) * | Slave | | | ACK | | ACK | | | ACK | DATA | | DATA | | | * |--------|--------|---------|--------|--------|--------|--------|---------|--------|--------|--------|--------|--------|--------| */ -void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) +esp_err_t ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) { uint32_t i = 0; uint32_t cmd_idx = 0; @@ -325,7 +325,7 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) if (size == 0) { // Quietly return - return; + return ESP_ERR_INVALID_ARG; } /* By default, RTC I2C controller is hard wired to use CMD2 register onwards for read operations */ @@ -382,6 +382,7 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) } else { status = READ_PERI_REG(RTC_I2C_INT_RAW_REG); SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, status); + ret = ESP_ERR_INVALID_RESPONSE; break; } } @@ -397,6 +398,8 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) /* Clear the RTC I2C transmission bits */ CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START_FORCE); CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START); + + return ret; } /* @@ -416,7 +419,7 @@ void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) * | Slave | | | ACK | | ACK | | ACK | | ACK | | * |--------|--------|---------|--------|--------|--------|--------|--------|--------|--------|--------| */ -void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) +esp_err_t ulp_riscv_i2c_master_write_to_device(const uint8_t *data_wr, size_t size) { uint32_t i = 0; uint32_t cmd_idx = 0; @@ -425,7 +428,7 @@ void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) if (size == 0) { // Quietly return - return; + return ESP_ERR_INVALID_ARG; } /* By default, RTC I2C controller is hard wired to use CMD0 and CMD1 registers for write operations */ @@ -462,6 +465,7 @@ void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) } else { status = READ_PERI_REG(RTC_I2C_INT_RAW_REG); SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, status); + ret = ESP_ERR_INVALID_RESPONSE; break; } } @@ -478,6 +482,8 @@ void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size) /* Clear the RTC I2C transmission bits */ CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START_FORCE); CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START); + + return ret; } esp_err_t ulp_riscv_i2c_master_init(const ulp_riscv_i2c_cfg_t *cfg)