You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2019/01/08 13:11:13 UTC

[GitHub] andrzej-kaczmarek closed pull request #1589: hw/util/i2cn: Add write-read transaction support

andrzej-kaczmarek closed pull request #1589: hw/util/i2cn: Add write-read transaction support
URL: https://github.com/apache/mynewt-core/pull/1589
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/hw/drivers/sensors/lps33hw/src/lps33hw.c b/hw/drivers/sensors/lps33hw/src/lps33hw.c
index a55a7a6143..b54527b2b4 100644
--- a/hw/drivers/sensors/lps33hw/src/lps33hw.c
+++ b/hw/drivers/sensors/lps33hw/src/lps33hw.c
@@ -354,35 +354,27 @@ lps33hw_i2c_get_regs(struct sensor_itf *itf, uint8_t reg, uint8_t size,
     uint8_t *buffer)
 {
     int rc;
-
-    struct hal_i2c_master_data data_struct = {
+    struct hal_i2c_master_data wdata = {
         .address = itf->si_addr,
         .len = 1,
         .buffer = &reg
     };
+    struct hal_i2c_master_data rdata = {
+        .address = itf->si_addr,
+        .len = size,
+        .buffer = buffer,
+    };
 
-    /* Register write */
-    rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(LPS33HW_I2C_TIMEOUT_TICKS), 0,
-                           MYNEWT_VAL(LPS33HW_I2C_RETRIES));
+    rc = i2cn_master_write_read_transact(itf->si_num, &wdata, &rdata,
+                                         MYNEWT_VAL(LPS33HW_I2C_TIMEOUT_TICKS) * (size + 1),
+                                         1, MYNEWT_VAL(LPS33HW_I2C_RETRIES));
     if (rc) {
         LPS33HW_LOG(ERROR, "I2C access failed at address 0x%02X\n",
                     itf->si_addr);
-        STATS_INC(g_lps33hwstats, write_errors);
+        STATS_INC(g_lps33hwstats, read_errors);
         return rc;
     }
 
-    /* Read */
-    data_struct.len = size;
-    data_struct.buffer = buffer;
-    rc = i2cn_master_read(itf->si_num, &data_struct,
-                          (MYNEWT_VAL(LPS33HW_I2C_TIMEOUT_TICKS)) * size, 1,
-                          MYNEWT_VAL(LPS33HW_I2C_RETRIES));
-
-    if (rc) {
-        LPS33HW_LOG(ERROR, "Failed to read from 0x%02X:0x%02X\n",
-                    itf->si_addr, reg);
-        STATS_INC(g_lps33hwstats, read_errors);
-    }
     return rc;
 }
 #endif
diff --git a/hw/drivers/sensors/lps33thw/src/lps33thw.c b/hw/drivers/sensors/lps33thw/src/lps33thw.c
index f80f05064d..d3d44cb71a 100644
--- a/hw/drivers/sensors/lps33thw/src/lps33thw.c
+++ b/hw/drivers/sensors/lps33thw/src/lps33thw.c
@@ -361,33 +361,25 @@ lps33thw_i2c_get_regs(struct sensor_itf *itf, uint8_t reg, uint8_t size,
 
     rc = bus_node_simple_write_read_transact(odev, &reg, 1, buffer, size);
 #else
-    struct hal_i2c_master_data data_struct = {
+    struct hal_i2c_master_data wdata = {
         .address = itf->si_addr,
         .len = 1,
         .buffer = &reg
     };
+    struct hal_i2c_master_data rdata = {
+        .address = itf->si_addr,
+        .len = size,
+        .buffer = buffer,
+    };
 
-    /* Register write */
-    rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(LPS33THW_I2C_TIMEOUT_TICKS), 0,
-                           MYNEWT_VAL(LPS33THW_I2C_RETRIES));
+    rc = i2cn_master_write_read_transact(itf->si_num, &wdata, &rdata,
+                                         MYNEWT_VAL(LPS33THW_I2C_TIMEOUT_TICKS) * (size + 1),
+                                         1, MYNEWT_VAL(LPS33THW_I2C_RETRIES));
     if (rc) {
         LPS33THW_LOG(ERROR, "I2C access failed at address 0x%02X\n",
-                    itf->si_addr);
-        STATS_INC(g_lps33thwstats, write_errors);
-        return rc;
-    }
-
-    /* Read */
-    data_struct.len = size;
-    data_struct.buffer = buffer;
-    rc = i2cn_master_read(itf->si_num, &data_struct,
-                          (MYNEWT_VAL(LPS33THW_I2C_TIMEOUT_TICKS)) * size, 1,
-                          MYNEWT_VAL(LPS33THW_I2C_RETRIES));
-
-    if (rc) {
-        LPS33THW_LOG(ERROR, "Failed to read from 0x%02X:0x%02X\n",
-                    itf->si_addr, reg);
+                     itf->si_addr);
         STATS_INC(g_lps33thwstats, read_errors);
+        return rc;
     }
 #endif
 
