You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by an...@apache.org on 2018/12/04 13:10:18 UTC
[mynewt-core] branch master updated (bd7a18b -> ce9e4c9)
This is an automated email from the ASF dual-hosted git repository.
andk pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git.
from bd7a18b sys/stats: Allow stats to be registered before sysinit
new bb1938c hw/mcu/nordic: Remove unnecessary RESUME task trigger
new 2aca00e hw/mcu/nordic: Reset shorts on RX error
new e22b981 hw/mcu/nordic: Rework workaround for unresponsive TWI
new ce9e4c9 hw/mcu/nordic: Fix check for unresponsive TWI on read_op
The 4 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.
Summary of changes:
hw/mcu/nordic/nrf52xxx/src/hal_i2c.c | 91 ++++++++++++++++++++++++++----------
hw/mcu/nordic/nrf52xxx/syscfg.yml | 8 ++++
2 files changed, 75 insertions(+), 24 deletions(-)
[mynewt-core] 02/04: hw/mcu/nordic: Reset shorts on RX error
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git
commit 2aca00e83b4100f89c972b282da6301288156d0b
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Mon Nov 26 11:10:35 2018 +0100
hw/mcu/nordic: Reset shorts on RX error
We manually trigger STOP task so there's no need for setting shorts
here. We should better just clear them instead.
---
hw/mcu/nordic/nrf52xxx/src/hal_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index cea346c..dcab21e 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -556,7 +556,7 @@ 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;
[mynewt-core] 04/04: hw/mcu/nordic: Fix check for unresponsive TWI
on read_op
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git
commit ce9e4c95d14a0b9d3c58b67c3ade181a15c16fed
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Fri Nov 30 16:12:57 2018 +0100
hw/mcu/nordic: Fix check for unresponsive TWI on read_op
When reading single byte on TWI it may happen that checks for SCL
line will start after this byte is received so SCL will never go to
low state because activity already happened on bus. To avoid this we'll
also check BB event which will also indicate that some activity on bus
happened.
For write ops there's no problem since after writing START and address
on bus controller will keep SCL low until we write 1st byte of data to
TXD.
---
hw/mcu/nordic/nrf52xxx/src/hal_i2c.c | 63 +++++++++++++++++++-----------------
1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index 301dcd5..cb23a0f 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -413,35 +413,10 @@ hal_i2c_config(uint8_t i2c_num, const struct hal_i2c_settings *cfg)
return 0;
}
-static bool
-hal_i2c_check_scl(int pin_scl)
-{
- uint32_t end_ticks;
- int toggled = 0;
-
- /*
- * 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 {
- if (!hal_gpio_read(pin_scl)) {
- toggled = 1;
- }
- } while (!toggled && CPUTIME_LT(os_cputime_get32(), end_ticks));
-
- return toggled;
-}
-
static inline void
hal_i2c_trigger_start(NRF_TWI_Type *twi, __O uint32_t *task)
{
+ uint32_t end_ticks;
int retry = 2;
/*
@@ -456,10 +431,40 @@ hal_i2c_trigger_start(NRF_TWI_Type *twi, __O uint32_t *task)
*/
do {
+ twi->EVENTS_BB = 0;
*task = 1;
- if (hal_i2c_check_scl(twi->PSELSCL)) {
- break;
- }
+
+ /*
+ * 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;
/*
[mynewt-core] 03/04: hw/mcu/nordic: Rework workaround for
unresponsive TWI
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git
commit e22b981dbe5178ac188911c95a1dd0142b415bad
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Fri Nov 23 17:46:03 2018 +0100
hw/mcu/nordic: Rework workaround for unresponsive TWI
Current workaround for unresponsive TWI will be triggered every time
there is operation timeout which has some drawbacks: it will be applied
to every operation that times out, it takes entire operation time to
recover and it will make operation fail so caller needs to retry.
It is possible to detect this condition and apply workaround quicker
without making operation fail by using simple hack: after triggering
START task SCL line should be pulled low to make START condition and
also when address is written. We can just wait a moment to see if SCL
is pulled low during this time - if not, restart controller and try
again.
This seems to work fine with LP5523 on the bus which can be used to
easily trigger this condition by sending reset command.
---
hw/mcu/nordic/nrf52xxx/src/hal_i2c.c | 83 ++++++++++++++++++++++++++----------
hw/mcu/nordic/nrf52xxx/syscfg.yml | 8 ++++
2 files changed, 69 insertions(+), 22 deletions(-)
diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index dcab21e..301dcd5 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -413,6 +413,64 @@ hal_i2c_config(uint8_t i2c_num, const struct hal_i2c_settings *cfg)
return 0;
}
+static bool
+hal_i2c_check_scl(int pin_scl)
+{
+ uint32_t end_ticks;
+ int toggled = 0;
+
+ /*
+ * 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 {
+ if (!hal_gpio_read(pin_scl)) {
+ toggled = 1;
+ }
+ } while (!toggled && CPUTIME_LT(os_cputime_get32(), end_ticks));
+
+ return toggled;
+}
+
+static inline void
+hal_i2c_trigger_start(NRF_TWI_Type *twi, __O uint32_t *task)
+{
+ 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 {
+ *task = 1;
+ if (hal_i2c_check_scl(twi->PSELSCL)) {
+ break;
+ }
+
+ 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)
@@ -437,7 +495,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;
+ hal_i2c_trigger_start(regs, ®s->TASKS_STARTTX);
start = os_time_get();
for (i = 0; i < pdata->len; i++) {
@@ -477,16 +535,6 @@ err:
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);
@@ -530,7 +578,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, ®s->TASKS_STARTRX);
for (i = 0; i < pdata->len; i++) {
regs->TASKS_RESUME = 1;
@@ -562,16 +611,6 @@ err:
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 a22c881..31820b9 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'
[mynewt-core] 01/04: hw/mcu/nordic: Remove unnecessary RESUME task
trigger
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git
commit bb1938c0d5f448cb6a1d4152c03995652959a562
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Mon Nov 26 11:09:39 2018 +0100
hw/mcu/nordic: Remove unnecessary RESUME task trigger
RESUME is used after SUSPEND, but we do not trigger SUSPEND anywhere so
this seems unnecessary.
---
hw/mcu/nordic/nrf52xxx/src/hal_i2c.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
index 24a6b25..cea346c 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_i2c.c
@@ -438,7 +438,6 @@ hal_i2c_master_write(uint8_t i2c_num, struct hal_i2c_master_data *pdata,
regs->SHORTS = 0;
regs->TASKS_STARTTX = 1;
- regs->TASKS_RESUME = 1;
start = os_time_get();
for (i = 0; i < pdata->len; i++) {