[PATCH] i2c: i2c_cdns: fix write timeout on fifo boundary
This fixes an issue that would cause I2C writes to timeout when the number of bytes is a multiple of the FIFO depth (i.e. 16 bytes).
Within the transfer loop, after writing the data register with a new byte to transfer, if the transfer size equals the FIFO depth, the loop pauses until the INTERRUPT_COMP bit asserts to indicate data has been sent. This same check is performed after the loop as well to ensure data has been transferred prior to returning.
In the case where the amount of data to be written is a multiple of the FIFO depth, the transfer loop would wait for the INTERRUPT_COMP bit to assert after writing the final byte, and then wait for this bit to assert once more. However, since the transfer has finished at this point, no new data has been written to the data register, and hence INTERRUPT_COMP will never assert.
Fix this by only waiting for INTERRUPT_COMP in the transfer loop if there's still data to be written.
Signed-off-by: Michael Auchter michael.auchter@ni.com --- drivers/i2c/i2c-cdns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-cdns.c b/drivers/i2c/i2c-cdns.c index 2c0301ad08..ff3956d8c2 100644 --- a/drivers/i2c/i2c-cdns.c +++ b/drivers/i2c/i2c-cdns.c @@ -265,7 +265,7 @@ static int cdns_i2c_write_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
while (len-- && !is_arbitration_lost(regs)) { writel(*(cur_data++), ®s->data); - if (readl(®s->transfer_size) == CDNS_I2C_FIFO_DEPTH) { + if (len && readl(®s->transfer_size) == CDNS_I2C_FIFO_DEPTH) { ret = cdns_i2c_wait(regs, CDNS_I2C_INTERRUPT_COMP | CDNS_I2C_INTERRUPT_ARBLOST); if (ret & CDNS_I2C_INTERRUPT_ARBLOST)
On Mon, 9 Dec 2019 at 11:19, Michael Auchter michael.auchter@ni.com wrote:
This fixes an issue that would cause I2C writes to timeout when the number of bytes is a multiple of the FIFO depth (i.e. 16 bytes).
Within the transfer loop, after writing the data register with a new byte to transfer, if the transfer size equals the FIFO depth, the loop pauses until the INTERRUPT_COMP bit asserts to indicate data has been sent. This same check is performed after the loop as well to ensure data has been transferred prior to returning.
In the case where the amount of data to be written is a multiple of the FIFO depth, the transfer loop would wait for the INTERRUPT_COMP bit to assert after writing the final byte, and then wait for this bit to assert once more. However, since the transfer has finished at this point, no new data has been written to the data register, and hence INTERRUPT_COMP will never assert.
Fix this by only waiting for INTERRUPT_COMP in the transfer loop if there's still data to be written.
Signed-off-by: Michael Auchter michael.auchter@ni.com
drivers/i2c/i2c-cdns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-cdns.c b/drivers/i2c/i2c-cdns.c index 2c0301ad08..ff3956d8c2 100644 --- a/drivers/i2c/i2c-cdns.c +++ b/drivers/i2c/i2c-cdns.c @@ -265,7 +265,7 @@ static int cdns_i2c_write_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
while (len-- && !is_arbitration_lost(regs)) { writel(*(cur_data++), ®s->data);
if (readl(®s->transfer_size) == CDNS_I2C_FIFO_DEPTH) {
if (len && readl(®s->transfer_size) == CDNS_I2C_FIFO_DEPTH) { ret = cdns_i2c_wait(regs, CDNS_I2C_INTERRUPT_COMP | CDNS_I2C_INTERRUPT_ARBLOST); if (ret & CDNS_I2C_INTERRUPT_ARBLOST)-- 2.23.0
Reviewed-by: Simon Glass sjg@chromium.org
Hello Michael,
Am 09.12.2019 um 19:16 schrieb Michael Auchter:
This fixes an issue that would cause I2C writes to timeout when the number of bytes is a multiple of the FIFO depth (i.e. 16 bytes).
Within the transfer loop, after writing the data register with a new byte to transfer, if the transfer size equals the FIFO depth, the loop pauses until the INTERRUPT_COMP bit asserts to indicate data has been sent. This same check is performed after the loop as well to ensure data has been transferred prior to returning.
In the case where the amount of data to be written is a multiple of the FIFO depth, the transfer loop would wait for the INTERRUPT_COMP bit to assert after writing the final byte, and then wait for this bit to assert once more. However, since the transfer has finished at this point, no new data has been written to the data register, and hence INTERRUPT_COMP will never assert.
Fix this by only waiting for INTERRUPT_COMP in the transfer loop if there's still data to be written.
Signed-off-by: Michael Auchter michael.auchter@ni.com
drivers/i2c/i2c-cdns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-cdns.c b/drivers/i2c/i2c-cdns.c index 2c0301ad08..ff3956d8c2 100644 --- a/drivers/i2c/i2c-cdns.c +++ b/drivers/i2c/i2c-cdns.c @@ -265,7 +265,7 @@ static int cdns_i2c_write_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
while (len-- && !is_arbitration_lost(regs)) { writel(*(cur_data++), ®s->data);
if (readl(®s->transfer_size) == CDNS_I2C_FIFO_DEPTH) {
if (len && readl(®s->transfer_size) == CDNS_I2C_FIFO_DEPTH) { ret = cdns_i2c_wait(regs, CDNS_I2C_INTERRUPT_COMP | CDNS_I2C_INTERRUPT_ARBLOST); if (ret & CDNS_I2C_INTERRUPT_ARBLOST)
Thanks for detecting this!
I soon send a pull request for Tom, if travis build is fine.
bye, Heiko
Hello Michael,
Am 09.12.2019 um 19:16 schrieb Michael Auchter:
This fixes an issue that would cause I2C writes to timeout when the number of bytes is a multiple of the FIFO depth (i.e. 16 bytes).
Within the transfer loop, after writing the data register with a new byte to transfer, if the transfer size equals the FIFO depth, the loop pauses until the INTERRUPT_COMP bit asserts to indicate data has been sent. This same check is performed after the loop as well to ensure data has been transferred prior to returning.
In the case where the amount of data to be written is a multiple of the FIFO depth, the transfer loop would wait for the INTERRUPT_COMP bit to assert after writing the final byte, and then wait for this bit to assert once more. However, since the transfer has finished at this point, no new data has been written to the data register, and hence INTERRUPT_COMP will never assert.
Fix this by only waiting for INTERRUPT_COMP in the transfer loop if there's still data to be written.
Signed-off-by: Michael Auchter michael.auchter@ni.com
drivers/i2c/i2c-cdns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks!
Applied to u-boot-i2c master
bye, Heiko
participants (3)
-
Heiko Schocher -
Michael Auchter -
Simon Glass