i2c: imx: ensure no clock is generated after last read

When reading from the I2DR register, right after releasing the bus by
clearing MSTA and MTX, the I2C controller might still generate an
additional clock cycle which can cause devices to misbehave. Ensure to
only read from I2DR after the bus is not busy anymore. Because this
requires polling, the read of the last byte is moved outside of the
interrupt handler.

An example for such a failing transfer is this:
i2ctransfer -y -a 0 w1@0x00 0x02 r1
Error: Sending messages failed: Connection timed out
It does not happen with every device because not all devices react to
the additional clock cycle.

Fixes: 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
Cc: stable@vger.kernel.org # v6.13+
Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
Link: https://lore.kernel.org/r/20260218150940.131354-3-eichest@gmail.com

authored by

Stefan Eichenberger and committed by

Andi Shyti 1 month ago 13101db7 f88e2e74

+32 -19

1 changed file

expand all

drivers

i2c

busses

i2c-imx.c

+32 -19

drivers/i2c/busses/i2c-imx.c

··· 1018 1018 return 0; 1019 1019 } 1020 1020 1021 - static inline void i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx) 1021 + static inline enum imx_i2c_state i2c_imx_isr_read_continue(struct imx_i2c_struct *i2c_imx) 1022 1022 { 1023 + enum imx_i2c_state next_state = IMX_I2C_STATE_READ_CONTINUE; 1023 1024 unsigned int temp; 1024 1025 1025 1026 if ((i2c_imx->msg->len - 1) == i2c_imx->msg_buf_idx) { ··· 1034 1033 i2c_imx->stopped = 1; 1035 1034 temp &= ~(I2CR_MSTA | I2CR_MTX); 1036 1035 imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); 1037 - } else { 1038 - /* 1039 - * For i2c master receiver repeat restart operation like: 1040 - * read -> repeat MSTA -> read/write 1041 - * The controller must set MTX before read the last byte in 1042 - * the first read operation, otherwise the first read cost 1043 - * one extra clock cycle. 1044 - */ 1045 - temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR); 1046 - temp |= I2CR_MTX; 1047 - imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); 1036 + 1037 + return IMX_I2C_STATE_DONE; 1048 1038 } 1039 + /* 1040 + * For i2c master receiver repeat restart operation like: 1041 + * read -> repeat MSTA -> read/write 1042 + * The controller must set MTX before read the last byte in 1043 + * the first read operation, otherwise the first read cost 1044 + * one extra clock cycle. 1045 + */ 1046 + temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR); 1047 + temp |= I2CR_MTX; 1048 + imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); 1049 + next_state = IMX_I2C_STATE_DONE; 1049 1050 } else if (i2c_imx->msg_buf_idx == (i2c_imx->msg->len - 2)) { 1050 1051 temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR); 1051 1052 temp |= I2CR_TXAK; ··· 1055 1052 } 1056 1053 1057 1054 i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); 1055 + return next_state; 1058 1056 } 1059 1057 1060 1058 static inline void i2c_imx_isr_read_block_data_len(struct imx_i2c_struct *i2c_imx) ··· 1092 1088 break; 1093 1089 1094 1090 case IMX_I2C_STATE_READ_CONTINUE: 1095 - i2c_imx_isr_read_continue(i2c_imx); 1096 - if (i2c_imx->msg_buf_idx == i2c_imx->msg->len) { 1097 - i2c_imx->state = IMX_I2C_STATE_DONE; 1091 + i2c_imx->state = i2c_imx_isr_read_continue(i2c_imx); 1092 + if (i2c_imx->state == IMX_I2C_STATE_DONE) 1098 1093 wake_up(&i2c_imx->queue); 1099 - } 1100 1094 break; 1101 1095 1102 1096 case IMX_I2C_STATE_READ_BLOCK_DATA: ··· 1492 1490 bool is_lastmsg) 1493 1491 { 1494 1492 int block_data = msgs->flags & I2C_M_RECV_LEN; 1493 + int ret = 0; 1495 1494 1496 1495 dev_dbg(&i2c_imx->adapter.dev, 1497 1496 "<%s> write slave address: addr=0x%x\n", ··· 1525 1522 dev_err(&i2c_imx->adapter.dev, "<%s> read timedout\n", __func__); 1526 1523 return -ETIMEDOUT; 1527 1524 } 1528 - if (i2c_imx->is_lastmsg && !i2c_imx->stopped) 1529 - return i2c_imx_bus_busy(i2c_imx, 0, false); 1525 + if (i2c_imx->is_lastmsg) { 1526 + if (!i2c_imx->stopped) 1527 + ret = i2c_imx_bus_busy(i2c_imx, 0, false); 1528 + /* 1529 + * Only read the last byte of the last message after the bus is 1530 + * not busy. Else the controller generates another clock which 1531 + * might confuse devices. 1532 + */ 1533 + if (!ret) 1534 + i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = imx_i2c_read_reg(i2c_imx, 1535 + IMX_I2C_I2DR); 1536 + } 1530 1537 1531 - return 0; 1538 + return ret; 1532 1539 } 1533 1540 1534 1541 static int i2c_imx_xfer_common(struct i2c_adapter *adapter,

Configure Feed

Configure Feed