You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by vi...@apache.org on 2018/11/19 17:07:47 UTC

[mynewt-core] branch dma_i2c_new_api created (now b9fd22e)

This is an automated email from the ASF dual-hosted git repository.

vipulrahane pushed a change to branch dma_i2c_new_api
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git.


      at b9fd22e  Add new API for double buffered I2C

This branch includes the following new commits:

     new 34b7101  Initial commit dma i2c
     new e4fb5a4  fix prototype and console_printf
     new b2a038c   fixes
     new 29b3078  fix address compare for transact_end
     new ec4b3fb  revert bq27z561 changes to break up transactions
     new 13a1c69  add logs
     new b9fd22e  Add new API for double buffered I2C

The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[mynewt-core] 02/07: fix prototype and console_printf

Posted by vi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vipulrahane pushed a commit to branch dma_i2c_new_api
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit e4fb5a407c5f496e768c7a9ad00782d785ef98e6
Author: Vipul Rahane <vi...@runtime.io>
AuthorDate: Thu Oct 18 14:13:35 2018 -0700

    fix prototype and console_printf
---
 hw/mcu/nordic/nrf52xxx/src/hal_i2c.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index b19e2ad..10e59d7 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -33,8 +33,11 @@
 #define I2C_WRITE 0
 #define I2C_READ  1
 
