Hi Greg,
I experienced some crashes when using I2C devices with lpc1769 micros
(again). I found that the root cause of this behaviour is a race
condition in which nxsem_wait(&priv->wait) inside lpc17_40_i2c_start()
doesn't resume fast enough causing the priv->wait semaphore to be
incremented twice.
I also improved the timeout calculation I did last year, patches attached.
Thanks,
Augusto.
From cb1fe553dfd405deb62cd329f476d6330f9675f2 Mon Sep 17 00:00:00 2001
From: Augusto Fraga Giachero <augustof...@gmail.com>
Date: Mon, 2 Mar 2020 16:34:54 -0300
Subject: [PATCH 1/2] arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c: Cancel timeout
on i2c stop
Not canceling the I2C timeout watch dog immediately after finishing
all I2C transactions in interrupt context can lead to a race condition
due to nxsem_wait(&priv->wait) in lpc17_40_i2c_start() not resuming
execution fast enough (this can be easily triggered if another task /
thread is using a lot of cpu time).
Falling to cancel the watchdog up to time will cause the priv->wait
semaphore to be incremented twice (first by lpc17_40_i2c_stop() then
by lpc17_40_i2c_timeout()), so all I2C transactions after that will
return immediately and priv->msgs will hold pointers to memory it
doesn't own anymore.
Canceling the priv->timeout watch dog in lpc17_40_i2c_stop() prevents
this as it is executed from the I2C interrupt handler.
---
arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c b/arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c
index f6e43d3c31..b4b8230b04 100644
--- a/arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c
+++ b/arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c
@@ -242,8 +242,6 @@ static int lpc17_40_i2c_start(struct lpc17_40_i2cdev_s *priv)
(uint32_t)priv);
nxsem_wait(&priv->wait);
- wd_cancel(priv->timeout);
-
return priv->nmsg;
}
@@ -263,6 +261,7 @@ static void lpc17_40_i2c_stop(struct lpc17_40_i2cdev_s *priv)
priv->base + LPC17_40_I2C_CONSET_OFFSET);
}
+ wd_cancel(priv->timeout);
nxsem_post(&priv->wait);
}
--
2.25.1
From 1e65ac2e20413cafb0c85684ce3535e6c3706ac9 Mon Sep 17 00:00:00 2001
From: Augusto Fraga Giachero <augustof...@gmail.com>
Date: Mon, 2 Mar 2020 16:35:47 -0300
Subject: [PATCH 2/2] arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c: Fix timeout
calculation
For each byte received / transmitted, an acknowledge bit is also
transmitted / received, requiring effectively 9 bits for each byte.
---
arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c b/arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c
index b4b8230b04..2052e7b40d 100644
--- a/arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c
+++ b/arch/arm/src/lpc17xx_40xx/lpc17_40_i2c.c
@@ -232,7 +232,7 @@ static int lpc17_40_i2c_start(struct lpc17_40_i2cdev_s *priv)
/* Calculate the approximate timeout */
- timeout = ((total_len * (8000000 / CONFIG_USEC_PER_TICK)) / freq) + 1;
+ timeout = ((total_len * (9000000 / CONFIG_USEC_PER_TICK)) / freq) + 1;
/* Initializes the I2C state machine to a known value */
--
2.25.1
