i2c: rx <-> tx typo's, NULLing free'd variable, consistent CRITICAL sects

A couple of typos referencing tx_ring_buf when rx_ring_buf, slv_tx_mux
instead of slv_rx_mux.

Also, I2C_ENTER_CRITICAL()/I2C_EXIT_CRITICAL() usage was not consistent.
Only some of the _set_ functions had them.  Most of the _get_ function
had them?  It is my understanding that they should be wrapped around
writes, not reads? (I think we still need the lock for reading pairs of consistent values)

Also, the ticks_to_wait timeout handling in i2c_master_cmd_begin() would
not handle integer rollover correctly.

Merges https://github.com/espressif/esp-idf/pull/1180

diff --git a/components/driver/i2c.c b/components/driver/i2c.c
index 2028b8b357d2232de7a114d7f1db62fca21d3845..57ae7547d25e58192611cd202443f78d108a0094 100644 (file)
--- a/components/driver/i2c.c
+++ b/components/driver/i2c.c
@@ -52,7 +52,7 @@ static DRAM_ATTR i2c_dev_t* const I2C[I2C_NUM_MAX] = { &I2C0, &I2C1 };
 #define I2C_TIMEING_VAL_ERR_STR        "i2c timing value error"\r
 #define I2C_ADDR_ERROR_STR             "i2c null address error"\r
 #define I2C_DRIVER_NOT_INSTALL_ERR_STR "i2c driver not installed"\r
-#define I2C_SLAVE_BUFFER_LEN_ERR_STR   "i2c buffer size too short for slave mode"\r
+#define I2C_SLAVE_BUFFER_LEN_ERR_STR   "i2c buffer size too small for slave mode"\r
 #define I2C_EVT_QUEUE_ERR_STR          "i2c evt queue error"\r
 #define I2C_SEM_ERR_STR                "i2c semaphore error"\r
 #define I2C_BUF_ERR_STR                "i2c ringbuffer error"\r
@@ -64,7 +64,7 @@ static DRAM_ATTR i2c_dev_t* const I2C[I2C_NUM_MAX] = { &I2C0, &I2C1 };
 #define I2C_SDA_IO_ERR_STR             "sda gpio number error"\r
 #define I2C_SCL_IO_ERR_STR             "scl gpio number error"\r
 #define I2C_CMD_LINK_INIT_ERR_STR      "i2c command link error"\r
-#define I2C_GPIO_PULLUP_ERR_STR        "this i2c pin do not support internal pull-up"\r
+#define I2C_GPIO_PULLUP_ERR_STR        "this i2c pin does not support internal pull-up"\r
 #define I2C_FIFO_FULL_THRESH_VAL       (28)\r
 #define I2C_FIFO_EMPTY_THRESH_VAL      (5)\r
 #define I2C_IO_INIT_LEVEL              (1)\r
@@ -179,7 +179,7 @@ esp_err_t i2c_driver_install(i2c_port_t i2c_num, i2c_mode_t mode, size_t slv_rx_
                 }\r
                 p_i2c->rx_buf_length = slv_rx_buf_len;\r
             } else {\r
-                p_i2c->tx_ring_buf = NULL;\r
+                p_i2c->rx_ring_buf = NULL;\r
                 p_i2c->rx_buf_length = 0;\r
             }\r
             if (slv_tx_buf_len > 0) {\r
@@ -195,7 +195,7 @@ esp_err_t i2c_driver_install(i2c_port_t i2c_num, i2c_mode_t mode, size_t slv_rx_
             }\r
             p_i2c->slv_rx_mux = xSemaphoreCreateMutex();\r
             p_i2c->slv_tx_mux = xSemaphoreCreateMutex();\r
-            if (p_i2c->slv_rx_mux == NULL || p_i2c->slv_rx_mux == NULL) {\r
+            if (p_i2c->slv_rx_mux == NULL || p_i2c->slv_tx_mux == NULL) {\r
                 ESP_LOGE(I2C_TAG, I2C_SEM_ERR_STR);\r
                 goto err;\r
             }\r
@@ -262,6 +262,7 @@ esp_err_t i2c_driver_install(i2c_port_t i2c_num, i2c_mode_t mode, size_t slv_rx_
         }\r
     }\r
     free(p_i2c_obj[i2c_num]);\r
+    p_i2c_obj[i2c_num] = NULL;\r
     return ESP_FAIL;\r
 }\r
 \r
@@ -657,8 +658,10 @@ esp_err_t i2c_set_start_timing(i2c_port_t i2c_num, int setup_time, int hold_time
     I2C_CHECK((hold_time <= I2C_SCL_START_HOLD_TIME_V) && (hold_time > 0), I2C_TIMEING_VAL_ERR_STR, ESP_ERR_INVALID_ARG);\r
     I2C_CHECK((setup_time <= I2C_SCL_RSTART_SETUP_TIME_V) && (setup_time > 0), I2C_TIMEING_VAL_ERR_STR, ESP_ERR_INVALID_ARG);\r
 \r
+    I2C_ENTER_CRITICAL(&i2c_spinlock[i2c_num]);\r
     I2C[i2c_num]->scl_start_hold.time = hold_time;\r
     I2C[i2c_num]->scl_rstart_setup.time = setup_time;\r
+    I2C_EXIT_CRITICAL(&i2c_spinlock[i2c_num]);\r
     return ESP_OK;\r
 }\r
 \r
