bugfix(i2c): use queue instead of event group for internal commands

Reported from github:
https://github.com/espressif/esp-idf/issues/1312
https://github.com/espressif/esp-idf/issues/1193

Issues:
1. We used to use event group in the driver, which would cause:
    a. longer operation time since the event group are based on FreeRTOS timer.
    b. Operation fails if the timer queue is not long enough.
2. There might be some issue with event group, we will still try to provide a small test code in other branch.

modification:
1. use queue instead of event-bit for internal commands
2. use queue overwrite for cmd_done event

diff --git a/components/driver/i2c.c b/components/driver/i2c.c
index c83570496e7cfc0a4b8144113fb4835aaadb8102..6c9d4fed1d6b55bb21724cb6e89c5e45afba86ed 100644 (file)
--- a/components/driver/i2c.c
+++ b/components/driver/i2c.c
@@ -23,7 +23,6 @@
 #include "freertos/xtensa_api.h"\r
 #include "freertos/task.h"\r
 #include "freertos/ringbuf.h"\r
-#include "freertos/event_groups.h"\r
 #include "soc/dport_reg.h"\r
 #include "soc/i2c_struct.h"\r
 #include "soc/i2c_reg.h"\r
@@ -70,8 +69,9 @@ static DRAM_ATTR i2c_dev_t* const I2C[I2C_NUM_MAX] = { &I2C0, &I2C1 };
 #define I2C_FIFO_EMPTY_THRESH_VAL      (5)\r
 #define I2C_IO_INIT_LEVEL              (1)\r
 #define I2C_CMD_ALIVE_INTERVAL_TICK    (1000 / portTICK_PERIOD_MS)\r
-#define I2C_CMD_EVT_ALIVE              (BIT0)\r
-#define I2C_CMD_EVT_DONE               (BIT1)\r
+#define I2C_CMD_EVT_ALIVE              (0)\r
+#define I2C_CMD_EVT_DONE               (1)\r
+#define I2C_EVT_QUEUE_LEN              (1)\r
 #define I2C_SLAVE_TIMEOUT_DEFAULT      (32000)     /* I2C slave timeout value, APB clock cycle number */\r
 #define I2C_SLAVE_SDA_SAMPLE_DEFAULT   (10)        /* I2C slave sample time after scl positive edge default value */\r
 #define I2C_SLAVE_SDA_HOLD_DEFAULT     (10)        /* I2C slave hold time after scl negative edge default value */\r
@@ -107,6 +107,10 @@ typedef enum {
     I2C_STATUS_TIMEOUT,   /*!< I2C bus status error, and operation timeout */\r
 } i2c_status_t;\r
 \r
