You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by je...@apache.org on 2021/02/01 07:19:46 UTC
[mynewt-core] 01/02: bus/i2c_da1469x: Fix race condition issue
This is an automated email from the ASF dual-hosted git repository.
jerzy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git
commit 403e1835007f9c15deb89f27d16d93a67a94d2cb
Author: Jerzy Kasenberg <je...@codecoup.pl>
AuthorDate: Tue Jan 5 15:21:23 2021 +0100
bus/i2c_da1469x: Fix race condition issue
STOP condition was correctly handled it could lead to
cases when stop condition from previous transaction was
detected later and disrupted code flow of next transfers.
- i2c_da1469x_clear_int() was called after reading active interrupts
but new interrupts could just pop up in after register was read.
This is the case of stop condition interrupt that has to be clear
by software. Now it is explicitly cleared when it was detected.
- when interrupt was fired for M_RX_FULL and all data requested was
read but stop condition was not detected yet. Transfer was finished
even though NOSTOP flag was not set, and code should wait for
stop M_STOP_DET. This lead to previous stop condition messing with
next transfer.
All this problem surfaced when system was running on 96MHz.
---
hw/bus/drivers/i2c_da1469x/src/i2c_da1469x.c | 37 +++++++++++++---------------
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/hw/bus/drivers/i2c_da1469x/src/i2c_da1469x.c b/hw/bus/drivers/i2c_da1469x/src/i2c_da1469x.c
index 79aa2d6..084b762 100644
--- a/hw/bus/drivers/i2c_da1469x/src/i2c_da1469x.c
+++ b/hw/bus/drivers/i2c_da1469x/src/i2c_da1469x.c
@@ -180,7 +180,7 @@ i2c_da1469x_fill_tx_dma_buffer(struct i2c_da1469x_dev_data *dd)
dd->tx_dma_buf[0] = I2C_I2C_DATA_CMD_REG_I2C_CMD_Msk |
I2C_I2C_DATA_CMD_REG_I2C_STOP_Msk;
} else {
- /* Stop requested more send lentght - 1 without stop */
+ /* Stop requested more send length - 1 without stop */
dd->tx_dma_buf[0] = I2C_I2C_DATA_CMD_REG_I2C_CMD_Msk;
--length;
}
@@ -256,8 +256,6 @@ i2c_da1469x_fill_fifo_for_rx(I2C_Type *i2c_regs, struct i2c_da1469x_dev_data *dd
return;
}
- i2c_da1469x_clear_int(i2c_regs);
-
while ((transfer->wlen > 0) &&
i2c_regs->I2C_STATUS_REG & I2C_I2C_STATUS_REG_TFNF_Msk) {
transfer->wlen--;
@@ -266,6 +264,8 @@ i2c_da1469x_fill_fifo_for_rx(I2C_Type *i2c_regs, struct i2c_da1469x_dev_data *dd
} else {
i2c_regs->I2C_DATA_CMD_REG = (1U << I2C_I2C_DATA_CMD_REG_I2C_CMD_Pos) |
(1U << I2C_I2C_DATA_CMD_REG_I2C_STOP_Pos);
+ /* All data in tx buffer, no need for TX interrupt any more */
+ i2c_regs->I2C_INTR_MASK_REG &= ~I2C_I2C_INTR_MASK_REG_M_TX_EMPTY_Msk;
}
}
@@ -275,29 +275,28 @@ i2c_da1469x_fill_fifo_for_rx(I2C_Type *i2c_regs, struct i2c_da1469x_dev_data *dd
*transfer->data++ = (uint8_t)i2c_regs->I2C_DATA_CMD_REG;
transfer->rlen--;
if (transfer->rlen == 0) {
- i2c_regs->I2C_INTR_MASK_REG = 0;
- i2c_da1469x_clear_int(i2c_regs);
- transfer->started = 0;
- os_sem_release(&dd->sem);
+ i2c_regs->I2C_INTR_MASK_REG &= ~I2C_I2C_INTR_MASK_REG_M_RX_FULL_Msk;
+ if (transfer->nostop) {
+ transfer->started = 0;
+ dd->errorsrc = 0;
+ os_sem_release(&dd->sem);
+ }
}
}
}
if (intr_stat & I2C_I2C_INTR_MASK_REG_M_STOP_DET_Msk) {
+ /* Clear STOP condition interrupt */
+ (void)i2c_regs->I2C_CLR_STOP_DET_REG;
i2c_regs->I2C_INTR_MASK_REG = 0;
- assert(transfer->started == 0 && transfer->rlen == 0);
- if (transfer->started || transfer->rlen) {
+ if (transfer->started) {
transfer->started = 0;
+ if (transfer->rlen > 0) {
+ dd->errorsrc = I2C_I2C_TX_ABRT_SOURCE_REG_ABRT_USER_ABRT_Msk;
+ }
transfer->rlen = 0;
- dd->errorsrc = I2C_I2C_TX_ABRT_SOURCE_REG_ABRT_USER_ABRT_Msk;
os_sem_release(&dd->sem);
}
- return;
- }
-
- /* All data in tx buffer, no need for TX interrupt any more */
- if (transfer->wlen == 0) {
- i2c_regs->I2C_INTR_MASK_REG &= ~I2C_I2C_INTR_MASK_REG_M_TX_EMPTY_Msk;
}
}
@@ -332,8 +331,6 @@ i2c_da1469x_fill_fifo_for_tx(I2C_Type *i2c_regs, struct i2c_da1469x_dev_data *dd
return;
}
- i2c_da1469x_clear_int(i2c_regs);
-
while ((transfer->wlen > 0) &&
i2c_regs->I2C_STATUS_REG & I2C_I2C_STATUS_REG_TFNF_Msk) {
transfer->wlen--;
@@ -346,7 +343,6 @@ i2c_da1469x_fill_fifo_for_tx(I2C_Type *i2c_regs, struct i2c_da1469x_dev_data *dd
if (transfer->wlen == 0) {
if (transfer->nostop) {
i2c_regs->I2C_INTR_MASK_REG = 0;
- i2c_da1469x_clear_int(i2c_regs);
transfer->started = 0;
os_sem_release(&dd->sem);
}
@@ -357,6 +353,8 @@ i2c_da1469x_fill_fifo_for_tx(I2C_Type *i2c_regs, struct i2c_da1469x_dev_data *dd
}
if (intr_stat & I2C_I2C_INTR_MASK_REG_M_STOP_DET_Msk) {
+ /* Clear STOP condition interrupt */
+ (void)i2c_regs->I2C_CLR_STOP_DET_REG;
i2c_regs->I2C_INTR_MASK_REG = 0;
if (transfer->started) {
transfer->started = 0;
@@ -563,7 +561,6 @@ i2c_da1469x_write(struct bus_dev *bdev, struct bus_node *bnode,
BUS_DEBUG_VERIFY_DEV(dev);
BUS_DEBUG_VERIFY_NODE(node);
-
i2c_regs = da1469x_i2c[dev->cfg.i2c_num].regs;
dd = &i2c_dev_data[dev->cfg.i2c_num];