drivers: i2c_nrfx_{twi, twim}: Remove potential I2C deadlock

Remove K_FOREVER wait on completion_sync.
In some situations (a short on I2C SDA line for example), this
semaphore will never be released and therefore we should not wait
it forever.
Instead we wait for a maximum of 100msec and return an error if we
weren't able to retrieve the semaphore.
In such situation, the program is not stuck anymore, but the I2C
driver must be uninit then init again to work again.

Fixes #25076.

Signed-off-by: Xavier Chapron <xavier.chapron@stimio.fr>
This commit is contained in:
Xavier Chapron 2020-04-17 16:22:35 +02:00 committed by Anas Nashif
parent fd5ce64db4
commit cecea7b536
2 changed files with 48 additions and 2 deletions

View file

@ -12,6 +12,8 @@
#include <logging/log.h> #include <logging/log.h>
LOG_MODULE_REGISTER(i2c_nrfx_twi, CONFIG_I2C_LOG_LEVEL); LOG_MODULE_REGISTER(i2c_nrfx_twi, CONFIG_I2C_LOG_LEVEL);
#define I2C_TRANSFER_TIMEOUT_MSEC K_MSEC(100)
struct i2c_nrfx_twi_data { struct i2c_nrfx_twi_data {
struct k_sem transfer_sync; struct k_sem transfer_sync;
struct k_sem completion_sync; struct k_sem completion_sync;
@ -44,6 +46,10 @@ static int i2c_nrfx_twi_transfer(struct device *dev, struct i2c_msg *msgs,
int ret = 0; int ret = 0;
k_sem_take(&(get_dev_data(dev)->transfer_sync), K_FOREVER); k_sem_take(&(get_dev_data(dev)->transfer_sync), K_FOREVER);
/* Dummy take on completion_sync sem to be sure that it is empty */
k_sem_take(&(get_dev_data(dev)->completion_sync), K_NO_WAIT);
nrfx_twi_enable(&get_dev_config(dev)->twi); nrfx_twi_enable(&get_dev_config(dev)->twi);
for (size_t i = 0; i < num_msgs; i++) { for (size_t i = 0; i < num_msgs; i++) {
@ -103,7 +109,24 @@ static int i2c_nrfx_twi_transfer(struct device *dev, struct i2c_msg *msgs,
} }
} }
k_sem_take(&(get_dev_data(dev)->completion_sync), K_FOREVER); ret = k_sem_take(&(get_dev_data(dev)->completion_sync),
I2C_TRANSFER_TIMEOUT_MSEC);
if (ret != 0) {
/* Whatever the frequency, completion_sync should have
* been give by the event handler.
*
* If it hasn't it's probably due to an hardware issue
* on the I2C line, for example a short between SDA and
* GND.
*
* Note to fully recover from this issue one should
* reinit nrfx twi.
*/
LOG_ERR("Error on I2C line occurred for message %d", i);
ret = -EIO;
break;
}
res = get_dev_data(dev)->res; res = get_dev_data(dev)->res;
if (res != NRFX_SUCCESS) { if (res != NRFX_SUCCESS) {
LOG_ERR("Error %d occurred for message %d", res, i); LOG_ERR("Error %d occurred for message %d", res, i);

View file

@ -12,6 +12,8 @@
#include <logging/log.h> #include <logging/log.h>
LOG_MODULE_REGISTER(i2c_nrfx_twim, CONFIG_I2C_LOG_LEVEL); LOG_MODULE_REGISTER(i2c_nrfx_twim, CONFIG_I2C_LOG_LEVEL);
#define I2C_TRANSFER_TIMEOUT_MSEC K_MSEC(100)
struct i2c_nrfx_twim_data { struct i2c_nrfx_twim_data {
struct k_sem transfer_sync; struct k_sem transfer_sync;
struct k_sem completion_sync; struct k_sem completion_sync;
@ -44,6 +46,10 @@ static int i2c_nrfx_twim_transfer(struct device *dev, struct i2c_msg *msgs,
int ret = 0; int ret = 0;
k_sem_take(&(get_dev_data(dev)->transfer_sync), K_FOREVER); k_sem_take(&(get_dev_data(dev)->transfer_sync), K_FOREVER);
/* Dummy take on completion_sync sem to be sure that it is empty */
k_sem_take(&(get_dev_data(dev)->completion_sync), K_NO_WAIT);
nrfx_twim_enable(&get_dev_config(dev)->twim); nrfx_twim_enable(&get_dev_config(dev)->twim);
for (size_t i = 0; i < num_msgs; i++) { for (size_t i = 0; i < num_msgs; i++) {
@ -74,7 +80,24 @@ static int i2c_nrfx_twim_transfer(struct device *dev, struct i2c_msg *msgs,
} }
} }
k_sem_take(&(get_dev_data(dev)->completion_sync), K_FOREVER); ret = k_sem_take(&(get_dev_data(dev)->completion_sync),
I2C_TRANSFER_TIMEOUT_MSEC);
if (ret != 0) {
/* Whatever the frequency, completion_sync should have
* been give by the event handler.
*
* If it hasn't it's probably due to an hardware issue
* on the I2C line, for example a short between SDA and
* GND.
*
* Note to fully recover from this issue one should
* reinit nrfx twim.
*/
LOG_ERR("Error on I2C line occurred for message %d", i);
ret = -EIO;
break;
}
res = get_dev_data(dev)->res; res = get_dev_data(dev)->res;
if (res != NRFX_SUCCESS) { if (res != NRFX_SUCCESS) {
LOG_ERR("Error %d occurred for message %d", res, i); LOG_ERR("Error %d occurred for message %d", res, i);