From 3ac6c10d11415d27407e6e54010fc5372efd4495 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Tue, 12 Mar 2024 12:07:07 +0800 Subject: [PATCH] fix(i2c_slave): Fix bugs on i2c slave, 1. Fixed read data number smaller than master has sent it will fail 2. Disable interrupt when destroy bus Closes https://github.com/espressif/esp-idf/issues/13354 --- components/driver/i2c/i2c_slave.c | 13 +-- .../i2c_test_apps/main/test_i2c_multi.c | 83 +++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/components/driver/i2c/i2c_slave.c b/components/driver/i2c/i2c_slave.c index e3bae30bc4..88ea7af99a 100644 --- a/components/driver/i2c/i2c_slave.c +++ b/components/driver/i2c/i2c_slave.c @@ -1,10 +1,11 @@ /* - * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ #include +#include #include "esp_types.h" #include "esp_intr_alloc.h" #include "freertos/FreeRTOS.h" @@ -59,10 +60,11 @@ static IRAM_ATTR void s_i2c_handle_rx_fifo_wm(i2c_slave_dev_handle_t i2c_slave, i2c_hal_context_t *hal = &i2c_slave->base->hal; uint32_t rx_fifo_cnt; i2c_ll_get_rxfifo_cnt(hal->dev, &rx_fifo_cnt); - i2c_ll_read_rxfifo(hal->dev, i2c_slave->data_buf, rx_fifo_cnt); - memcpy(t->buffer + i2c_slave->already_receive_len, i2c_slave->data_buf, rx_fifo_cnt); - i2c_slave->already_receive_len += rx_fifo_cnt; - t->rcv_fifo_cnt -= rx_fifo_cnt; + uint32_t fifo_cnt_rd = MIN(t->rcv_fifo_cnt, rx_fifo_cnt); + i2c_ll_read_rxfifo(hal->dev, i2c_slave->data_buf, fifo_cnt_rd); + memcpy(t->buffer + i2c_slave->already_receive_len, i2c_slave->data_buf, fifo_cnt_rd); + i2c_slave->already_receive_len += fifo_cnt_rd; + t->rcv_fifo_cnt -= fifo_cnt_rd; } static IRAM_ATTR void s_i2c_handle_complete(i2c_slave_dev_handle_t i2c_slave, i2c_slave_receive_t *t, BaseType_t *do_yield) @@ -280,6 +282,7 @@ err: static esp_err_t i2c_slave_bus_destroy(i2c_slave_dev_handle_t i2c_slave) { if (i2c_slave) { + i2c_ll_disable_intr_mask(i2c_slave->base->hal.dev, I2C_LL_SLAVE_EVENT_INTR); if (i2c_slave->slv_rx_mux) { vSemaphoreDeleteWithCaps(i2c_slave->slv_rx_mux); i2c_slave->slv_rx_mux = NULL; diff --git a/components/driver/test_apps/i2c_test_apps/main/test_i2c_multi.c b/components/driver/test_apps/i2c_test_apps/main/test_i2c_multi.c index be4c97cddf..87e206cee9 100644 --- a/components/driver/test_apps/i2c_test_apps/main/test_i2c_multi.c +++ b/components/driver/test_apps/i2c_test_apps/main/test_i2c_multi.c @@ -139,6 +139,89 @@ static void i2c_slave_read_test(void) TEST_CASE_MULTIPLE_DEVICES("I2C master write slave test", "[i2c][test_env=generic_multi_device][timeout=150]", i2c_master_write_test, i2c_slave_read_test); +static void i2c_master_write_test_large_write_small_read(void) +{ + uint8_t data_wr[DATA_LENGTH] = { 0 }; + int i; + + i2c_master_bus_config_t i2c_mst_config = { + .clk_source = I2C_CLK_SRC_DEFAULT, + .i2c_port = TEST_I2C_PORT, + .scl_io_num = I2C_MASTER_SCL_IO, + .sda_io_num = I2C_MASTER_SDA_IO, + .flags.enable_internal_pullup = true, + }; + i2c_master_bus_handle_t bus_handle; + + TEST_ESP_OK(i2c_new_master_bus(&i2c_mst_config, &bus_handle)); + + i2c_device_config_t dev_cfg = { + .dev_addr_length = I2C_ADDR_BIT_LEN_7, + .device_address = 0x58, + .scl_speed_hz = 100000, + }; + + i2c_master_dev_handle_t dev_handle; + TEST_ESP_OK(i2c_master_bus_add_device(bus_handle, &dev_cfg, &dev_handle)); + + unity_wait_for_signal("i2c slave init finish"); + + unity_send_signal("master write"); + for (i = 0; i < DATA_LENGTH; i++) { + data_wr[i] = i; + } + + disp_buf(data_wr, i); + TEST_ESP_OK(i2c_master_transmit(dev_handle, data_wr, DATA_LENGTH, -1)); + unity_wait_for_signal("ready to delete"); + TEST_ESP_OK(i2c_master_bus_rm_device(dev_handle)); + + TEST_ESP_OK(i2c_del_master_bus(bus_handle)); +} + +static void i2c_slave_read_test_large_write_small_read(void) +{ + uint8_t data_rd[7] = {0}; + + i2c_slave_config_t i2c_slv_config = { + .addr_bit_len = I2C_ADDR_BIT_LEN_7, + .clk_source = I2C_CLK_SRC_DEFAULT, + .i2c_port = TEST_I2C_PORT, + .send_buf_depth = 256, + .scl_io_num = I2C_SLAVE_SCL_IO, + .sda_io_num = I2C_SLAVE_SDA_IO, + .slave_addr = 0x58, + }; + + i2c_slave_dev_handle_t slave_handle; + TEST_ESP_OK(i2c_new_slave_device(&i2c_slv_config, &slave_handle)); + + s_receive_queue = xQueueCreate(1, sizeof(i2c_slave_rx_done_event_data_t)); + i2c_slave_event_callbacks_t cbs = { + .on_recv_done = test_i2c_rx_done_callback, + }; + ESP_ERROR_CHECK(i2c_slave_register_event_callbacks(slave_handle, &cbs, s_receive_queue)); + + i2c_slave_rx_done_event_data_t rx_data; + TEST_ESP_OK(i2c_slave_receive(slave_handle, data_rd, 7)); + + unity_send_signal("i2c slave init finish"); + + unity_wait_for_signal("master write"); + xQueueReceive(s_receive_queue, &rx_data, pdMS_TO_TICKS(10000)); + disp_buf(data_rd, 7); + for (int i = 0; i < 7; i++) { + TEST_ASSERT(data_rd[i] == i); + } + for (int i = 0; i < 7; i++) { + TEST_ASSERT(rx_data.buffer[i] == i); + } + vQueueDelete(s_receive_queue); + unity_send_signal("ready to delete"); + TEST_ESP_OK(i2c_del_slave_device(slave_handle)); +} + +TEST_CASE_MULTIPLE_DEVICES("I2C master write slave test (large write small read)", "[i2c][test_env=generic_multi_device][timeout=150]", i2c_master_write_test_large_write_small_read, i2c_slave_read_test_large_write_small_read); static void master_read_slave_test(void) {