-#define WAIT_FOR_EVT(__r, __e)\
+#define WAIT_FOR_EVT(__r, __e, __op)\
     while (!(__r)->EVENTS_####__e) {\
+        console_printf("while op: %u macro: STOPPED: %lu SUSPENDED: %lu LASTTX: %lu LASTRX: %lu\n",\
+                   __op, regs->EVENTS_STOPPED, regs->EVENTS_SUSPENDED, regs->EVENTS_LASTTX,\
+                   regs->EVENTS_LASTRX);\
         now = os_time_get();\
         if (OS_TIME_TICK_GT(now, abs_timo)) {\
             rc = HAL_I2C_ERR_TIMEOUT;\
@@ -353,12 +356,12 @@ hal_i2c_handle_transact_start(struct nrf52_hal_i2c *i2c, uint8_t op,
     if (op == I2C_WRITE) {
         /* Start an I2C transmit transaction */
         regs->TASKS_STARTTX = 1;
-        WAIT_FOR_EVT(regs, TXSTARTED);
+        WAIT_FOR_EVT(regs, TXSTARTED, op);
         regs->EVENTS_TXSTARTED = 0;
     } else {
         /* Start an I2C receive transaction */
         regs->TASKS_STARTRX = 1;
-        WAIT_FOR_EVT(regs, RXSTARTED);
+        WAIT_FOR_EVT(regs, RXSTARTED, op);
         regs->EVENTS_RXSTARTED = 0;
     }
 
@@ -368,9 +371,8 @@ err:
 }
 
 static int
-hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint32_t start,
-                            uint32_t abs_timo, uint8_t last_op,
-                            uint8_t op)
+hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
+                            uint32_t abs_timo, uint8_t last_op)
 {
     int rc;
     uint32_t evt;
@@ -399,7 +401,7 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint32_t start,
         }
 
         if (evt) {
-            if (evt == regs->EVENTS_STOPPED) {
+            if (&evt == &regs->EVENTS_STOPPED) {
                 regs->EVENTS_LASTTX = 0;
                 regs->EVENTS_LASTRX = 0;
             }
@@ -410,6 +412,11 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint32_t start,
             goto err;
         }
 
+        console_printf("while end: last_op: %u STOPPED: %lu SUSPENDED: %lu"
+                       "LASTTX: %lu LASTRX: %lu\n", last_op, regs->EVENTS_STOPPED,
+                       regs->EVENTS_SUSPENDED, regs->EVENTS_LASTTX,
+                       regs->EVENTS_LASTRX);
+
         now = os_time_get();
         if (OS_TIME_TICK_GT(now, abs_timo)) {
             rc = HAL_I2C_ERR_TIMEOUT;
@@ -437,8 +444,9 @@ hal_i2c_bus_error_detect(struct nrf52_hal_i2c *i2c)
          */
         rc = HAL_I2C_ERR_SUSPEND;
         goto err;
-    } else if (i2c->nhi_prev_last_op  == 1 &&
-               (!regs->EVENTS_STOPPED && regs->EVENTS_SUSPENDED)) {
+    } else if ((i2c->nhi_prev_last_op == 1 &&
+               !regs->EVENTS_STOPPED) && regs->EVENTS_SUSPENDED) {
+
         /*
          * return HAL_I2C_ERR_STOP if previous last_op was 1 and
          * the bus is not in the stopped state or in initial state
@@ -450,6 +458,8 @@ hal_i2c_bus_error_detect(struct nrf52_hal_i2c *i2c)
 
     return 0;
 err:
+    console_printf("i2c failure: rc:%u evt_stopped: %lu evt_suspended: %lu\n",
+                   rc, regs->EVENTS_STOPPED, regs->EVENTS_SUSPENDED);
     return rc;
 }
 


[mynewt-core] 04/07: fix address compare for transact_end

Posted by vi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vipulrahane pushed a commit to branch dma_i2c_new_api
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit 29b3078afb7621d58970d48a214df88b2b10b11b
Author: Vipul Rahane <vi...@runtime.io>
AuthorDate: Mon Oct 22 16:21:03 2018 -0700

    fix address compare for transact_end
---
 hw/mcu/nordic/nrf52xxx/src/hal_i2c.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index 133e330..5a81e86 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -396,7 +396,7 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
                             os_time_t abs_timo, uint8_t last_op)
 {
     int rc;
-    uint32_t evt;
+    volatile uint32_t *evt;
     os_time_t now;
 
     while(1) {
@@ -405,13 +405,13 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
          * monitored
          */
         if (last_op) {
-            evt = regs->EVENTS_STOPPED;
+            evt = &regs->EVENTS_STOPPED;
         } else {
-            evt = regs->EVENTS_SUSPENDED;
+            evt = &regs->EVENTS_SUSPENDED;
         }
 
-        if (evt) {
-            if (&evt == &regs->EVENTS_STOPPED) {
+        if (*evt) {
+            if (evt == &regs->EVENTS_STOPPED) {
 #if MYNEWT_VAL(NRF52_HANDLE_ANOMALY_109)
                 if (regs->FREQUENCY) {
                     regs->FREQUENCY = 0;


[mynewt-core] 06/07: add logs

Posted by vi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vipulrahane pushed a commit to branch dma_i2c_new_api
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit 13a1c69c76edd16a7964953967ae76bb2ce32c73
Author: Vipul Rahane <vi...@runtime.io>
AuthorDate: Wed Oct 24 20:00:17 2018 -0700

    add logs
---
 hw/mcu/nordic/nrf52xxx/src/hal_i2c.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index 5a81e86..c2c5e71 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -391,13 +391,15 @@ hal_i2c_handle_transact_start(struct nrf52_hal_i2c *i2c, uint8_t op,
     return 0;
 }
 
+os_time_t g_start;
+
 static int
 hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
                             os_time_t abs_timo, uint8_t last_op)
 {
     int rc;
     volatile uint32_t *evt;
-    os_time_t now;
+    os_time_t now = 0;
 
     while(1) {
         /*
@@ -432,6 +434,10 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
         }
     }
 
+    g_start = os_cputime_get32() - g_start;
+
+    console_printf("t:%lu r:%u\n", g_start, op);
+
     return 0;
 err:
     return rc;
@@ -510,6 +516,8 @@ hal_i2c_handle_errors(struct nrf52_hal_i2c *i2c, int rc, os_time_t abs_timo)
     return rc;
 }
 
+os_time_t g_start;
+
 /* Perform I2C master writes using TWIM/EasyDMA */
 int
 hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
@@ -521,6 +529,7 @@ hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
     struct nrf52_hal_i2c *i2c;
 
     start = os_time_get();
+    g_start = os_cputime_get32();
 
     /* Resolve the I2C bus */
     rc = hal_i2c_resolve(i2c_num, &i2c);
@@ -595,6 +604,7 @@ hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
     struct nrf52_hal_i2c *i2c;
 
     start = os_time_get();
+    g_start = os_cputime_get32();
 
     /* Resolve the I2C bus */
     rc = hal_i2c_resolve(i2c_num, &i2c);


[mynewt-core] 05/07: revert bq27z561 changes to break up transactions

Posted by vi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vipulrahane pushed a commit to branch dma_i2c_new_api
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit ec4b3fb7a148b0a32cbe715a3876074604c4f7c7
Author: Vipul Rahane <vi...@runtime.io>
AuthorDate: Mon Oct 22 16:26:12 2018 -0700

    revert bq27z561 changes to break up transactions
    
    - not needed
---
 hw/drivers/bq27z561/src/bq27z561.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/drivers/bq27z561/src/bq27z561.c b/hw/drivers/bq27z561/src/bq27z561.c
index 234647d..cf455df 100644
--- a/hw/drivers/bq27z561/src/bq27z561.c
+++ b/hw/drivers/bq27z561/src/bq27z561.c
@@ -142,7 +142,7 @@ bq27z561_rd_std_reg_byte(struct bq27z561 *dev, uint8_t reg, uint8_t *val)
     }
 
 
-    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 1,
+    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 0,
                            MYNEWT_VAL(BQ27Z561_I2C_RETRIES));
     if (rc != 0) {
         BQ27Z561_LOG(ERROR, "I2C reg read (wr) failed 0x%02X\n", reg);
@@ -178,7 +178,7 @@ bq27z561_rd_std_reg_word(struct bq27z561 *dev, uint8_t reg, uint16_t *val)
         return rc;
     }
 
-    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 1,
+    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 0,
                            MYNEWT_VAL(BQ27Z561_I2C_RETRIES));
     if (rc != 0) {
         BQ27Z561_LOG(ERROR, "I2C reg read (wr) failed 0x%02X\n", reg);
@@ -353,7 +353,7 @@ bq27x561_rd_alt_mfg_cmd(struct bq27z561 *dev, uint16_t cmd, uint8_t *val,
     i2c.len = 1;
     i2c.buffer = tmpbuf;
 
-    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 1,
+    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 0,
                            MYNEWT_VAL(BQ27Z561_I2C_RETRIES));
     if (rc != 0) {
         BQ27Z561_LOG(ERROR, "I2C reg read (wr) failed 0x%02X\n", tmpbuf[0]);
@@ -459,7 +459,7 @@ bq27x561_rd_flash(struct bq27z561 *dev, uint16_t addr, uint8_t *buf, int buflen)
     i2c.len = 1;
     i2c.buffer = tmpbuf;
 
-    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 1,
+    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 0,
                            MYNEWT_VAL(BQ27Z561_I2C_RETRIES));
     if (rc != 0) {
         BQ27Z561_LOG(ERROR, "I2C reg read (wr) failed 0x%02X\n", tmpbuf[0]);


[mynewt-core] 01/07: Initial commit dma i2c

Posted by vi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vipulrahane pushed a commit to branch dma_i2c_new_api
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit 34b7101e04e7ad2f20c989961719685dd46fa5b4
Author: Vipul Rahane <vi...@runtime.io>
AuthorDate: Wed Oct 17 12:58:20 2018 -0700

    Initial commit dma i2c
---
 hw/drivers/sensors/tsl2561/src/tsl2561_shell.c |   2 +-
 hw/hal/include/hal/hal_i2c.h                   |   9 +
 hw/mcu/nordic/nrf52xxx/src/hal_i2c.c           | 376 +++++++++++++++++--------
 3 files changed, 273 insertions(+), 114 deletions(-)

diff --git a/hw/drivers/sensors/tsl2561/src/tsl2561_shell.c b/hw/drivers/sensors/tsl2561/src/tsl2561_shell.c
index 71201e6..169e9c3 100644
--- a/hw/drivers/sensors/tsl2561/src/tsl2561_shell.c
+++ b/hw/drivers/sensors/tsl2561/src/tsl2561_shell.c
@@ -348,7 +348,7 @@ tsl2561_shell_cmd_en(int argc, char **argv)
     /* Update the enable state */
     if (argc == 3) {
         lval = strtol(argv[2], &endptr, 10); /* Base 10 */
-        if (argv[2] != '\0' && *endptr == '\0' &&
+        if (*argv[2] != '\0' && *endptr == '\0' &&
             lval >= 0 && lval <= 1) {
             rc = tsl2561_enable(&g_sensor_itf, lval);
             if (rc) {
diff --git a/hw/hal/include/hal/hal_i2c.h b/hw/hal/include/hal/hal_i2c.h
index be080ca..50fd1df 100644
--- a/hw/hal/include/hal/hal_i2c.h
+++ b/hw/hal/include/hal/hal_i2c.h
@@ -79,6 +79,15 @@ extern "C" {
 /** Slave responded to data byte with NACK. */
 #define HAL_I2C_ERR_DATA_NACK           5
 
+/** Overrun error */
+#define HAL_I2C_ERR_OVERRUN             6
+
+/** I2C bus in suspended state, but last op was 0 */
+#define HAL_I2C_ERR_SUSPEND             7
+
+/** I2C bus in stopped state, but last op was 1   */
+#define HAL_I2C_ERR_STOP                8
+
 /**
  * When sending a packet, use this structure to pass the arguments.
  */
diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index b925394..b19e2ad 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -26,9 +26,22 @@
 #include <hal/hal_gpio.h>
 #include <mcu/nrf52_hal.h>
 #include "nrf_twim.h"
+#include <console/console.h>
 
 #include <nrf.h>
 
+#define I2C_WRITE 0
+#define I2C_READ  1
+
+#define WAIT_FOR_EVT(__r, __e)\
+    while (!(__r)->EVENTS_####__e) {\
+        now = os_time_get();\
+        if (OS_TIME_TICK_GT(now, abs_timo)) {\
+            rc = HAL_I2C_ERR_TIMEOUT;\
+            goto err;\
+        }\
+    }
+
 #define NRF52_HAL_I2C_MAX (2)
 
 #define NRF52_SCL_PIN_CONF                                              \
@@ -48,17 +61,22 @@
 #define NRF52_SDA_PIN_CONF_CLR    NRF52_SCL_PIN_CONF_CLR
 
 struct nrf52_hal_i2c {
-    NRF_TWI_Type *nhi_regs;
+    NRF_TWIM_Type *nhi_regs;
+    /*
+     * previous last_op, this is just to check if the previous
+     * transaction actually put the I2C bus in the suspended state
+     */
+    int nhi_prev_last_op;
 };
 
 #if MYNEWT_VAL(I2C_0)
 struct nrf52_hal_i2c hal_twi_i2c0 = {
-    .nhi_regs = NRF_TWI0
+    .nhi_regs = NRF_TWIM0
 };
 #endif
 #if MYNEWT_VAL(I2C_1)
 struct nrf52_hal_i2c hal_twi_i2c1 = {
-    .nhi_regs = NRF_TWI1
+    .nhi_regs = NRF_TWIM1
 };
 #endif
 
@@ -180,6 +198,8 @@ hal_i2c_convert_status(int nrf_status)
         return HAL_I2C_ERR_DATA_NACK;
     } else if (nrf_status & NRF_TWIM_ERROR_ADDRESS_NACK) {
         return HAL_I2C_ERR_ADDR_NACK;
+    } else if (nrf_status & TWIM_ERRORSRC_OVERRUN_Msk) {
+        return HAL_I2C_ERR_OVERRUN;
     } else {
         return HAL_I2C_ERR_UNKNOWN;
     }
@@ -257,7 +277,7 @@ int
 hal_i2c_init(uint8_t i2c_num, void *usercfg)
 {
     struct nrf52_hal_i2c *i2c;
-    NRF_TWI_Type *regs;
+    NRF_TWIM_Type *regs;
     struct nrf52_hal_i2c_cfg *cfg;
     uint32_t freq;
     int rc;
@@ -275,13 +295,13 @@ hal_i2c_init(uint8_t i2c_num, void *usercfg)
 
     switch (cfg->i2c_frequency) {
     case 100:
-        freq = TWI_FREQUENCY_FREQUENCY_K100;
+        freq = TWIM_FREQUENCY_FREQUENCY_K100;
         break;
     case 250:
-        freq = TWI_FREQUENCY_FREQUENCY_K250;
+        freq = TWIM_FREQUENCY_FREQUENCY_K250;
         break;
     case 400:
-        freq = TWI_FREQUENCY_FREQUENCY_K400;
+        freq = TWIM_FREQUENCY_FREQUENCY_K400;
         break;
     default:
         rc = HAL_I2C_ERR_INVAL;
@@ -297,179 +317,309 @@ hal_i2c_init(uint8_t i2c_num, void *usercfg)
     scl_port->PIN_CNF[cfg->scl_pin] = NRF52_SCL_PIN_CONF;
     sda_port->PIN_CNF[cfg->sda_pin] = NRF52_SDA_PIN_CONF;
 
-    regs->PSELSCL = cfg->scl_pin;
-    regs->PSELSDA = cfg->sda_pin;
+    regs->PSEL.SCL = cfg->scl_pin;
+    regs->PSEL.SDA = cfg->sda_pin;
     regs->FREQUENCY = freq;
-    regs->ENABLE = TWI_ENABLE_ENABLE_Enabled;
+    regs->ENABLE = TWIM_ENABLE_ENABLE_Enabled;
+    i2c->nhi_prev_last_op = -1;
 
     return (0);
 err:
     return (rc);
 }
 
-int
-hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
-                     uint32_t timo, uint8_t last_op)
+/*
+ * Starts an I2C transaction either read or write using EasyDMA/TWIM
+ */
+static int
+hal_i2c_handle_transact_start(struct nrf52_hal_i2c *i2c, uint8_t op,
+                              uint32_t abs_timo)
 {
-    struct nrf52_hal_i2c *i2c;
-    NRF_TWI_Type *regs;
-    int nrf_status;
+    NRF_TWIM_Type *regs;
+    uint32_t now;
     int rc;
-    int i;
-    uint32_t start;
 
-    rc = hal_i2c_resolve(i2c_num, &i2c);
-    if (rc != 0) {
-        return rc;
-    }
     regs = i2c->nhi_regs;
 
-    regs->ADDRESS = pdata->address;
-
     regs->EVENTS_ERROR = 0;
+
+    /* If a transaction was previously suspended, resume it */
+    if (regs->EVENTS_SUSPENDED) {
+        regs->TASKS_RESUME = 1;
+    }
+
     regs->EVENTS_STOPPED = 0;
     regs->EVENTS_SUSPENDED = 0;
-    regs->SHORTS = 0;
+    if (op == I2C_WRITE) {
+        /* Start an I2C transmit transaction */
+        regs->TASKS_STARTTX = 1;
+        WAIT_FOR_EVT(regs, TXSTARTED);
+        regs->EVENTS_TXSTARTED = 0;
+    } else {
+        /* Start an I2C receive transaction */
+        regs->TASKS_STARTRX = 1;
+        WAIT_FOR_EVT(regs, RXSTARTED);
+        regs->EVENTS_RXSTARTED = 0;
+    }
 
-    regs->TASKS_STARTTX = 1;
-    regs->TASKS_RESUME = 1;
+    return 0;
+err:
+    return rc;
+}
 
-    start = os_time_get();
-    for (i = 0; i < pdata->len; i++) {
-        regs->EVENTS_TXDSENT = 0;
-        regs->TXD = pdata->buffer[i];
-        while (!regs->EVENTS_TXDSENT && !regs->EVENTS_ERROR) {
-            if (os_time_get() - start > timo) {
-                rc = HAL_I2C_ERR_TIMEOUT;
-                goto err;
-            }
-        }
-        if (regs->EVENTS_ERROR) {
-            goto err;
+static int
+hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint32_t start,
+                            uint32_t abs_timo, uint8_t last_op,
+                            uint8_t op)
+{
+    int rc;
+    uint32_t evt;
+    os_time_t now;
+
+    if (last_op) {
+        /* this is to take care of clock stretches as per the datasheet */
+        if (regs->EVENTS_SUSPENDED) {
+            /* If there is a clock stretch, the event would be suspended for
+             * that TWIM interface, it would need to be resumed to finish the
+             * transaction
+             */
+            regs->TASKS_RESUME = 1;
         }
     }
-    /* If last_op is zero it means we dont put a stop at end. */
-    if (last_op) {
-        regs->EVENTS_STOPPED = 0;
-        regs->TASKS_STOP = 1;
-        while (!regs->EVENTS_STOPPED && !regs->EVENTS_ERROR) {
-            if (os_time_get() - start > timo) {
-                rc = HAL_I2C_ERR_TIMEOUT;
-                goto err;
+
+    while(1) {
+        /*
+         * Use last_op as the determining factor for the type of event to be
+         * monitored
+         */
+        if (last_op) {
+            evt = regs->EVENTS_STOPPED;
+        } else {
+            evt = regs->EVENTS_SUSPENDED;
+        }
+
+        if (evt) {
+            if (evt == regs->EVENTS_STOPPED) {
+                regs->EVENTS_LASTTX = 0;
+                regs->EVENTS_LASTRX = 0;
             }
+            break;
         }
+
         if (regs->EVENTS_ERROR) {
             goto err;
         }
+
+        now = os_time_get();
+        if (OS_TIME_TICK_GT(now, abs_timo)) {
+            rc = HAL_I2C_ERR_TIMEOUT;
+            goto err;
+        }
     }
 
-    rc = 0;
+    return 0;
+err:
+    return rc;
+}
+
+static int
+hal_i2c_bus_error_detect(struct nrf52_hal_i2c *i2c)
+{
+    int rc;
+    NRF_TWIM_Type *regs;
+
+    regs = i2c->nhi_regs;
+
+    if (i2c->nhi_prev_last_op == 0 && !regs->EVENTS_SUSPENDED) {
+        /*
+         * return HAL_I2C_ERR_SUSPEND if previous last_op was 0 and
+         * the bus was not in the suspended state
+         */
+        rc = HAL_I2C_ERR_SUSPEND;
+        goto err;
+    } else if (i2c->nhi_prev_last_op  == 1 &&
+               (!regs->EVENTS_STOPPED && regs->EVENTS_SUSPENDED)) {
+        /*
+         * return HAL_I2C_ERR_STOP if previous last_op was 1 and
+         * the bus is not in the stopped state or in initial state
+         * which is EVENTS_SUSPENDED is 1 and EVENTS_STOPPED is 1
+         */
+        rc = HAL_I2C_ERR_STOP;
+        goto err;
+    }
 
+    return 0;
 err:
+    return rc;
+}
+
+/* Handle errors returned from the TWIM peripheral along with timeouts */
+static int
+hal_i2c_handle_errors(NRF_TWIM_Type *regs, int rc)
+{
+    int nrf_status;
+
+    regs->TASKS_RESUME = 1;
     regs->TASKS_STOP = 1;
 
     if (regs->EVENTS_ERROR) {
+        regs->EVENTS_ERROR = 0;
         nrf_status = regs->ERRORSRC;
         regs->ERRORSRC = nrf_status;
         rc = hal_i2c_convert_status(nrf_status);
-    } else if (rc == HAL_I2C_ERR_TIMEOUT) {
+    } else if (rc) {
         /* Some I2C slave peripherals cause a glitch on the bus when they
-         * reset which puts the TWI in an unresponsive state.  Disabling and
+         * reset which puts the TWI in an unresponsive state. Disabling and
          * re-enabling the TWI returns it to normal operation.
          * A clear operation is performed in case one of the devices on
          * the bus is in a bad state.
          */
-        regs->ENABLE = TWI_ENABLE_ENABLE_Disabled;
-        hal_i2c_clear_bus(regs->PSELSCL, regs->PSELSDA);
-        regs->ENABLE = TWI_ENABLE_ENABLE_Enabled;
+        regs->ENABLE = TWIM_ENABLE_ENABLE_Disabled;
+        hal_i2c_clear_bus(regs->PSEL.SCL, regs->PSEL.SDA);
+        regs->ENABLE = TWIM_ENABLE_ENABLE_Enabled;
     }
 
-    return (rc);
+    return rc;
 }
 
+/* Perform I2C master writes using TWIM/EasyDMA */
 int
-hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
-                    uint32_t timo, uint8_t last_op)
+hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
+                     uint32_t timo, uint8_t last_op)
 {
-    struct nrf52_hal_i2c *i2c;
-    NRF_TWI_Type *regs;
-    int nrf_status;
     int rc;
-    int i;
     uint32_t start;
+    NRF_TWIM_Type *regs;
+    struct nrf52_hal_i2c *i2c;
 
+    start = os_time_get();
+
+    /* Resolve the I2C bus */
     rc = hal_i2c_resolve(i2c_num, &i2c);
     if (rc != 0) {
         return rc;
     }
+
     regs = i2c->nhi_regs;
 
-    start = os_time_get();
+    /* Detect errors on the bus based on the previous and current
+     * condition of the bus
+     */
+    rc = hal_i2c_bus_error_detect(i2c);
+    if (rc) {
+        goto err;
+    }
 
-    if (regs->EVENTS_RXDREADY) {
-        /*
-         * If previous read was interrupted, flush RXD.
+    /* Configure the TXD registers for EasyDMA access to work with buffers of
+     * specific length and address of the slave
+     */
+    regs->ADDRESS    = pdata->address;
+    regs->TXD.MAXCNT = pdata->len;
+    regs->TXD.PTR    = (uint32_t)pdata->buffer;
+    regs->TXD.LIST   = 0;
+    /* Disable and clear interrupts */
+    regs->INTENCLR   = NRF_TWIM_ALL_INTS_MASK;
+    regs->INTEN      = 0;
+
+    /* Setup shorts to end transaction based on last_op,
+     * 0 : STOP transaction,
+     * 1 : SUSPEND transaction
+     */
+    if (last_op) {
+        /* EVENT_STOPPED would get set after LASTTX gets set at
+         * the end of the transaction for the last byte
+         */
+        regs->SHORTS = TWIM_SHORTS_LASTTX_STOP_Msk;
+    } else {
+        /* EVENT_SUSPENDED would get set after LASTTX gets set at
+         * the end of the transaction for the last byte
          */
-        (void)regs->RXD;
-        (void)regs->RXD;
+        regs->SHORTS = TWIM_SHORTS_LASTTX_SUSPEND_Msk;
     }
-    regs->EVENTS_ERROR = 0;
-    regs->EVENTS_STOPPED = 0;
-    regs->EVENTS_SUSPENDED = 0;
-    regs->EVENTS_RXDREADY = 0;
 
-    regs->ADDRESS = pdata->address;
-
-    if (pdata->len == 1 && last_op) {
-        regs->SHORTS = TWI_SHORTS_BB_STOP_Msk;
-    } else {
-        regs->SHORTS = TWI_SHORTS_BB_SUSPEND_Msk;
+    /* Starts an I2C transaction using TWIM/EasyDMA */
+    rc = hal_i2c_handle_transact_start(i2c, I2C_WRITE, start + timo);
+    if (rc) {
+        goto err;
     }
-    regs->TASKS_STARTRX = 1;
 
-    for (i = 0; i < pdata->len; i++) {
-        regs->TASKS_RESUME = 1;
-        while (!regs->EVENTS_RXDREADY && !regs->EVENTS_ERROR) {
-            if (os_time_get() - start > timo) {
-                rc = HAL_I2C_ERR_TIMEOUT;
-                goto err;
-            }
-        }
-        if (regs->EVENTS_ERROR) {
-            goto err;
-        }
-        pdata->buffer[i] = regs->RXD;
-        if (i == pdata->len - 2) {
-            if (last_op) {
-                regs->SHORTS = TWI_SHORTS_BB_STOP_Msk;
-            }
-        }
-        regs->EVENTS_RXDREADY = 0;
+    /* Ends an I2C transaction using TWIM/EasyDMA */
+    rc = hal_i2c_handle_transact_end(regs, I2C_WRITE, start, start + timo, last_op);
+    if (rc) {
+        goto err;
     }
 
-    return (0);
+    /* Save the current last op to detect bus errors later */
+    i2c->nhi_prev_last_op = last_op;
 
+    return 0;
 err:
-    regs->TASKS_STOP = 1;
-    regs->SHORTS = TWI_SHORTS_BB_STOP_Msk;
+    return hal_i2c_handle_errors(regs, rc);
+}
 
-    if (regs->EVENTS_ERROR) {
-        nrf_status = regs->ERRORSRC;
-        regs->ERRORSRC = nrf_status;
-        rc = hal_i2c_convert_status(nrf_status);
-    } else if (rc == HAL_I2C_ERR_TIMEOUT) {
-       /* Some I2C slave peripherals cause a glitch on the bus when they
-        * reset which puts the TWI in an unresponsive state.  Disabling and
-        * re-enabling the TWI returns it to normal operation.
-        * A clear operation is performed in case one of the devices on
-        * the bus is in a bad state.
-        */
-        regs->ENABLE = TWI_ENABLE_ENABLE_Disabled;
-        hal_i2c_clear_bus(regs->PSELSCL, regs->PSELSDA);
-        regs->ENABLE = TWI_ENABLE_ENABLE_Enabled;
+int
+hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
+                    uint32_t timo, uint8_t last_op)
+{
+    int rc;
+    uint32_t start;
+    NRF_TWIM_Type *regs;
+    struct nrf52_hal_i2c *i2c;
+
+    start = os_time_get();
+
+    /* Resolve the I2C bus */
+    rc = hal_i2c_resolve(i2c_num, &i2c);
+    if (rc != 0) {
+        return rc;
     }
 
-    return (rc);
+    regs = i2c->nhi_regs;
+
+    /* Detect errors on the bus based on the previous and current
+     * condition of the bus
+     */
+    rc = hal_i2c_bus_error_detect(i2c);
+    if (rc) {
+        goto err;
+    }
+
+    /* Configure the RXD registers for EasyDMA access to work with buffers of
+     * specific length and address of the slave
+     */
+    regs->ADDRESS    = pdata->address;
+    regs->RXD.MAXCNT = pdata->len;
+    regs->RXD.PTR    = (uint32_t)pdata->buffer;
+    regs->RXD.LIST   = 0;
+    /* Disable and clear interrupts */
+    regs->INTENCLR   = NRF_TWIM_ALL_INTS_MASK;
+    regs->INTEN      = 0;
+
+    /* Only set short for RX->STOP for last_op:1 since there is no suspend short
+     * available in nrf52832
+     */
+    if (last_op) {
+        regs->SHORTS = TWIM_SHORTS_LASTRX_STOP_Msk;
+    }
+
+    /* Starts an I2C transaction using TWIM/EasyDMA */
+    rc = hal_i2c_handle_transact_start(i2c, I2C_READ, start + timo);
+    if (rc) {
+        goto err;
+    }
+
+    /* Ends an I2C transaction using TWIM/EasyDMA */
+    rc = hal_i2c_handle_transact_end(regs, I2C_READ, start, start + timo, last_op);
+    if (rc) {
+        goto err;
+    }
+
+    /* Save the current last op to detect bus errors later */
+    i2c->nhi_prev_last_op = last_op;
+
+    return 0;
+err:
+    return hal_i2c_handle_errors(regs, rc);
 }
 
 int


[mynewt-core] 03/07: fixes

Posted by vi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vipulrahane pushed a commit to branch dma_i2c_new_api
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit b2a038c567cf20bb329d9086551ec2ea02bc514e
Author: Vipul Rahane <vi...@runtime.io>
AuthorDate: Mon Oct 22 13:36:03 2018 -0700

     fixes
---
 hw/drivers/bq27z561/src/bq27z561.c         |   8 +-
 hw/drivers/sensors/lis2dw12/src/lis2dw12.c |  14 ++--
 hw/hal/include/hal/hal_i2c.h               |  26 +++++--
 hw/mcu/nordic/nrf52xxx/src/hal_i2c.c       | 116 +++++++++++++++++------------
 hw/mcu/nordic/nrf52xxx/syscfg.yml          |   4 +
 5 files changed, 100 insertions(+), 68 deletions(-)

diff --git a/hw/drivers/bq27z561/src/bq27z561.c b/hw/drivers/bq27z561/src/bq27z561.c
index cf455df..234647d 100644
--- a/hw/drivers/bq27z561/src/bq27z561.c
+++ b/hw/drivers/bq27z561/src/bq27z561.c
@@ -142,7 +142,7 @@ bq27z561_rd_std_reg_byte(struct bq27z561 *dev, uint8_t reg, uint8_t *val)
     }
 
 
-    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 0,
+    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 1,
                            MYNEWT_VAL(BQ27Z561_I2C_RETRIES));
     if (rc != 0) {
         BQ27Z561_LOG(ERROR, "I2C reg read (wr) failed 0x%02X\n", reg);
@@ -178,7 +178,7 @@ bq27z561_rd_std_reg_word(struct bq27z561 *dev, uint8_t reg, uint16_t *val)
         return rc;
     }
 
-    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 0,
+    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 1,
                            MYNEWT_VAL(BQ27Z561_I2C_RETRIES));
     if (rc != 0) {
         BQ27Z561_LOG(ERROR, "I2C reg read (wr) failed 0x%02X\n", reg);
@@ -353,7 +353,7 @@ bq27x561_rd_alt_mfg_cmd(struct bq27z561 *dev, uint16_t cmd, uint8_t *val,
     i2c.len = 1;
     i2c.buffer = tmpbuf;
 
-    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 0,
+    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 1,
                            MYNEWT_VAL(BQ27Z561_I2C_RETRIES));
     if (rc != 0) {
         BQ27Z561_LOG(ERROR, "I2C reg read (wr) failed 0x%02X\n", tmpbuf[0]);
@@ -459,7 +459,7 @@ bq27x561_rd_flash(struct bq27z561 *dev, uint16_t addr, uint8_t *buf, int buflen)
     i2c.len = 1;
     i2c.buffer = tmpbuf;
 
-    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 0,
+    rc = i2cn_master_write(dev->bq27_itf.itf_num, &i2c, MYNEWT_VAL(BQ27Z561_I2C_TIMEOUT_TICKS), 1,
                            MYNEWT_VAL(BQ27Z561_I2C_RETRIES));
     if (rc != 0) {
         BQ27Z561_LOG(ERROR, "I2C reg read (wr) failed 0x%02X\n", tmpbuf[0]);
diff --git a/hw/drivers/sensors/lis2dw12/src/lis2dw12.c b/hw/drivers/sensors/lis2dw12/src/lis2dw12.c
index f690400..11a7309 100644
--- a/hw/drivers/sensors/lis2dw12/src/lis2dw12.c
+++ b/hw/drivers/sensors/lis2dw12/src/lis2dw12.c
@@ -235,8 +235,8 @@ lis2dw12_i2c_writelen(struct sensor_itf *itf, uint8_t addr, uint8_t *buffer,
     rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(LIS2DW12_I2C_TIMEOUT_TICKS), 1,
                            MYNEWT_VAL(LIS2DW12_I2C_RETRIES));
     if (rc) {
-        LIS2DW12_LOG(ERROR, "I2C access failed at address 0x%02X\n",
-                     data_struct.address);
+        LIS2DW12_LOG(ERROR, "I2C write failed err=%d at addr:0x%02X:0x%02X\n",
+                     rc, data_struct.address, payload[0]);
         STATS_INC(g_lis2dw12stats, write_errors);
         goto err;
     }
@@ -290,7 +290,7 @@ lis2dw12_spi_writelen(struct sensor_itf *itf, uint8_t addr, uint8_t *payload,
         rc = hal_spi_tx_val(itf->si_num, payload[i]);
         if (rc == 0xFFFF) {
             rc = SYS_EINVAL;
-            LIS2DW12_LOG(ERROR, "SPI_%u write failed addr:0x%02X:0x%02X\n",
+            LIS2DW12_LOG(ERROR, "SPI_%u write failed addr:0x%02X\n",
                          itf->si_num, addr);
             STATS_INC(g_lis2dw12stats, write_errors);
             goto err;
@@ -365,8 +365,8 @@ lis2dw12_i2c_readlen(struct sensor_itf *itf, uint8_t reg, uint8_t *buffer,
     rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(LIS2DW12_I2C_TIMEOUT_TICKS), 1,
                            MYNEWT_VAL(LIS2DW12_I2C_RETRIES));
     if (rc) {
-        LIS2DW12_LOG(ERROR, "I2C access failed at address 0x%02X\n",
-                     itf->si_addr);
+        LIS2DW12_LOG(ERROR, "I2C %u write failed at address 0x%02X:0x%02X\n",
+                     itf->si_num, itf->si_addr, reg);
         STATS_INC(g_lis2dw12stats, write_errors);
         return rc;
     }
@@ -378,8 +378,8 @@ lis2dw12_i2c_readlen(struct sensor_itf *itf, uint8_t reg, uint8_t *buffer,
                           MYNEWT_VAL(LIS2DW12_I2C_RETRIES));
 
     if (rc) {
-        LIS2DW12_LOG(ERROR, "Failed to read from 0x%02X:0x%02X\n",
-                     itf->si_addr, reg);
+        LIS2DW12_LOG(ERROR, "I2C %u read failed at address 0x%02X:0x%02X\n",
+                     itf->si_num, itf->si_addr, reg);
         STATS_INC(g_lis2dw12stats, read_errors);
     }
 
diff --git a/hw/hal/include/hal/hal_i2c.h b/hw/hal/include/hal/hal_i2c.h
index 50fd1df..0d1173e 100644
--- a/hw/hal/include/hal/hal_i2c.h
+++ b/hw/hal/include/hal/hal_i2c.h
@@ -64,30 +64,40 @@ extern "C" {
 
 /*** I2C status codes (0=success). */
 
-/** Unknown error. */
+/* Unknown error. */
 #define HAL_I2C_ERR_UNKNOWN             1
 
-/** Invalid argument. */
+/* Invalid argument. */
 #define HAL_I2C_ERR_INVAL               2
 
-/** MCU failed to report result of I2C operation. */
+/* MCU failed to report result of I2C operation. */
 #define HAL_I2C_ERR_TIMEOUT             3
 
-/** Slave responded to address with NACK. */
+/* Slave responded to address with NACK. */
 #define HAL_I2C_ERR_ADDR_NACK           4
 
-/** Slave responded to data byte with NACK. */
+/* Slave responded to data byte with NACK. */
 #define HAL_I2C_ERR_DATA_NACK           5
 
-/** Overrun error */
+/* Overrun error */
 #define HAL_I2C_ERR_OVERRUN             6
 
-/** I2C bus in suspended state, but last op was 0 */
+/* I2C bus in suspended state, but last op was 0 */
 #define HAL_I2C_ERR_SUSPEND             7
 
-/** I2C bus in stopped state, but last op was 1   */
+/* I2C bus in stopped state, but last op was 1   */
 #define HAL_I2C_ERR_STOP                8
 
+/* At boot up or on facing an error I2C bus in
+ * suspended state
+ */
+#define HAL_I2C_ERR_INIT_SUSPEND        9
+
+/* At boot up or on facing an error I2C bus in
+ * suspended state
+ */
+#define HAL_I2C_ERR_INIT_STOP          10
+
 /**
  * When sending a packet, use this structure to pass the arguments.
  */
diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index 10e59d7..133e330 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -33,18 +33,6 @@
 #define I2C_WRITE 0
 #define I2C_READ  1
 
-#define WAIT_FOR_EVT(__r, __e, __op)\
-    while (!(__r)->EVENTS_####__e) {\
-        console_printf("while op: %u macro: STOPPED: %lu SUSPENDED: %lu LASTTX: %lu LASTRX: %lu\n",\
-                   __op, regs->EVENTS_STOPPED, regs->EVENTS_SUSPENDED, regs->EVENTS_LASTTX,\
-                   regs->EVENTS_LASTRX);\
-        now = os_time_get();\
-        if (OS_TIME_TICK_GT(now, abs_timo)) {\
-            rc = HAL_I2C_ERR_TIMEOUT;\
-            goto err;\
-        }\
-    }
-
 #define NRF52_HAL_I2C_MAX (2)
 
 #define NRF52_SCL_PIN_CONF                                              \
@@ -70,6 +58,7 @@ struct nrf52_hal_i2c {
      * transaction actually put the I2C bus in the suspended state
      */
     int nhi_prev_last_op;
+    uint32_t nhi_freq;
 };
 
 #if MYNEWT_VAL(I2C_0)
@@ -322,7 +311,13 @@ hal_i2c_init(uint8_t i2c_num, void *usercfg)
 
     regs->PSEL.SCL = cfg->scl_pin;
     regs->PSEL.SDA = cfg->sda_pin;
+#if MYNEWT_VAL(NRF52_HANDLE_ANOMALY_109)
+    i2c->nhi_freq = freq;
+    regs->FREQUENCY = 0;
+#else
     regs->FREQUENCY = freq;
+#endif
+    regs->ADDRESS = 0;
     regs->ENABLE = TWIM_ENABLE_ENABLE_Enabled;
     i2c->nhi_prev_last_op = -1;
 
@@ -331,64 +326,79 @@ err:
     return (rc);
 }
 
+static void
+hal_i2c_handle_anomaly_109(NRF_TWIM_Type *regs, uint32_t prev_freq, uint8_t address)
+{
+    if (regs->FREQUENCY == 0) {
+        regs->ENABLE = TWIM_ENABLE_ENABLE_Disabled;
+        regs->ENABLE = TWIM_ENABLE_ENABLE_Enabled;
+        regs->ADDRESS = address;
+        regs->FREQUENCY = prev_freq;
+        regs->TASKS_STARTTX = 1;
+    }
+}
+
 /*
  * Starts an I2C transaction either read or write using EasyDMA/TWIM
  */
 static int
 hal_i2c_handle_transact_start(struct nrf52_hal_i2c *i2c, uint8_t op,
-                              uint32_t abs_timo)
+                              os_time_t abs_timo, uint8_t address)
 {
     NRF_TWIM_Type *regs;
-    uint32_t now;
+    os_time_t now;
     int rc;
 
     regs = i2c->nhi_regs;
 
     regs->EVENTS_ERROR = 0;
+    regs->EVENTS_STOPPED = 0;
 
     /* If a transaction was previously suspended, resume it */
     if (regs->EVENTS_SUSPENDED) {
         regs->TASKS_RESUME = 1;
+        regs->EVENTS_SUSPENDED = 0;
+        return 0;
     }
 
-    regs->EVENTS_STOPPED = 0;
-    regs->EVENTS_SUSPENDED = 0;
     if (op == I2C_WRITE) {
         /* Start an I2C transmit transaction */
         regs->TASKS_STARTTX = 1;
-        WAIT_FOR_EVT(regs, TXSTARTED, op);
+
+#if MYNEWT_VAL(NRF52_HANDLE_ANOMALY_109)
+        while (!regs->EVENTS_TXSTARTED) {
+            now = os_time_get();
+            if (OS_TIME_TICK_GT(now, abs_timo)) {
+                rc = HAL_I2C_ERR_TIMEOUT;
+                return rc;
+            }
+        }
+
         regs->EVENTS_TXSTARTED = 0;
+        if (!regs->FREQUENCY) {
+            hal_i2c_handle_anomaly_109(regs, i2c->nhi_freq, address);
+            regs->EVENTS_TXSTARTED = 0;
+        }
+#endif
     } else {
+        if (!regs->FREQUENCY) {
+            regs->FREQUENCY = i2c->nhi_freq;
+        }
         /* Start an I2C receive transaction */
         regs->TASKS_STARTRX = 1;
-        WAIT_FOR_EVT(regs, RXSTARTED, op);
-        regs->EVENTS_RXSTARTED = 0;
     }
 
     return 0;
-err:
-    return rc;
 }
 
 static int
 hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
-                            uint32_t abs_timo, uint8_t last_op)
+                            os_time_t abs_timo, uint8_t last_op)
 {
     int rc;
     uint32_t evt;
     os_time_t now;
 
-    if (last_op) {
-        /* this is to take care of clock stretches as per the datasheet */
-        if (regs->EVENTS_SUSPENDED) {
-            /* If there is a clock stretch, the event would be suspended for
-             * that TWIM interface, it would need to be resumed to finish the
-             * transaction
-             */
-            regs->TASKS_RESUME = 1;
-        }
-    }
-
     while(1) {
         /*
          * Use last_op as the determining factor for the type of event to be
@@ -402,8 +412,11 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
 
         if (evt) {
             if (&evt == &regs->EVENTS_STOPPED) {
-                regs->EVENTS_LASTTX = 0;
-                regs->EVENTS_LASTRX = 0;
+#if MYNEWT_VAL(NRF52_HANDLE_ANOMALY_109)
+                if (regs->FREQUENCY) {
+                    regs->FREQUENCY = 0;
+                }
+#endif
             }
             break;
         }
@@ -412,11 +425,6 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
             goto err;
         }
 
-        console_printf("while end: last_op: %u STOPPED: %lu SUSPENDED: %lu"
-                       "LASTTX: %lu LASTRX: %lu\n", last_op, regs->EVENTS_STOPPED,
-                       regs->EVENTS_SUSPENDED, regs->EVENTS_LASTTX,
-                       regs->EVENTS_LASTRX);
-
         now = os_time_get();
         if (OS_TIME_TICK_GT(now, abs_timo)) {
             rc = HAL_I2C_ERR_TIMEOUT;
@@ -445,8 +453,7 @@ hal_i2c_bus_error_detect(struct nrf52_hal_i2c *i2c)
         rc = HAL_I2C_ERR_SUSPEND;
         goto err;
     } else if ((i2c->nhi_prev_last_op == 1 &&
-               !regs->EVENTS_STOPPED) && regs->EVENTS_SUSPENDED) {
-
+               (!regs->EVENTS_STOPPED || regs->EVENTS_SUSPENDED))) {
         /*
          * return HAL_I2C_ERR_STOP if previous last_op was 1 and
          * the bus is not in the stopped state or in initial state
@@ -454,20 +461,29 @@ hal_i2c_bus_error_detect(struct nrf52_hal_i2c *i2c)
          */
         rc = HAL_I2C_ERR_STOP;
         goto err;
+    } else if (i2c->nhi_prev_last_op == -1 &&
+               (regs->EVENTS_STOPPED || regs->EVENTS_SUSPENDED)) {
+        if (regs->EVENTS_STOPPED) {
+            rc = HAL_I2C_ERR_INIT_STOP;
+        } else {
+            rc = HAL_I2C_ERR_INIT_SUSPEND;
+        }
+        goto err;
     }
 
     return 0;
 err:
-    console_printf("i2c failure: rc:%u evt_stopped: %lu evt_suspended: %lu\n",
-                   rc, regs->EVENTS_STOPPED, regs->EVENTS_SUSPENDED);
     return rc;
 }
 
 /* Handle errors returned from the TWIM peripheral along with timeouts */
 static int
-hal_i2c_handle_errors(NRF_TWIM_Type *regs, int rc)
+hal_i2c_handle_errors(struct nrf52_hal_i2c *i2c, int rc, os_time_t abs_timo)
 {
     int nrf_status;
+    NRF_TWIM_Type *regs;
+
+    regs = i2c->nhi_regs;
 
     regs->TASKS_RESUME = 1;
     regs->TASKS_STOP = 1;
@@ -487,6 +503,8 @@ hal_i2c_handle_errors(NRF_TWIM_Type *regs, int rc)
         regs->ENABLE = TWIM_ENABLE_ENABLE_Disabled;
         hal_i2c_clear_bus(regs->PSEL.SCL, regs->PSEL.SDA);
         regs->ENABLE = TWIM_ENABLE_ENABLE_Enabled;
+        i2c->nhi_prev_last_op = -1;
+        regs->EVENTS_STOPPED = 0;
     }
 
     return rc;
@@ -548,7 +566,7 @@ hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
     }
 
     /* Starts an I2C transaction using TWIM/EasyDMA */
-    rc = hal_i2c_handle_transact_start(i2c, I2C_WRITE, start + timo);
+    rc = hal_i2c_handle_transact_start(i2c, I2C_WRITE, start + timo, pdata->address);
     if (rc) {
         goto err;
     }
@@ -564,7 +582,7 @@ hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
 
     return 0;
 err:
-    return hal_i2c_handle_errors(regs, rc);
+    return hal_i2c_handle_errors(i2c, rc, start + timo);
 }
 
 int
@@ -613,7 +631,7 @@ hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
     }
 
     /* Starts an I2C transaction using TWIM/EasyDMA */
-    rc = hal_i2c_handle_transact_start(i2c, I2C_READ, start + timo);
+    rc = hal_i2c_handle_transact_start(i2c, I2C_READ, start + timo, pdata->address);
     if (rc) {
         goto err;
     }
@@ -629,7 +647,7 @@ hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
 
     return 0;
 err:
-    return hal_i2c_handle_errors(regs, rc);
+    return hal_i2c_handle_errors(i2c, rc, start + timo);
 }
 
 int
diff --git a/hw/mcu/nordic/nrf52xxx/syscfg.yml b/hw/mcu/nordic/nrf52xxx/syscfg.yml
index 2295677..acd907f 100644
--- a/hw/mcu/nordic/nrf52xxx/syscfg.yml
+++ b/hw/mcu/nordic/nrf52xxx/syscfg.yml
@@ -392,6 +392,10 @@ syscfg.defs:
             - "!XTAL_RC"
             - "!XTAL_32768"
 
+    NRF52_HANDLE_ANOMALY_109:
+        description: 'TWIM NRF52XXX anomaly 109 fix'
+        value: 1
+
 syscfg.restrictions:
     - "MCU_NRF52832 ^^ MCU_NRF52840"
     - "!I2C_0 || (I2C_0_PIN_SCL && I2C_0_PIN_SDA)"


[mynewt-core] 07/07: Add new API for double buffered I2C

Posted by vi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vipulrahane pushed a commit to branch dma_i2c_new_api
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit b9fd22e9c34cca5eed58ebf148415e5e2a341d62
Author: Vipul Rahane <vi...@runtime.io>
AuthorDate: Mon Nov 19 09:07:02 2018 -0800

    Add new API for double buffered I2C
---
 hw/drivers/led/lp5523/src/lp5523.c         |  27 ++++++-
 hw/drivers/led/lp5523/syscfg.yml           |   4 +-
 hw/drivers/sensors/lis2dh12/src/lis2dh12.c |  22 ++++-
 hw/drivers/sensors/lis2dh12/syscfg.yml     |   2 +-
 hw/drivers/sensors/lps33hw/src/lps33hw.c   |  21 ++++-
 hw/drivers/sensors/lps33hw/syscfg.yml      |   4 +-
 hw/drivers/sensors/lps33thw/src/lps33thw.c |  19 ++++-
 hw/drivers/sensors/lps33thw/syscfg.yml     |   2 +-
 hw/drivers/sensors/ms5840/src/ms5840.c     |  21 ++++-
 hw/drivers/sensors/ms5840/syscfg.yml       |   2 +-
 hw/hal/include/hal/hal_i2c.h               |  20 ++++-
 hw/hal/syscfg.yml                          |   4 +
 hw/mcu/nordic/nrf52xxx/src/hal_i2c.c       | 124 ++++++++++++++++++++++++++---
 hw/mcu/nordic/nrf52xxx/syscfg.yml          |   1 +
 hw/util/i2cn/include/i2cn/i2cn.h           |   4 +
 hw/util/i2cn/src/i2cn.c                    |  26 ++++++
 16 files changed, 267 insertions(+), 36 deletions(-)

diff --git a/hw/drivers/led/lp5523/src/lp5523.c b/hw/drivers/led/lp5523/src/lp5523.c
index 894a5fc..6e0bdb4 100644
--- a/hw/drivers/led/lp5523/src/lp5523.c
+++ b/hw/drivers/led/lp5523/src/lp5523.c
@@ -31,12 +31,14 @@
 STATS_SECT_START(lp5523_stat_section)
     STATS_SECT_ENTRY(read_errors)
     STATS_SECT_ENTRY(write_errors)
+    STATS_SECT_ENTRY(write_read_errors)
 STATS_SECT_END
 
 /* Define stat names for querying */
 STATS_NAME_START(lp5523_stat_section)
     STATS_NAME(lp5523_stat_section, read_errors)
     STATS_NAME(lp5523_stat_section, write_errors)
+    STATS_NAME(lp5523_stat_section, write_read_errors)
 STATS_NAME_END(lp5523_stat_section)
 
 /* Global variable used to hold stats data */
@@ -87,8 +89,10 @@ lp5523_get_reg(struct led_itf *itf, enum lp5523_registers addr,
 
     struct hal_i2c_master_data data_struct = {
         .address = itf->li_addr,
-        .len = 1,
-        .buffer = &addr
+        .len1 = 1,
+        .buffer1 = &addr,
+        .len2 = 1,
+        .buffer2 = value
     };
 
     rc = led_itf_lock(itf, MYNEWT_VAL(LP5523_ITF_LOCK_TMO));
@@ -96,6 +100,7 @@ lp5523_get_reg(struct led_itf *itf, enum lp5523_registers addr,
         return rc;
     }
 
+#if 0
     /* Register write */
     rc = i2cn_master_write(itf->li_num, &data_struct, MYNEWT_VAL(LP5523_I2C_TIMEOUT_TICKS), 0,
                            MYNEWT_VAL(LP5523_I2C_RETRIES));
@@ -117,6 +122,16 @@ lp5523_get_reg(struct led_itf *itf, enum lp5523_registers addr,
                     itf->li_addr, addr);
          STATS_INC(g_lp5523stats, read_errors);
     }
+#endif
+    rc = i2cn_master_write_read(itf->li_num, &data_struct, MYNEWT_VAL(LP5523_I2C_TIMEOUT_TICKS), 0,
+                                MYNEWT_VAL(LP5523_I2C_RETRIES));
+
+    if (rc) {
+        LP5523_LOG(ERROR, "I2C wr failed at address 0x%02X\n",
+                   itf->li_addr);
+        STATS_INC(g_lp5523stats, write_read_errors);
+        goto err;
+    }
 
 err:
     led_itf_unlock(itf);
@@ -554,7 +569,13 @@ lp5523_get_engine_int(struct led_itf *itf, uint8_t engine, uint8_t *flag)
 int
 lp5523_reset(struct led_itf *itf)
 {
-    return lp5523_set_reg(itf, LP5523_RESET, 0xff);
+    int rc;
+
+    rc = lp5523_set_reg(itf, LP5523_RESET, 0xff);
+
+    os_time_delay(1);
+
+    return rc;
 }
 
 int
diff --git a/hw/drivers/led/lp5523/syscfg.yml b/hw/drivers/led/lp5523/syscfg.yml
index 49a64c3..bea4c2c 100644
--- a/hw/drivers/led/lp5523/syscfg.yml
+++ b/hw/drivers/led/lp5523/syscfg.yml
@@ -43,7 +43,7 @@ syscfg.defs:
         description: >
             Number of retries to use for failed I2C communication.  A retry is
             used when the LP5523 sends an unexpected NACK.
-        value: 2
+        value: 0
     LP5523_STARTUP_SEQ_DELAY:
         description: >
             Startup sequence delay for chip to finish start sequence
@@ -52,4 +52,4 @@ syscfg.defs:
     LP5523_I2C_TIMEOUT_TICKS:
         description: >
             Number of OS ticks to wait for each I2C transaction to complete.
-        value: 3
+        value: 10
diff --git a/hw/drivers/sensors/lis2dh12/src/lis2dh12.c b/hw/drivers/sensors/lis2dh12/src/lis2dh12.c
index fcc272a..3558f35 100644
--- a/hw/drivers/sensors/lis2dh12/src/lis2dh12.c
+++ b/hw/drivers/sensors/lis2dh12/src/lis2dh12.c
@@ -152,6 +152,7 @@ STATS_SECT_START(lis2dh12_stat_section)
     STATS_SECT_ENTRY(orient_chg_y_notify)
     STATS_SECT_ENTRY(orient_chg_z_notify)
 #endif
+    STATS_SECT_ENTRY(write_read_errors)
 STATS_SECT_END
 
 /* Define stat names for querying */
@@ -170,6 +171,7 @@ STATS_NAME_START(lis2dh12_stat_section)
     STATS_NAME(lis2dh12_stat_section, orient_chg_y_notify)
     STATS_NAME(lis2dh12_stat_section, orient_chg_z_notify)
 #endif
+    STATS_NAME(lis2dh12_stat_section, write_read_errors)
 STATS_NAME_END(lis2dh12_stat_section)
 
 /* Global variable used to hold stats data */
@@ -227,24 +229,27 @@ lis2dh12_i2c_readlen(struct sensor_itf *itf, uint8_t addr, uint8_t *buffer,
                      uint8_t len)
 {
     int rc;
+
     if (len > 1)
     {
         addr |= 0x80;
     }
 
-    uint8_t payload[20] = { addr, 0, 0, 0, 0, 0, 0, 0,
+    uint8_t payload[20] = {   0, 0, 0, 0, 0, 0, 0, 0,
                               0, 0, 0, 0, 0, 0, 0, 0,
                               0, 0, 0, 0};
 
     struct hal_i2c_master_data data_struct = {
         .address = itf->si_addr,
-        .len = 1,
-        .buffer = payload
+        .len1 = 1,
+        .buffer1 = &addr,
+        .len2 = len,
+        .buffer2 = payload
     };
 
     /* Clear the supplied buffer */
     memset(buffer, 0, len);
-
+#if 0
     /* Register write */
     rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(LIS2DH12_I2C_TIMEOUT_TICKS), 1,
                            MYNEWT_VAL(LIS2DH12_I2C_RETRIES));
@@ -266,6 +271,15 @@ lis2dh12_i2c_readlen(struct sensor_itf *itf, uint8_t addr, uint8_t *buffer,
         STATS_INC(g_lis2dh12stats, read_errors);
         goto err;
     }
+#endif
+    rc = i2cn_master_write_read(itf->si_num, &data_struct, MYNEWT_VAL(LIS2DH12_I2C_TIMEOUT_TICKS) * (len + 1), 1,
+                                MYNEWT_VAL(LIS2DH12_I2C_RETRIES));
+    if (rc) {
+        LIS2DH12_LOG(ERROR, "Failed to read from 0x%02X:0x%02X\n",
+                     data_struct.address, addr);
+        STATS_INC(g_lis2dh12stats, write_read_errors);
+        goto err;
+    }
 
     /* Copy the I2C results into the supplied buffer */
     memcpy(buffer, payload, len);
diff --git a/hw/drivers/sensors/lis2dh12/syscfg.yml b/hw/drivers/sensors/lis2dh12/syscfg.yml
index 72cd520..d2251f0 100644
--- a/hw/drivers/sensors/lis2dh12/syscfg.yml
+++ b/hw/drivers/sensors/lis2dh12/syscfg.yml
@@ -28,7 +28,7 @@ syscfg.defs:
         description: >
             Number of retries to use for failed I2C communication.  A retry is
             used when the LIS2DH12 sends an unexpected NACK.
-        value: 2
+        value: 0
     LIS2DH12_I2C_TIMEOUT_TICKS:
         description: >
             Number of OS ticks to wait for each I2C transaction to complete.
diff --git a/hw/drivers/sensors/lps33hw/src/lps33hw.c b/hw/drivers/sensors/lps33hw/src/lps33hw.c
index 62706b5..dd096c7 100644
--- a/hw/drivers/sensors/lps33hw/src/lps33hw.c
+++ b/hw/drivers/sensors/lps33hw/src/lps33hw.c
@@ -47,12 +47,14 @@ static struct hal_spi_settings spi_lps33hw_settings = {
 STATS_SECT_START(lps33hw_stat_section)
     STATS_SECT_ENTRY(read_errors)
     STATS_SECT_ENTRY(write_errors)
+    STATS_SECT_ENTRY(write_read_errors)
 STATS_SECT_END
 
 /* Define stat names for querying */
 STATS_NAME_START(lps33hw_stat_section)
     STATS_NAME(lps33hw_stat_section, read_errors)
     STATS_NAME(lps33hw_stat_section, write_errors)
+    STATS_NAME(lps33hw_stat_section, write_read_errors)
 STATS_NAME_END(lps33hw_stat_section)
 
 /* Global variable used to hold stats data */
@@ -183,7 +185,7 @@ lps33hw_i2c_set_reg(struct sensor_itf *itf, uint8_t reg, uint8_t value)
         LPS33HW_LOG(ERROR,
                     "Failed to write to 0x%02X:0x%02X with value 0x%02X\n",
                     itf->si_addr, reg, value);
-        STATS_INC(g_lps33hwstats, read_errors);
+        STATS_INC(g_lps33hwstats, write_errors);
     }
 
     return rc;
@@ -342,10 +344,13 @@ lps33hw_i2c_get_regs(struct sensor_itf *itf, uint8_t reg, uint8_t size,
 
     struct hal_i2c_master_data data_struct = {
         .address = itf->si_addr,
-        .len = 1,
-        .buffer = &reg
+        .len1 = 1,
+        .buffer1 = &reg,
+        .len2 = size,
+        .buffer2 = buffer
     };
 
+#if 0
     /* Register write */
     rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(LPS33HW_I2C_TIMEOUT_TICKS), 0,
                            MYNEWT_VAL(LPS33HW_I2C_RETRIES));
@@ -368,6 +373,16 @@ lps33hw_i2c_get_regs(struct sensor_itf *itf, uint8_t reg, uint8_t size,
                     itf->si_addr, reg);
         STATS_INC(g_lps33hwstats, read_errors);
     }
+#endif
+    rc = i2cn_master_write_read(itf->si_num, &data_struct,
+                                (MYNEWT_VAL(LPS33HW_I2C_TIMEOUT_TICKS)) * (size + 1), 1,
+                                MYNEWT_VAL(LPS33HW_I2C_RETRIES));
+
+    if (rc) {
+        LPS33HW_LOG(ERROR, "Failed to write-read from 0x%02X:0x%02X\n",
+                    itf->si_addr, reg);
+        STATS_INC(g_lps33hwstats, write_read_errors);
+    }
     return rc;
 }
 
diff --git a/hw/drivers/sensors/lps33hw/syscfg.yml b/hw/drivers/sensors/lps33hw/syscfg.yml
index 12f5a63..89eaa8e 100644
--- a/hw/drivers/sensors/lps33hw/syscfg.yml
+++ b/hw/drivers/sensors/lps33hw/syscfg.yml
@@ -42,9 +42,9 @@ syscfg.defs:
         description: >
             Number of retries to use for failed I2C communication.  A retry is
             used when the LPS33HW sends an unexpected NACK.
-        value: 2
+        value: 0
     LPS33HW_I2C_TIMEOUT_TICKS:
         description: >
             Number of OS ticks to wait for each I2C transaction to complete.
         value: 3
- 
\ No newline at end of file
+ 
diff --git a/hw/drivers/sensors/lps33thw/src/lps33thw.c b/hw/drivers/sensors/lps33thw/src/lps33thw.c
index 0d38500..1b368d3 100644
--- a/hw/drivers/sensors/lps33thw/src/lps33thw.c
+++ b/hw/drivers/sensors/lps33thw/src/lps33thw.c
@@ -342,10 +342,12 @@ lps33thw_i2c_get_regs(struct sensor_itf *itf, uint8_t reg, uint8_t size,
 
     struct hal_i2c_master_data data_struct = {
         .address = itf->si_addr,
-        .len = 1,
-        .buffer = &reg
+        .len1 = 1,
+        .buffer1 = &reg,
+        .len2 = size,
+        .buffer2 = buffer
     };
-
+#if 0
     /* Register write */
     rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(LPS33THW_I2C_TIMEOUT_TICKS), 0,
                            MYNEWT_VAL(LPS33THW_I2C_RETRIES));
@@ -368,6 +370,17 @@ lps33thw_i2c_get_regs(struct sensor_itf *itf, uint8_t reg, uint8_t size,
                     itf->si_addr, reg);
         STATS_INC(g_lps33thwstats, read_errors);
     }
+#endif
+    rc = i2cn_master_write_read(itf->si_num, &data_struct,
+                                (MYNEWT_VAL(LPS33THW_I2C_TIMEOUT_TICKS)) * (size + 1), 1,
+                                MYNEWT_VAL(LPS33THW_I2C_RETRIES));
+
+    if (rc) {
+        LPS33THW_LOG(ERROR, "Failed to write-read from 0x%02X:0x%02X\n",
+                    itf->si_addr, reg);
+        STATS_INC(g_lps33thwstats, read_errors);
+    }
+
     return rc;
 }
 
diff --git a/hw/drivers/sensors/lps33thw/syscfg.yml b/hw/drivers/sensors/lps33thw/syscfg.yml
index ba1aa29..46773d5 100644
--- a/hw/drivers/sensors/lps33thw/syscfg.yml
+++ b/hw/drivers/sensors/lps33thw/syscfg.yml
@@ -42,7 +42,7 @@ syscfg.defs:
         description: >
             Number of retries to use for failed I2C communication.  A retry is
             used when the LPS33THW sends an unexpected NACK.
-        value: 2
+        value: 0
     LPS33THW_I2C_TIMEOUT_TICKS:
         description: >
             Number of OS ticks to wait for each I2C transaction to complete.
diff --git a/hw/drivers/sensors/ms5840/src/ms5840.c b/hw/drivers/sensors/ms5840/src/ms5840.c
index ab421de..dac0321 100644
--- a/hw/drivers/sensors/ms5840/src/ms5840.c
+++ b/hw/drivers/sensors/ms5840/src/ms5840.c
@@ -51,6 +51,7 @@ static uint16_t cnv_time[6] = {
 STATS_SECT_START(ms5840_stat_section)
     STATS_SECT_ENTRY(read_errors)
     STATS_SECT_ENTRY(write_errors)
+    STATS_SECT_ENTRY(write_read_errors)
     STATS_SECT_ENTRY(eeprom_crc_errors)
 STATS_SECT_END
 
@@ -58,6 +59,7 @@ STATS_SECT_END
 STATS_NAME_START(ms5840_stat_section)
     STATS_NAME(ms5840_stat_section, read_errors)
     STATS_NAME(ms5840_stat_section, write_errors)
+    STATS_NAME(ms5840_stat_section, write_read_errors)
     STATS_NAME(ms5840_stat_section, eeprom_crc_errors)
 STATS_NAME_END(ms5840_stat_section)
 
@@ -365,12 +367,14 @@ ms5840_readlen(struct sensor_itf *itf, uint8_t addr, uint8_t *buffer,
                uint8_t len)
 {
     int rc;
-    uint8_t payload[3] = {addr, 0, 0};
+    uint8_t payload[3] = {0};
 
     struct hal_i2c_master_data data_struct = {
         .address = itf->si_addr,
-        .len = 1,
-        .buffer = payload
+        .len1 = 1,
+        .buffer1 = &addr,
+        .len2 = len,
+        .buffer2 = payload,
     };
 
     /* Clear the supplied buffer */
@@ -380,7 +384,7 @@ ms5840_readlen(struct sensor_itf *itf, uint8_t addr, uint8_t *buffer,
     if (rc) {
         return rc;
     }
-
+#if 0
     /* Command write */
     rc = i2cn_master_write(itf->si_num, &data_struct, MYNEWT_VAL(MS5840_I2C_TIMEOUT_TICKS), 1,
                            MYNEWT_VAL(MS5840_I2C_RETRIES));
@@ -402,6 +406,15 @@ ms5840_readlen(struct sensor_itf *itf, uint8_t addr, uint8_t *buffer,
         STATS_INC(g_ms5840stats, read_errors);
         goto err;
     }
+#endif
+    rc = i2cn_master_write_read(itf->si_num, &data_struct, MYNEWT_VAL(MS5840_I2C_TIMEOUT_TICKS) * (len + 1), 1,
+                                MYNEWT_VAL(MS5840_I2C_RETRIES));
+    if (rc) {
+        MS5840_LOG(ERROR, "Failed to write-read from 0x%02X:0x%02X\n",
+                   data_struct.address, addr);
+        STATS_INC(g_ms5840stats, write_read_errors);
+        goto err;
+    }
 
     /* Copy the I2C results into the supplied buffer */
     memcpy(buffer, payload, len);
diff --git a/hw/drivers/sensors/ms5840/syscfg.yml b/hw/drivers/sensors/ms5840/syscfg.yml
index 7a405ea..8c3f76d 100644
--- a/hw/drivers/sensors/ms5840/syscfg.yml
+++ b/hw/drivers/sensors/ms5840/syscfg.yml
@@ -27,7 +27,7 @@ syscfg.defs:
         description: >
             Number of retries to use for failed I2C communication.  A retry is
             used when the MS5840 sends an unexpected NACK.
-        value: 2
+        value: 0
     MS5840_I2C_TIMEOUT_TICKS:
         description: >
             Number of OS ticks to wait for each I2C transaction to complete.
diff --git a/hw/hal/include/hal/hal_i2c.h b/hw/hal/include/hal/hal_i2c.h
index 0d1173e..a794621 100644
--- a/hw/hal/include/hal/hal_i2c.h
+++ b/hw/hal/include/hal/hal_i2c.h
@@ -114,10 +114,25 @@ struct hal_i2c_master_data {
      * only the top 7-bits to this function as 0x40
      */
     uint8_t  address;
-    /** Number of buffer bytes to transmit or receive */
+//#if MYNEWT_VAL(HAL_I2C_DOUBLE_BUFFERED)
+    /** Number of buffer bytes to transmit or receive using buffer 1*/
     uint16_t len;
+    /** Number of buffer bytes to transmit or receive using buffer 2*/
+    uint16_t len1;
+//#else
+    /** Number of buffer bytes to transmit or receive */
+    uint16_t len2;
+//#endif
+
+//#if MYNEWT_VAL(HAL_I2C_DOUBLE_BUFFERED)
+    /** Buffer space to hold the transmit or receive */
+    uint8_t *buffer1;
+    /** Buffer space to hold the transmit or receive */
+    uint8_t *buffer2;
+//#else
     /** Buffer space to hold the transmit or receive */
     uint8_t *buffer;
+//#endif
 };
 
 /**
@@ -180,6 +195,9 @@ int hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
  */
 int hal_i2c_master_probe(uint8_t i2c_num, uint8_t address,
                          uint32_t timeout);
+int
+hal_i2c_master_write_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
+                          uint32_t timo, uint8_t last_op);
 
 #ifdef __cplusplus
 }
diff --git a/hw/hal/syscfg.yml b/hw/hal/syscfg.yml
index f49dff1..4333534 100644
--- a/hw/hal/syscfg.yml
+++ b/hw/hal/syscfg.yml
@@ -33,6 +33,10 @@ syscfg.defs:
             buffer of this size is allocated on the stack during verify
             operations.
         value: 16
+    HAL_I2C_DOUBLE_BUFFERED:
+        description: >
+            Specify whether the HAL uses double buffered implementation
+        value: 0
 
 syscfg.vals.OS_DEBUG_MODE:
     HAL_FLASH_VERIFY_WRITES: 1
diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index c2c5e71..b792318 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -399,19 +399,34 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
 {
     int rc;
     volatile uint32_t *evt;
-    os_time_t now = 0;
+    volatile os_time_t now = 0;
 
-    while(1) {
-        /*
-         * Use last_op as the determining factor for the type of event to be
-         * monitored
-         */
-        if (last_op) {
-            evt = &regs->EVENTS_STOPPED;
-        } else {
-            evt = &regs->EVENTS_SUSPENDED;
+    /*
+     * Use last_op as the determining factor for the type of event to be
+     * monitored
+     */
+    if (last_op) {
+        evt = &regs->EVENTS_STOPPED;
+    } else {
+        evt = &regs->EVENTS_SUSPENDED;
+    }
+
+    /* since there is no SUSPEND short for RX, we have to specifically
+     * deal with it
+     */
+    if (op == I2C_READ && !last_op) {
+        while (!regs->EVENTS_LASTRX) {
+            now = os_time_get();
+            if (OS_TIME_TICK_GT(now, abs_timo)) {
+                rc = HAL_I2C_ERR_TIMEOUT;
+                goto err;
+            }
         }
 
+        regs->TASKS_SUSPEND = 1;
+    }
+
+    while(1) {
         if (*evt) {
             if (evt == &regs->EVENTS_STOPPED) {
 #if MYNEWT_VAL(NRF52_HANDLE_ANOMALY_109)
@@ -433,10 +448,11 @@ hal_i2c_handle_transact_end(NRF_TWIM_Type *regs, uint8_t op, uint32_t start,
             goto err;
         }
     }
+    regs->TASKS_RESUME  = 1;
 
     g_start = os_cputime_get32() - g_start;
 
-    console_printf("t:%lu r:%u\n", g_start, op);
+    //console_printf("t:%lu r:%u\n", g_start, op);
 
     return 0;
 err:
@@ -638,6 +654,8 @@ hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
      */
     if (last_op) {
         regs->SHORTS = TWIM_SHORTS_LASTRX_STOP_Msk;
+    } else {
+        regs->SHORTS = 0;
     }
 
     /* Starts an I2C transaction using TWIM/EasyDMA */
@@ -661,6 +679,90 @@ err:
 }
 
 int
+hal_i2c_master_write_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
+                          uint32_t timo, uint8_t last_op)
+{
+    int rc;
+    os_time_t now;
+    uint32_t start;
+    NRF_TWIM_Type *regs;
+    struct nrf52_hal_i2c *i2c;
+
+    start = os_time_get();
+    g_start = os_cputime_get32();
+
+    /* Resolve the I2C bus */
+    rc = hal_i2c_resolve(i2c_num, &i2c);
+    if (rc != 0) {
+        return rc;
+    }
+
+    regs = i2c->nhi_regs;
+
+    /* Detect errors on the bus based on the previous and current
+     * condition of the bus
+     */
+    rc = hal_i2c_bus_error_detect(i2c);
+    if (rc) {
+        goto err;
+    }
+
+    /* Configure the RXD registers for EasyDMA access to work with buffers of
+     * specific length and address of the slave
+     */
+    regs->ADDRESS    = pdata->address;
+    regs->TXD.MAXCNT = pdata->len1;
+    regs->TXD.PTR    = (uint32_t)pdata->buffer1;
+    regs->TXD.LIST   = 0;
+    /* Disable and clear interrupts */
+    regs->INTENCLR   = NRF_TWIM_ALL_INTS_MASK;
+    regs->INTEN      = 0;
+    regs->SHORTS = TWIM_SHORTS_LASTTX_STARTRX_Msk;
+
+    /* Starts an I2C transaction using TWIM/EasyDMA */
+    rc = hal_i2c_handle_transact_start(i2c, I2C_WRITE, start + timo, pdata->address);
+    if (rc) {
+        goto err;
+    }
+
+    while (!regs->EVENTS_LASTTX) {
+        now = os_time_get();
+        if (OS_TIME_TICK_GT(now, start + timo)) {
+            rc = HAL_I2C_ERR_TIMEOUT;
+            return rc;
+        }
+    }
+
+    regs->RXD.MAXCNT = pdata->len2;
+    regs->RXD.PTR    = (uint32_t)pdata->buffer2;
+    regs->RXD.LIST   = 0;
+    /* Only set short for RX->STOP for last_op:1 since there is no suspend short
+     * available in nrf52832
+     */
+    if (last_op) {
+        regs->SHORTS = TWIM_SHORTS_LASTRX_STOP_Msk;
+    } else {
+        regs->SHORTS = 0;
+    }
+
+    regs->TASKS_RESUME  = 1;
+
+    /* Ends an I2C transaction using TWIM/EasyDMA */
+    rc = hal_i2c_handle_transact_end(regs, I2C_READ, start, start + timo, last_op);
+    if (rc) {
+        goto err;
+    }
+
+    /* Save the current last op to detect bus errors later */
+    i2c->nhi_prev_last_op = last_op;
+
+    return 0;
+err:
+    return hal_i2c_handle_errors(i2c, rc, start + timo);
+}
+
+
+int
 hal_i2c_master_probe(uint8_t i2c_num, uint8_t address, uint32_t timo)
 {
     struct hal_i2c_master_data rx;
diff --git a/hw/mcu/nordic/nrf52xxx/syscfg.yml b/hw/mcu/nordic/nrf52xxx/syscfg.yml
index acd907f..98dde96 100644
--- a/hw/mcu/nordic/nrf52xxx/syscfg.yml
+++ b/hw/mcu/nordic/nrf52xxx/syscfg.yml
@@ -410,3 +410,4 @@ syscfg.restrictions:
     - "!SPI_3_SLAVE || (SPI_3_SLAVE_PIN_SCK && SPI_3_SLAVE_PIN_MOSI && SPI_3_SLAVE_PIN_MISO && SPI_3_SLAVE_PIN_SS)"
     - "!UART_0 || (UART_0_PIN_TX && UART_0_PIN_RX)"
     - "!UART_1 || (UART_1_PIN_TX && UART_1_PIN_RX)"
+    - "HAL_I2C_DOUBLE_BUFFERED"
diff --git a/hw/util/i2cn/include/i2cn/i2cn.h b/hw/util/i2cn/include/i2cn/i2cn.h
index d6caaae..f2b4851 100644
--- a/hw/util/i2cn/include/i2cn/i2cn.h
+++ b/hw/util/i2cn/include/i2cn/i2cn.h
@@ -64,6 +64,10 @@ int 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);
 
+int
+i2cn_master_write_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
+                       uint32_t timeout, uint8_t last_op, int retries);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/hw/util/i2cn/src/i2cn.c b/hw/util/i2cn/src/i2cn.c
index 2322e1f..536a8bd 100644
--- a/hw/util/i2cn/src/i2cn.c
+++ b/hw/util/i2cn/src/i2cn.c
@@ -19,6 +19,7 @@
 
 #include "hal/hal_i2c.h"
 #include "i2cn/i2cn.h"
+#include <console/console.h>
 
 int
 i2cn_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
@@ -37,6 +38,7 @@ i2cn_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
         if (rc == 0) {
             break;
         }
+        console_printf("r addr: %02x n: %u rc: %d\n", pdata->address, retries, rc);
     }
 
     return rc;
@@ -59,6 +61,30 @@ i2cn_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
         if (rc == 0) {
             break;
         }
+        console_printf("w addr: %02x n: %u rc: %d\n", pdata->address, retries, rc);
+    }
+
+    return rc;
+}
+
+int
+i2cn_master_write_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
+                       uint32_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_read(i2c_num, pdata, timeout, last_op);
+        if (rc == 0) {
+            break;
+        }
+        console_printf("wr addr: %02x n: %u rc: %d\n", pdata->address, retries, rc);
     }
 
     return rc;