From 19205b0696b17e7a04ba19c61ad738f77006461b Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Tue, 10 Aug 2021 15:29:07 +0800 Subject: [PATCH 1/3] I2C: Fix i2c write fake timeout and WDT triggered --- components/hal/i2c_hal_iram.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/components/hal/i2c_hal_iram.c b/components/hal/i2c_hal_iram.c index 197143ae14..080d19c919 100644 --- a/components/hal/i2c_hal_iram.c +++ b/components/hal/i2c_hal_iram.c @@ -16,25 +16,30 @@ void i2c_hal_master_handle_tx_event(i2c_hal_context_t *hal, i2c_intr_event_t *event) { - i2c_ll_master_get_event(hal->dev, event); - if ((*event < I2C_INTR_EVENT_END_DET) || - (*event == I2C_INTR_EVENT_TRANS_DONE)) { - i2c_ll_master_disable_tx_it(hal->dev); - i2c_ll_master_clr_tx_it(hal->dev); - } else if (*event == I2C_INTR_EVENT_END_DET) { - i2c_ll_master_clr_tx_it(hal->dev); + if (i2c_ll_get_intsts_mask(hal->dev) != 0) { + // If intr status is 0, no need to handle it. + i2c_ll_master_get_event(hal->dev, event); + if ((*event < I2C_INTR_EVENT_END_DET) || + (*event == I2C_INTR_EVENT_TRANS_DONE)) { + i2c_ll_master_disable_tx_it(hal->dev); + i2c_ll_master_clr_tx_it(hal->dev); + } else if (*event == I2C_INTR_EVENT_END_DET) { + i2c_ll_master_clr_tx_it(hal->dev); + } } } void i2c_hal_master_handle_rx_event(i2c_hal_context_t *hal, i2c_intr_event_t *event) { - i2c_ll_master_get_event(hal->dev, event); - if ((*event < I2C_INTR_EVENT_END_DET) || - (*event == I2C_INTR_EVENT_TRANS_DONE)) { - i2c_ll_master_disable_rx_it(hal->dev); - i2c_ll_master_clr_rx_it(hal->dev); - } else if (*event == I2C_INTR_EVENT_END_DET) { - i2c_ll_master_clr_rx_it(hal->dev); + if (i2c_ll_get_intsts_mask(hal->dev) != 0) { + i2c_ll_master_get_event(hal->dev, event); + if ((*event < I2C_INTR_EVENT_END_DET) || + (*event == I2C_INTR_EVENT_TRANS_DONE)) { + i2c_ll_master_disable_rx_it(hal->dev); + i2c_ll_master_clr_rx_it(hal->dev); + } else if (*event == I2C_INTR_EVENT_END_DET) { + i2c_ll_master_clr_rx_it(hal->dev); + } } } From b6bbb5e0a2722ea8e7c7ba035976ff99f4d3214f Mon Sep 17 00:00:00 2001 From: Chen Yi Qun Date: Fri, 6 Aug 2021 15:47:12 +0800 Subject: [PATCH 2/3] I2C: add conf_update for esp32c3 i2c --- components/driver/i2c.c | 9 ++++++++- components/hal/esp32c3/include/hal/i2c_ll.h | 7 +++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/components/driver/i2c.c b/components/driver/i2c.c index f02254a0fe..0fa119d934 100644 --- a/components/driver/i2c.c +++ b/components/driver/i2c.c @@ -542,6 +542,7 @@ esp_err_t i2c_set_data_mode(i2c_port_t i2c_num, i2c_trans_mode_t tx_trans_mode, I2C_CHECK(rx_trans_mode < I2C_DATA_MODE_MAX, I2C_TRANS_MODE_ERR_STR, ESP_ERR_INVALID_ARG); I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); i2c_hal_set_data_mode(&(i2c_context[i2c_num].hal), tx_trans_mode, rx_trans_mode); + i2c_hal_update_config(&(i2c_context[i2c_num].hal)); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); return ESP_OK; } @@ -678,13 +679,13 @@ esp_err_t i2c_param_config(i2c_port_t i2c_num, const i2c_config_t *i2c_conf) i2c_hal_set_sda_timing(&(i2c_context[i2c_num].hal), I2C_SLAVE_SDA_SAMPLE_DEFAULT, I2C_SLAVE_SDA_HOLD_DEFAULT); i2c_hal_set_tout(&(i2c_context[i2c_num].hal), I2C_SLAVE_TIMEOUT_DEFAULT); i2c_hal_enable_slave_rx_it(&(i2c_context[i2c_num].hal)); - i2c_hal_update_config(&(i2c_context[i2c_num].hal)); } else { i2c_hal_master_init(&(i2c_context[i2c_num].hal), i2c_num); //Default, we enable hardware filter i2c_hal_set_filter(&(i2c_context[i2c_num].hal), I2C_FILTER_CYC_NUM_DEF); i2c_hal_set_bus_timing(&(i2c_context[i2c_num].hal), i2c_conf->master.clk_speed, i2c_get_clk_src(i2c_conf)); } + i2c_hal_update_config(&(i2c_context[i2c_num].hal)); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); return ESP_OK; } @@ -697,6 +698,7 @@ esp_err_t i2c_set_period(i2c_port_t i2c_num, int high_period, int low_period) I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); i2c_hal_set_scl_timing(&(i2c_context[i2c_num].hal), high_period, low_period); + i2c_hal_update_config(&(i2c_context[i2c_num].hal)); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); return ESP_OK; } @@ -716,6 +718,7 @@ esp_err_t i2c_filter_enable(i2c_port_t i2c_num, uint8_t cyc_num) I2C_CHECK(p_i2c_obj[i2c_num] != NULL, I2C_DRIVER_ERR_STR, ESP_FAIL); I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); i2c_hal_set_filter(&(i2c_context[i2c_num].hal), cyc_num); + i2c_hal_update_config(&(i2c_context[i2c_num].hal)); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); return ESP_OK; } @@ -725,6 +728,7 @@ esp_err_t i2c_filter_disable(i2c_port_t i2c_num) I2C_CHECK(i2c_num < I2C_NUM_MAX, I2C_NUM_ERROR_STR, ESP_ERR_INVALID_ARG); I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); i2c_hal_set_filter(&(i2c_context[i2c_num].hal), 0); + i2c_hal_update_config(&(i2c_context[i2c_num].hal)); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); return ESP_OK; } @@ -737,6 +741,7 @@ esp_err_t i2c_set_start_timing(i2c_port_t i2c_num, int setup_time, int hold_time I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); i2c_hal_set_start_timing(&(i2c_context[i2c_num].hal), setup_time, hold_time); + i2c_hal_update_config(&(i2c_context[i2c_num].hal)); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); return ESP_OK; } @@ -758,6 +763,7 @@ esp_err_t i2c_set_stop_timing(i2c_port_t i2c_num, int setup_time, int hold_time) I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); i2c_hal_set_stop_timing(&(i2c_context[i2c_num].hal), setup_time, hold_time); + i2c_hal_update_config(&(i2c_context[i2c_num].hal)); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); return ESP_OK; } @@ -779,6 +785,7 @@ esp_err_t i2c_set_data_timing(i2c_port_t i2c_num, int sample_time, int hold_time I2C_ENTER_CRITICAL(&(i2c_context[i2c_num].spinlock)); i2c_hal_set_sda_timing(&(i2c_context[i2c_num].hal), sample_time, hold_time); + i2c_hal_update_config(&(i2c_context[i2c_num].hal)); I2C_EXIT_CRITICAL(&(i2c_context[i2c_num].spinlock)); return ESP_OK; } diff --git a/components/hal/esp32c3/include/hal/i2c_ll.h b/components/hal/esp32c3/include/hal/i2c_ll.h index af1ef16e2d..7aa035676f 100644 --- a/components/hal/esp32c3/include/hal/i2c_ll.h +++ b/components/hal/esp32c3/include/hal/i2c_ll.h @@ -801,9 +801,12 @@ static inline void i2c_ll_master_fsm_rst(i2c_dev_t *hw) static inline void i2c_ll_master_clr_bus(i2c_dev_t *hw) { hw->scl_sp_conf.scl_rst_slv_num = 9; - hw->scl_sp_conf.scl_rst_slv_en = 0; - hw->ctr.conf_upgate = 1; hw->scl_sp_conf.scl_rst_slv_en = 1; + hw->ctr.conf_upgate = 1; + // hardward will clear scl_rst_slv_en after sending SCL pulses, + // and we should set conf_upgate bit to synchronize register value. + while (hw->scl_sp_conf.scl_rst_slv_en); + hw->ctr.conf_upgate = 1; } /** From 4d7f0076456904443158156368a87ba5251aa37a Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Sat, 7 May 2022 12:31:36 +0800 Subject: [PATCH 3/3] I2C: Fix a bug making the I2C task wait too long on an event The I2C ISR will now notify the task waiting on an event. Thus, the task can stop waiting on the event queue as soon as possible. --- components/driver/i2c.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/components/driver/i2c.c b/components/driver/i2c.c index 0fa119d934..812c5e888f 100644 --- a/components/driver/i2c.c +++ b/components/driver/i2c.c @@ -199,7 +199,7 @@ static i2c_clk_alloc_t i2c_clk_alloc[I2C_SCLK_MAX] = { static i2c_obj_t *p_i2c_obj[I2C_NUM_MAX] = {0}; static void i2c_isr_handler_default(void *arg); -static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num); +static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num, portBASE_TYPE* HPTaskAwoken); static esp_err_t i2c_hw_fsm_reset(i2c_port_t i2c_num); static void i2c_hw_disable(i2c_port_t i2c_num) @@ -478,6 +478,7 @@ static void IRAM_ATTR i2c_isr_handler_default(void *arg) } i2c_intr_event_t evt_type = I2C_INTR_EVENT_ERR; portBASE_TYPE HPTaskAwoken = pdFALSE; + portBASE_TYPE HPTaskAwokenCallee = pdFALSE; if (p_i2c->mode == I2C_MODE_MASTER) { if (p_i2c->status == I2C_STATUS_WRITE) { i2c_hal_master_handle_tx_event(&(i2c_context[i2c_num].hal), &evt_type); @@ -486,18 +487,18 @@ static void IRAM_ATTR i2c_isr_handler_default(void *arg) } if (evt_type == I2C_INTR_EVENT_NACK) { p_i2c_obj[i2c_num]->status = I2C_STATUS_ACK_ERROR; - i2c_master_cmd_begin_static(i2c_num); + i2c_master_cmd_begin_static(i2c_num, &HPTaskAwokenCallee); } else if (evt_type == I2C_INTR_EVENT_TOUT) { p_i2c_obj[i2c_num]->status = I2C_STATUS_TIMEOUT; - i2c_master_cmd_begin_static(i2c_num); + i2c_master_cmd_begin_static(i2c_num, &HPTaskAwokenCallee); } else if (evt_type == I2C_INTR_EVENT_ARBIT_LOST) { p_i2c_obj[i2c_num]->status = I2C_STATUS_TIMEOUT; - i2c_master_cmd_begin_static(i2c_num); + i2c_master_cmd_begin_static(i2c_num, &HPTaskAwokenCallee); } else if (evt_type == I2C_INTR_EVENT_END_DET) { - i2c_master_cmd_begin_static(i2c_num); + i2c_master_cmd_begin_static(i2c_num, &HPTaskAwokenCallee); } else if (evt_type == I2C_INTR_EVENT_TRANS_DONE) { if (p_i2c->status != I2C_STATUS_ACK_ERROR && p_i2c->status != I2C_STATUS_IDLE) { - i2c_master_cmd_begin_static(i2c_num); + i2c_master_cmd_begin_static(i2c_num, &HPTaskAwokenCallee); } } else { // Do nothing if there is no proper event. @@ -530,7 +531,7 @@ static void IRAM_ATTR i2c_isr_handler_default(void *arg) } } //We only need to check here if there is a high-priority task needs to be switched. - if (HPTaskAwoken == pdTRUE) { + if (HPTaskAwoken == pdTRUE || HPTaskAwokenCallee == pdTRUE) { portYIELD_FROM_ISR(); } } @@ -1057,11 +1058,10 @@ esp_err_t i2c_master_read(i2c_cmd_handle_t cmd_handle, uint8_t *data, size_t dat } } -static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num) +static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num, portBASE_TYPE* HPTaskAwoken) { i2c_obj_t *p_i2c = p_i2c_obj[i2c_num]; - portBASE_TYPE HPTaskAwoken = pdFALSE; - i2c_cmd_evt_t evt; + i2c_cmd_evt_t evt = { 0 }; if (p_i2c->cmd_link.head != NULL && p_i2c->status == I2C_STATUS_READ) { i2c_cmd_t *cmd = &p_i2c->cmd_link.head->cmd; i2c_hal_read_rxfifo(&(i2c_context[i2c_num].hal), cmd->data, p_i2c->rx_cnt); @@ -1073,17 +1073,19 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num) } } else if ((p_i2c->status == I2C_STATUS_ACK_ERROR) || (p_i2c->status == I2C_STATUS_TIMEOUT)) { + assert(HPTaskAwoken != NULL); evt.type = I2C_CMD_EVT_DONE; - xQueueOverwriteFromISR(p_i2c->cmd_evt_queue, &evt, &HPTaskAwoken); + xQueueOverwriteFromISR(p_i2c->cmd_evt_queue, &evt, HPTaskAwoken); return; } else if (p_i2c->status == I2C_STATUS_DONE) { return; } if (p_i2c->cmd_link.head == NULL) { + assert(HPTaskAwoken != NULL); p_i2c->cmd_link.cur = NULL; evt.type = I2C_CMD_EVT_DONE; - xQueueOverwriteFromISR(p_i2c->cmd_evt_queue, &evt, &HPTaskAwoken); + xQueueOverwriteFromISR(p_i2c->cmd_evt_queue, &evt, HPTaskAwoken); // Return to the IDLE status after cmd_eve_done signal were send out. p_i2c->status = I2C_STATUS_IDLE; return; @@ -1212,7 +1214,7 @@ esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle, i2c_hal_disable_intr_mask(&(i2c_context[i2c_num].hal), I2C_LL_INTR_MASK); i2c_hal_clr_intsts_mask(&(i2c_context[i2c_num].hal), I2C_LL_INTR_MASK); //start send commands, at most 32 bytes one time, isr handler will process the remaining commands. - i2c_master_cmd_begin_static(i2c_num); + i2c_master_cmd_begin_static(i2c_num, NULL); // Wait event bits i2c_cmd_evt_t evt;