@@ -682,8 +685,10 @@ esp_err_t i2c_set_stop_timing(i2c_port_t i2c_num, int setup_time, int hold_time)
     I2C_CHECK((setup_time <= I2C_SCL_STOP_SETUP_TIME_V) && (setup_time > 0), I2C_TIMEING_VAL_ERR_STR, ESP_ERR_INVALID_ARG);\r
     I2C_CHECK((hold_time <= I2C_SCL_STOP_HOLD_TIME_V) && (hold_time > 0), I2C_TIMEING_VAL_ERR_STR, ESP_ERR_INVALID_ARG);\r
 \r
+    I2C_ENTER_CRITICAL(&i2c_spinlock[i2c_num]);\r
     I2C[i2c_num]->scl_stop_hold.time = hold_time;\r
     I2C[i2c_num]->scl_stop_setup.time = setup_time;\r
+    I2C_EXIT_CRITICAL(&i2c_spinlock[i2c_num]);\r
     return ESP_OK;\r
 }\r
 \r
@@ -707,8 +712,10 @@ esp_err_t i2c_set_data_timing(i2c_port_t i2c_num, int sample_time, int hold_time
     I2C_CHECK((sample_time <= I2C_SDA_SAMPLE_TIME_V) && (sample_time > 0), I2C_TIMEING_VAL_ERR_STR, ESP_ERR_INVALID_ARG);\r
     I2C_CHECK((hold_time <= I2C_SDA_HOLD_TIME_V) && (hold_time > 0), I2C_TIMEING_VAL_ERR_STR, ESP_ERR_INVALID_ARG);\r
 \r
+    I2C_ENTER_CRITICAL(&i2c_spinlock[i2c_num]);\r
     I2C[i2c_num]->sda_hold.time = hold_time;\r
     I2C[i2c_num]->sda_sample.time = sample_time;\r
+    I2C_EXIT_CRITICAL(&i2c_spinlock[i2c_num]);\r
     return ESP_OK;\r
 }\r
 \r
@@ -729,7 +736,7 @@ esp_err_t i2c_get_data_timing(i2c_port_t i2c_num, int* sample_time, int* hold_ti
 esp_err_t i2c_set_timeout(i2c_port_t i2c_num, int timeout)\r
 {\r
     I2C_CHECK(i2c_num < I2C_NUM_MAX, I2C_NUM_ERROR_STR, ESP_ERR_INVALID_ARG);\r
-    I2C_CHECK((timeout <= I2C_TIME_OUT_REG_V) && (timeout > 0), I2C_TIMEING_VAL_ERR_STR, ESP_ERR_INVALID_ARG);
+    I2C_CHECK((timeout <= I2C_TIME_OUT_REG_V) && (timeout > 0), I2C_TIMEING_VAL_ERR_STR, ESP_ERR_INVALID_ARG);\r
 \r
     I2C_ENTER_CRITICAL(&i2c_spinlock[i2c_num]);\r
     I2C[i2c_num]->timeout.tout = timeout;\r
@@ -1111,7 +1118,7 @@ esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle,
 \r
     esp_err_t ret = ESP_FAIL;\r
     i2c_obj_t* p_i2c = p_i2c_obj[i2c_num];\r
-    portTickType ticks_end = xTaskGetTickCount() + ticks_to_wait;\r
+    portTickType ticks_start = xTaskGetTickCount();\r
     portBASE_TYPE res = xSemaphoreTake(p_i2c->cmd_mux, ticks_to_wait);\r
     if (res == pdFALSE) {\r
         return ESP_ERR_TIMEOUT;\r
@@ -1137,17 +1144,19 @@ esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle,
 \r
     //start send commands, at most 32 bytes one time, isr handler will process the remaining commands.\r
     i2c_master_cmd_begin_static(i2c_num);\r
-    if (ticks_to_wait == portMAX_DELAY) {\r
 \r
-    } else if (ticks_to_wait == 0) {\r
-\r
-    } else {\r
-        ticks_to_wait = ticks_end - xTaskGetTickCount();\r
-    }\r
     // Wait event bits\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
+        TickType_t wait_time = xTaskGetTickCount();\r
+        if (wait_time - ticks_start > ticks_to_wait) { // out of time\r
+            wait_time = I2C_CMD_ALIVE_INTERVAL_TICK;\r
+        } else {\r
+            wait_time = ticks_to_wait - (wait_time - ticks_start);\r
+            if (wait_time < I2C_CMD_ALIVE_INTERVAL_TICK) {\r
+                wait_time = I2C_CMD_ALIVE_INTERVAL_TICK;\r
+            }\r
+        }\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
@@ -1175,12 +1184,6 @@ esp_err_t i2c_master_cmd_begin(i2c_port_t i2c_num, i2c_cmd_handle_t cmd_handle,
             i2c_hw_fsm_reset(i2c_num);\r
             break;\r
         }\r
-        if (ticks_to_wait == portMAX_DELAY) {\r
-\r
-        } else {\r
-            TickType_t now = xTaskGetTickCount();\r
-            ticks_to_wait = ticks_end > now ? (ticks_end - now) : 0;\r
-        }\r
     }\r
     p_i2c->status = I2C_STATUS_DONE;\r
     xSemaphoreGive(p_i2c->cmd_mux);\r
@@ -1258,6 +1261,3 @@ int i2c_slave_read_buffer(i2c_port_t i2c_num, uint8_t* data, size_t max_size, Ti
     xSemaphoreGive(p_i2c->slv_rx_mux);\r
     return cnt;\r
 }\r
-\r
-\r
-\r