diff --git a/hw/drivers/sensors/ms5840/src/ms5840.c b/hw/drivers/sensors/ms5840/src/ms5840.c
index 7892601c2f..0038a08a2b 100644
--- a/hw/drivers/sensors/ms5840/src/ms5840.c
+++ b/hw/drivers/sensors/ms5840/src/ms5840.c
@@ -379,12 +379,15 @@ ms5840_readlen(struct sensor_itf *itf, uint8_t addr, uint8_t *buffer,
 #if MYNEWT_VAL(BUS_DRIVER_PRESENT)
     rc = bus_node_simple_write_read_transact(itf->si_dev, &addr, 1, buffer, len);
 #else
-    uint8_t payload[3] = {addr, 0, 0};
-
-    struct hal_i2c_master_data data_struct = {
+    struct hal_i2c_master_data wdata = {
         .address = itf->si_addr,
         .len = 1,
-        .buffer = payload
+        .buffer = &addr,
+    };
+    struct hal_i2c_master_data rdata = {
+        .address = itf->si_addr,
+        .len = len,
+        .buffer = buffer,
     };
 
     /* Clear the supplied buffer */
@@ -395,32 +398,14 @@ ms5840_readlen(struct sensor_itf *itf, uint8_t addr, uint8_t *buffer,
         return rc;
     }
 
-    /* Command write */
-    rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(MS5840_I2C_TIMEOUT_TICKS), 1,
-                           MYNEWT_VAL(MS5840_I2C_RETRIES));
-    if (rc) {
-        MS5840_LOG(ERROR, "I2C read command write failed at address 0x%02X\n",
-                   data_struct.address);
-        STATS_INC(g_ms5840stats, write_errors);
-        goto err;
-    }
-
-    /* Read len bytes back */
-    memset(payload, 0, sizeof(payload));
-    data_struct.len = len;
-    rc = i2cn_master_read(itf->si_num, &data_struct, MYNEWT_VAL(MS5840_I2C_TIMEOUT_TICKS), 1,
-                          MYNEWT_VAL(MS5840_I2C_RETRIES));
+    rc = i2cn_master_write_read_transact(itf->si_num, &wdata, &rdata, MYNEWT_VAL(MS5840_I2C_TIMEOUT_TICKS) * 2,
+                                         1, MYNEWT_VAL(MS5840_I2C_RETRIES));
     if (rc) {
         MS5840_LOG(ERROR, "Failed to read from 0x%02X:0x%02X\n",
-                   data_struct.address, addr);
+                   itf->si_addr, addr);
         STATS_INC(g_ms5840stats, read_errors);
-        goto err;
     }
 
-    /* Copy the I2C results into the supplied buffer */
-    memcpy(buffer, payload, len);
-
-err:
     sensor_itf_unlock(itf);
 #endif
 
