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 <[email protected]> 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];