+typedef struct {\r
+    int type;\r
+} i2c_cmd_evt_t;\r
+\r
 typedef struct {\r
     int i2c_num;                     /*!< I2C port number */\r
     int mode;                        /*!< I2C mode, master or slave */\r
@@ -117,7 +121,7 @@ typedef struct {
     uint8_t data_buf[I2C_FIFO_LEN];  /*!< a buffer to store i2c fifo data */\r
 \r
     i2c_cmd_desc_t cmd_link;         /*!< I2C command link */\r
-    EventGroupHandle_t cmd_evt;      /*!< I2C command event bits */\r
+    QueueHandle_t cmd_evt_queue;     /*!< I2C command event queue */\r
     xSemaphoreHandle cmd_mux;        /*!< semaphore to lock command process */\r
     size_t tx_fifo_remain;           /*!< tx fifo remain length, for master mode */\r
     size_t rx_fifo_remain;           /*!< rx fifo remain length, for master mode */\r
@@ -199,8 +203,8 @@ esp_err_t i2c_driver_install(i2c_port_t i2c_num, i2c_mode_t mode, size_t slv_rx_
         } else {\r
             //semaphore to sync sending process, because we only have 32 bytes for hardware fifo.\r
             p_i2c->cmd_mux = xSemaphoreCreateMutex();\r
-            p_i2c->cmd_evt = xEventGroupCreate();\r
-            if (p_i2c->cmd_mux == NULL || p_i2c->cmd_evt == NULL) {\r
+            p_i2c->cmd_evt_queue = xQueueCreate(I2C_EVT_QUEUE_LEN, sizeof(i2c_cmd_evt_t));\r
+            if (p_i2c->cmd_mux == NULL || p_i2c->cmd_evt_queue == NULL) {\r
                 ESP_LOGE(I2C_TAG, I2C_SEM_ERR_STR);\r
                 goto err;\r
             }\r
@@ -243,9 +247,9 @@ esp_err_t i2c_driver_install(i2c_port_t i2c_num, i2c_mode_t mode, size_t slv_rx_
             p_i2c_obj[i2c_num]->tx_ring_buf = NULL;\r
             p_i2c_obj[i2c_num]->tx_buf_length = 0;\r
         }\r
-        if (p_i2c_obj[i2c_num]->cmd_evt) {\r
-            vEventGroupDelete(p_i2c_obj[i2c_num]->cmd_evt);\r
-            p_i2c_obj[i2c_num]->cmd_evt = NULL;\r
+        if (p_i2c_obj[i2c_num]->cmd_evt_queue) {\r
+            vQueueDelete(p_i2c_obj[i2c_num]->cmd_evt_queue);\r
+            p_i2c_obj[i2c_num]->cmd_evt_queue = NULL;\r
         }\r
         if (p_i2c_obj[i2c_num]->cmd_mux) {\r
             vSemaphoreDelete(p_i2c_obj[i2c_num]->cmd_mux);\r
@@ -296,9 +300,9 @@ esp_err_t i2c_driver_delete(i2c_port_t i2c_num)
         xSemaphoreTake(p_i2c->cmd_mux, portMAX_DELAY);\r
         vSemaphoreDelete(p_i2c->cmd_mux);\r
     }\r
-    if (p_i2c_obj[i2c_num]->cmd_evt) {\r
-        vEventGroupDelete(p_i2c_obj[i2c_num]->cmd_evt);\r
-        p_i2c_obj[i2c_num]->cmd_evt = NULL;\r
+    if (p_i2c_obj[i2c_num]->cmd_evt_queue) {\r
+        vQueueDelete(p_i2c_obj[i2c_num]->cmd_evt_queue);\r
+        p_i2c_obj[i2c_num]->cmd_evt_queue = NULL;\r
     }\r
     if (p_i2c->slv_rx_mux) {\r
         vSemaphoreDelete(p_i2c->slv_rx_mux);\r
@@ -437,7 +441,9 @@ static void IRAM_ATTR i2c_isr_handler_default(void* arg)
         }\r
     }\r
     if (p_i2c->mode == I2C_MODE_MASTER) {\r
-        xEventGroupSetBitsFromISR(p_i2c->cmd_evt, I2C_CMD_EVT_ALIVE, &HPTaskAwoken);\r
+        i2c_cmd_evt_t evt;\r
+        evt.type = I2C_CMD_EVT_ALIVE;\r
+        xQueueSendFromISR(p_i2c->cmd_evt_queue, &evt, &HPTaskAwoken);\r
         if (HPTaskAwoken == pdTRUE) {\r
             portYIELD_FROM_ISR();\r
         }\r
@@ -990,7 +996,7 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
 {\r
     i2c_obj_t* p_i2c = p_i2c_obj[i2c_num];\r
     portBASE_TYPE HPTaskAwoken = pdFALSE;\r
-\r
+    i2c_cmd_evt_t evt;\r
     //This should never happen\r
     if (p_i2c->mode == I2C_MODE_SLAVE) {\r
         return;\r
@@ -1005,7 +1011,8 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
             I2C[i2c_num]->int_clr.time_out = 1;\r
             I2C[i2c_num]->int_ena.val = 0;\r
         }\r
-        xEventGroupSetBitsFromISR(p_i2c->cmd_evt, I2C_CMD_EVT_DONE, &HPTaskAwoken);\r
+        evt.type = I2C_CMD_EVT_DONE;\r
+        xQueueOverwriteFromISR(p_i2c->cmd_evt_queue, &evt, &HPTaskAwoken);\r
         if (HPTaskAwoken == pdTRUE) {\r
             portYIELD_FROM_ISR();\r
         }\r
@@ -1024,7 +1031,8 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
     }\r
     if (p_i2c->cmd_link.head == NULL) {\r
         p_i2c->cmd_link.cur = NULL;\r
-        xEventGroupSetBitsFromISR(p_i2c->cmd_evt, I2C_CMD_EVT_DONE, &HPTaskAwoken);\r
+        evt.type = I2C_CMD_EVT_DONE;\r
+        xQueueOverwriteFromISR(p_i2c->cmd_evt_queue, &evt, &HPTaskAwoken);\r
         if (HPTaskAwoken == pdTRUE) {\r
             portYIELD_FROM_ISR();\r
         }\r
@@ -1108,7 +1116,7 @@ esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle,
     if (res == pdFALSE) {\r
         return ESP_ERR_TIMEOUT;\r
     }\r
-    xEventGroupClearBits(p_i2c->cmd_evt, I2C_CMD_EVT_DONE | I2C_CMD_EVT_ALIVE);\r
+    xQueueReset(p_i2c->cmd_evt_queue);\r
     if (p_i2c->status == I2C_STATUS_TIMEOUT\r
         || I2C[i2c_num]->status_reg.bus_busy == 1) {\r
         i2c_hw_fsm_reset(i2c_num);\r
@@ -1137,16 +1145,15 @@ esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle,
         ticks_to_wait = ticks_end - xTaskGetTickCount();\r
     }\r
     // Wait event bits\r
-    EventBits_t uxBits;\r
+    i2c_cmd_evt_t evt;\r
     while (1) {\r
         TickType_t wait_time = (ticks_to_wait < (I2C_CMD_ALIVE_INTERVAL_TICK) ? ticks_to_wait : (I2C_CMD_ALIVE_INTERVAL_TICK));\r
         // In master mode, since we don't have an interrupt to detective bus error or FSM state, what we do here is to make\r
         // sure the interrupt mechanism for master mode is still working.\r
         // If the command sending is not finished and there is no interrupt any more, the bus is probably dead caused by external noise.\r
-        uxBits = xEventGroupWaitBits(p_i2c->cmd_evt, I2C_CMD_EVT_ALIVE | I2C_CMD_EVT_DONE, false, false, wait_time);\r
-        if (uxBits) {\r
-            if (uxBits & I2C_CMD_EVT_DONE) {\r
-                xEventGroupClearBits(p_i2c->cmd_evt, I2C_CMD_EVT_DONE);\r
+        portBASE_TYPE evt_res = xQueueReceive(p_i2c->cmd_evt_queue, &evt, wait_time);\r
+        if (evt_res == pdTRUE) {\r
+            if (evt.type == I2C_CMD_EVT_DONE) {\r
                 if (p_i2c->status == I2C_STATUS_TIMEOUT) {\r
                     // If the I2C slave are powered off or the SDA/SCL are connected to ground, for example,\r
                     // I2C hw FSM would get stuck in wrong state, we have to reset the I2C module in this case.\r
@@ -1159,8 +1166,7 @@ esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle,
                 }\r
                 break;\r
             }\r
-            if (uxBits & I2C_CMD_EVT_ALIVE) {\r
-                xEventGroupClearBits(p_i2c->cmd_evt, I2C_CMD_EVT_ALIVE);\r
+            if (evt.type == I2C_CMD_EVT_ALIVE) {\r
             }\r
         } else {\r
             ret = ESP_ERR_TIMEOUT;\r