diff --git a/hw/util/i2cn/include/i2cn/i2cn.h b/hw/util/i2cn/include/i2cn/i2cn.h
index d6caaae1d3..16072b0ebe 100644
--- a/hw/util/i2cn/include/i2cn/i2cn.h
+++ b/hw/util/i2cn/include/i2cn/i2cn.h
@@ -35,8 +35,8 @@ extern "C" {
  * @param i2c_num               The index of the I2C interface to read from.
  * @param pdata                 Additional parameters describing the read
  *                                  operation.
- * @param timeout               The time, in OS ticks, to wait for the MCU to
- *                                  indicate completion of each clocked byte.
+ * @param timeout               The time, in OS ticks, to wait for operation
+ *                                  completion.
  * @param last_op               1 if this is the final message in the
  *                                  transaction.
  *
@@ -44,7 +44,7 @@ extern "C" {
  *                              HAL_I2C_ERR_[...] code on failure.
  */
 int i2cn_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
-                     uint32_t timeout, uint8_t last_op, int retries);
+                     os_time_t timeout, uint8_t last_op, int retries);
 
 /**
  * @brief Writes to an I2C slave, retrying the specified number of times on
@@ -53,8 +53,8 @@ int i2cn_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
  * @param i2c_num               The index of the I2C interface to write to.
  * @param pdata                 Additional parameters describing the write
  *                                  operation.
- * @param timeout               The time, in OS ticks, to wait for the MCU to
- *                                  indicate completion of each clocked byte.
+ * @param timeout               The time, in OS ticks, to wait for operation
+ *                                  completion.
  * @param last_op               1 if this is the final message in the
  *                                  transaction.
  *
@@ -62,7 +62,35 @@ int i2cn_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
  *                              HAL_I2C_ERR_[...] code on failure.
  */
 int i2cn_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
-                      uint32_t timeout, uint8_t last_op, int retries);
+                      os_time_t timeout, uint8_t last_op, int retries);
+
+/**
+ * @brief Writes to and then read from an I2C slave, retrying the specified
+ * number of times on failure.
+ *
+ * This should be used whenever sequence of write (e.g. register address on
+ * sensor) and read (e.g. data from that register) is required in favor of
+ * separate write and read as it guaranteed that in case of failed read, write
+ * will be performed once more to guarantee read is handled properly by device.
+ *
+ * @param i2c_num               The index of the I2C interface to write to.
+ * @param wdata                 Additional parameters describing the write
+ *                                  operation.
+ * @param rdata                 Additional parameters describing the read
+ *                                  operation.
+ * @param timeout               The time, in OS ticks, to wait for either read
+ *                                  or write operation completion.
+ * @param last_op               1 if read is the final message in the
+ *                                  transaction.
+ *
+ * @return                      0 on success;
+ *                              HAL_I2C_ERR_[...] code on failure.
+ */
+int i2cn_master_write_read_transact(uint8_t i2c_num,
+                                    struct hal_i2c_master_data *wdata,
+                                    struct hal_i2c_master_data *rdata,
+                                    os_time_t timeout, uint8_t last_op,
+                                    int retries);
 
 #ifdef __cplusplus
 }
diff --git a/hw/util/i2cn/src/i2cn.c b/hw/util/i2cn/src/i2cn.c
index 2322e1f97c..cd350c49df 100644
--- a/hw/util/i2cn/src/i2cn.c
+++ b/hw/util/i2cn/src/i2cn.c
@@ -22,7 +22,7 @@
 
 int
 i2cn_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
-                 uint32_t timeout, uint8_t last_op, int retries)
+                 os_time_t timeout, uint8_t last_op, int retries)
 {
     int rc = 0;
     int i;
@@ -44,7 +44,7 @@ i2cn_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
 
 int
 i2cn_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
-                  uint32_t timeout, uint8_t last_op, int retries)
+                  os_time_t timeout, uint8_t last_op, int retries)
 {
     int rc = 0;
     int i;
@@ -63,3 +63,33 @@ i2cn_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
 
     return rc;
 }
+
+int
+i2cn_master_write_read_transact(uint8_t i2c_num,
+                                struct hal_i2c_master_data *wdata,
+                                struct hal_i2c_master_data *rdata,
+                                os_time_t timeout, uint8_t last_op, int retries)
+{
+    int rc = 0;
+    int i;
+
+    /* Ensure at least one try. */
+    if (retries < 0) {
+        retries = 0;
+    }
+
+    for (i = 0; i <= retries; i++) {
+        rc = hal_i2c_master_write(i2c_num, wdata, timeout, 0);
+        if (rc != 0) {
+            continue;
+        }
+
+        rc = hal_i2c_master_read(i2c_num, rdata, timeout, last_op);
+        if (rc == 0) {
+            break;
+        }
+    }
+
+    return rc;
+
+}


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services