Skip to contentSkip to conversation

NVIDIA Tegra I2C synchronization correction

PATCH Superseded Dmitry Osipenko 1 participant 3 messages Mar 19, 2020

Subsystems (4)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Dmitry Osipenko

author reviewer
#0 lore raw

cover 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
author reviewer
#1 lore raw

PATCH 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
index cbc2ad49043e..dabb9223990c 100644
@@ -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
author reviewer
#2 lore raw

PATCH 2/2 i2c: tegra: Synchronize DMA before termination

In reply to Dmitry Osipenko

DMA 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
index dabb9223990c..b69a10497cd5 100644
@@ -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