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 2018/12/04 13:10:16 UTC

[GitHub] andrzej-kaczmarek closed pull request #1533: hw/mcu/nordic: Rework workaround for unresponsive TWI

andrzej-kaczmarek closed pull request #1533: hw/mcu/nordic: Rework workaround for unresponsive TWI
URL: https://github.com/apache/mynewt-core/pull/1533
 
 
   

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/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index 5340ee3ac4..9de73fd891 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -410,6 +410,69 @@ hal_i2c_config(uint8_t i2c_num, const struct hal_i2c_settings *cfg)
     return 0;
 }
 
+static inline void
+hal_i2c_trigger_start(NRF_TWI_Type *twi, __O uint32_t *task)
+{
+    uint32_t end_ticks;
+    int retry = 2;
+
+    /*
+     * Some devices [1] can cause glitch on I2C bus which makes TWI controller
+     * unresponsive and it won't write anything onto bus until disabled and
+     * enabled again. To workaround this we can just check if SCL line is
+     * pulled low after triggering start task which means controller works
+     * properly. In other case, we disable and enable TWI controller and try
+     * again. If this still fails to make controller work properly
+     *
+     * [1] LP5523 does this on reset
+     */
+
+    do {
+        twi->EVENTS_BB = 0;
+        *task = 1;
+
+        /*
+         * Wait a bit for low state on SCL as this indicates that controller has
+         * started writing something one the bus. It does not matter whether low
+         * state is due to START condition on bus or one of clock cycles when
+         * writing address on bus - in any case this means controller seems to
+         * write something on bus.
+         */
+
+        end_ticks = os_cputime_get32() +
+                    os_cputime_usecs_to_ticks(MYNEWT_VAL(MCU_I2C_RECOVERY_DELAY_USEC));
+
+        do {
+            /*
+             * For write op controller will always keep SCL low after writing
+             * START and address on bus and until we write 1st byte of data to
+             * TXD. This allows to reliably detect activity on bus by using SCL
+             * only.
+             *
+             * For read op with only single byte to read it's possible that it
+             * will be read before we start checking SCL line and thus we'll
+             * never detect any activity this way. To avoid this, we'll also
+             * check BB event which in such case indicates that some activity
+             * on bus happened. This won't work for writes since BB is generated
+             * after byte is transmitted, so we need to use both methods to be
+             * able to handle unresponsive TWI controller for both reads and
+             * writes.
+             */
+            if (!hal_gpio_read(twi->PSELSCL) || twi->EVENTS_BB) {
+                return;
+            }
+        } while (CPUTIME_LT(os_cputime_get32(), end_ticks));
+
+        twi->ENABLE = TWI_ENABLE_ENABLE_Disabled;
+        /*
+         * This is to "clear" other devices on bus which may be affected by the
+         * same glitch.
+         */
+        hal_i2c_clear_bus(twi->PSELSCL, twi->PSELSDA);
+        twi->ENABLE = TWI_ENABLE_ENABLE_Enabled;
+    } while (--retry);
+}
+
 int
 hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
                      uint32_t timo, uint8_t last_op)
@@ -434,8 +497,7 @@ hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
     regs->EVENTS_SUSPENDED = 0;
     regs->SHORTS = 0;
 
-    regs->TASKS_STARTTX = 1;
-    regs->TASKS_RESUME = 1;
+    hal_i2c_trigger_start(regs, &regs->TASKS_STARTTX);
 
     start = os_time_get();
     for (i = 0; i < pdata->len; i++) {
@@ -475,16 +537,6 @@ hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
         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;
     }
 
     return (rc);
@@ -528,7 +580,8 @@ hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
     } else {
         regs->SHORTS = TWI_SHORTS_BB_SUSPEND_Msk;
     }
-    regs->TASKS_STARTRX = 1;
+
+    hal_i2c_trigger_start(regs, &regs->TASKS_STARTRX);
 
     for (i = 0; i < pdata->len; i++) {
         regs->TASKS_RESUME = 1;
@@ -554,22 +607,12 @@ hal_i2c_master_read(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
 
 err:
     regs->TASKS_STOP = 1;
-    regs->SHORTS = TWI_SHORTS_BB_STOP_Msk;
+    regs->SHORTS = 0;
 
     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;
     }
 
     return (rc);
diff --git a/hw/mcu/nordic/nrf52xxx/syscfg.yml b/hw/mcu/nordic/nrf52xxx/syscfg.yml
index 2295677761..d0fb585d40 100644
--- a/hw/mcu/nordic/nrf52xxx/syscfg.yml
+++ b/hw/mcu/nordic/nrf52xxx/syscfg.yml
@@ -37,6 +37,14 @@ syscfg.defs:
             expected to be overridden by the BSP.
         value: 0
 
+    MCU_I2C_RECOVERY_DELAY_USEC:
+        description: >
+            Time to wait for activity on SCL line after triggering start task
+            before restarting TWI controller. This is to recover from state
+            where controller is unresponsive due to glitch on I2C bus.
+            Note: Default value seems to work fine, but may need to be tuned.
+        value: 100
+
 # MCU peripherals definitions
     I2C_0:
         description: 'Enable nRF52xxx I2C (TWI) 0'


 

----------------------------------------------------------------
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