NVIDIA Tegra I2C synchronization correction
Dmitry Osipenko
reviewercover NVIDIA Tegra I2C synchronization correction
Hello,
Recently I found a way to reliably reproduce I2C timeouts that happen due to improper synchronizations made by the I2C driver. It's quite easy to reproduce the problem when memory is running on a lower freq + there is some memory activity + CPU could get busy for a significant time. This is the case when KASAN is enabled and CPU is busy while accessing FS via NFS. This small series addresses the found problems.
Please note that the "Synchronize DMA before termination" patch implicitly depends on the "dmaengine: tegra-apb: Improve DMA synchronization" patch, which is sent separately, otherwise DMA synchronization won't happen.
Dmitry Osipenko (2):
i2c: tegra: Better handle case where CPU0 is busy for a long time
i2c: tegra: Synchronize DMA before termination
drivers/i2c/busses/i2c-tegra.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)
Signature · 1 line
2.25.1
Dmitry Osipenko
reviewerPATCH 1/2 i2c: tegra: Better handle case where CPU0 is busy for a long time
Boot CPU0 always handle I2C interrupt and under some rare circumstances (like running KASAN + NFS root) it may stuck in uninterruptible state for a significant time. In this case we will get timeout if I2C transfer is running on a sibling CPU, despite of IRQ being raised. In order to handle this rare condition, the IRQ status needs to be checked after completion timeout.
--- drivers/i2c/busses/i2c-tegra.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
drivers/i2c/busses/i2c-tegra.c +11 -7
@@ -1034,14 +1034,18 @@ tegra_i2c_wait_completion_timeout(struct tegra_i2c_dev *i2c_dev,disable_irq (i2c_dev->irq);/* -* There is a chance that completion may happen after IRQ -* synchronization, which is done by disable_irq(). +* Under some rare circumstances (like running KASAN + +* NFS root) CPU, which handles interrupt, may stuck in +* uninterruptible state for a significant time. In this +* case we will get timeout if I2C transfer is running on +* a sibling CPU, despite of IRQ being raised. +* +* In order to handle this rare condition, the IRQ status +* needs to be checked after timeout. */ -if (ret ==0 &&completion_done (complete)) { -dev_warn (i2c_dev->dev, -"completion done after timeout\n" ); - ret =1 ; - } +if (ret ==0 ) + ret =tegra_i2c_poll_completion_timeout (i2c_dev, + complete,0 ); }return ret;
Signature · 1 line
2.25.1
Dmitry Osipenko
reviewerPATCH 2/2 i2c: tegra: Synchronize DMA before termination
In reply to Dmitry OsipenkoDMA transfer could be completed, but CPU (which handles DMA interrupt) may get too busy and can't handle the interrupt in a timely manner, despite of DMA IRQ being raised. In this case the DMA state needs to synchronized before terminating DMA transfer in order not to miss the DMA transfer completion.
--- drivers/i2c/busses/i2c-tegra.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/i2c/busses/i2c-tegra.c +9 -0
@@ -1224,6 +1224,15 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, time_left =tegra_i2c_wait_completion_timeout ( i2c_dev, &i2c_dev->dma_complete, xfer_time); +/* +* Synchronize DMA first, since dmaengine_terminate_sync() +* performs synchronization after the transfer's termination +* and we want to get a completion if transfer succeeded. +*/ +dmaengine_synchronize (i2c_dev->msg_read ? + i2c_dev->rx_dma_chan : + i2c_dev->tx_dma_chan); +dmaengine_terminate_sync (i2c_dev->msg_read ? i2c_dev->rx_dma_chan : i2c_dev->tx_dma_chan);
Signature · 1 line
2.25.1