From f0a82d71850648babb6271a69f822ae9b1b5ce8b Mon Sep 17 00:00:00 2001
From: Ivan Grokhotkov <ivan@espressif.com>
Date: Mon, 18 Mar 2019 18:20:34 +0800
Subject: [PATCH] driver/i2c: write i2c command structure to hardware in one
 operation

GCC compiler can generate 8-bit stores when modifying bitfields of
volatile structs (https://github.com/espressif/esp-idf/issues/597).
In the specific case of I2C driver, this resulted in byte_num field
to be written using s8i. However the peripheral requires 32-bit
writes, and ignores 8-bit writes. This change modifies the code to
compose the 32-bit command register value first, and then write the
complete value to the hardware.
---
 components/driver/i2c.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/components/driver/i2c.c b/components/driver/i2c.c
index ad67b1639..447f788c4 100644
--- a/components/driver/i2c.c
+++ b/components/driver/i2c.c
@@ -90,8 +90,10 @@ typedef struct {
     uint8_t ack_val;   /*!< ack value to send */
     uint8_t* data;     /*!< data address */
     uint8_t byte_cmd;  /*!< to save cmd for one byte command mode */
-    i2c_opmode_t op_code; /*!< haredware cmd type */
-}i2c_cmd_t;
+    i2c_opmode_t op_code; /*!< hardware cmd type */
+} i2c_cmd_t;
+
+typedef typeof(I2C[0]->command[0]) i2c_hw_cmd_t;
 
 typedef struct i2c_cmd_link{
     i2c_cmd_t cmd;              /*!< command in current cmd link */
@@ -1160,12 +1162,17 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
     }
     while (p_i2c->cmd_link.head) {
         i2c_cmd_t *cmd = &p_i2c->cmd_link.head->cmd;
-        I2C[i2c_num]->command[p_i2c->cmd_idx].val = 0;
-        I2C[i2c_num]->command[p_i2c->cmd_idx].ack_en = cmd->ack_en;
-        I2C[i2c_num]->command[p_i2c->cmd_idx].ack_exp = cmd->ack_exp;
-        I2C[i2c_num]->command[p_i2c->cmd_idx].ack_val = cmd->ack_val;
-        I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num = cmd->byte_num;
-        I2C[i2c_num]->command[p_i2c->cmd_idx].op_code = cmd->op_code;
+        i2c_hw_cmd_t * const p_cur_hw_cmd = &I2C[i2c_num]->command[p_i2c->cmd_idx];
+        i2c_hw_cmd_t hw_cmd = {
+            .ack_en = cmd->ack_en,
+            .ack_exp = cmd->ack_exp,
+            .ack_val = cmd->ack_val,
+            .byte_num = cmd->byte_num,
+            .op_code = cmd->op_code
+        };
+        const i2c_hw_cmd_t hw_end_cmd = {
+            .op_code = I2C_CMD_END
+        };
         if (cmd->op_code == I2C_CMD_WRITE) {
             uint32_t wr_filled = 0;
             //TODO: to reduce interrupt number
@@ -1182,10 +1189,9 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
                 cmd->byte_num--;
                 wr_filled ++;
             }
-            //Workaround for register field operation.
-            I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num = wr_filled;
-            I2C[i2c_num]->command[p_i2c->cmd_idx + 1].val = 0;
-            I2C[i2c_num]->command[p_i2c->cmd_idx + 1].op_code = I2C_CMD_END;
+            hw_cmd.byte_num = wr_filled;
+            *p_cur_hw_cmd = hw_cmd;
+            *(p_cur_hw_cmd + 1) = hw_end_cmd;
             p_i2c->tx_fifo_remain = I2C_FIFO_LEN;
             p_i2c->cmd_idx = 0;
             if (cmd->byte_num > 0) {
@@ -1198,13 +1204,14 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
             //TODO: to reduce interrupt number
             p_i2c->rx_cnt = cmd->byte_num > p_i2c->rx_fifo_remain ? p_i2c->rx_fifo_remain : cmd->byte_num;
             cmd->byte_num -= p_i2c->rx_cnt;
-            I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num = p_i2c->rx_cnt;
-            I2C[i2c_num]->command[p_i2c->cmd_idx].ack_val = cmd->ack_val;
-            I2C[i2c_num]->command[p_i2c->cmd_idx + 1].val = 0;
-            I2C[i2c_num]->command[p_i2c->cmd_idx + 1].op_code = I2C_CMD_END;
+            hw_cmd.byte_num = p_i2c->rx_cnt;
+            hw_cmd.ack_val = cmd->ack_val;
+            *p_cur_hw_cmd = hw_cmd;
+            *(p_cur_hw_cmd + 1) = hw_end_cmd;
             p_i2c->status = I2C_STATUS_READ;
             break;
         } else {
+            *p_cur_hw_cmd = hw_cmd;
         }
         p_i2c->cmd_idx++;
         p_i2c->cmd_link.head = p_i2c->cmd_link.head->next;