linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] NVIDIA Tegra I2C synchronization correction
@ 2020-03-19 20:11 Dmitry Osipenko
  2020-03-19 20:11 ` [PATCH v1 1/2] i2c: tegra: Better handle case where CPU0 is busy for a long time Dmitry Osipenko
  2020-03-19 20:11 ` [PATCH v1 2/2] i2c: tegra: Synchronize DMA before termination Dmitry Osipenko
  0 siblings, 2 replies; 3+ messages in thread
From: Dmitry Osipenko @ 2020-03-19 20:11 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, Vinod Koul, Dan Williams, dmaengine,
	linux-kernel

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(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v1 1/2] i2c: tegra: Better handle case where CPU0 is busy for a long time
  2020-03-19 20:11 [PATCH v1 0/2] NVIDIA Tegra I2C synchronization correction Dmitry Osipenko
@ 2020-03-19 20:11 ` Dmitry Osipenko
  2020-03-19 20:11 ` [PATCH v1 2/2] i2c: tegra: Synchronize DMA before termination Dmitry Osipenko
  1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Osipenko @ 2020-03-19 20:11 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, Vinod Koul, Dan Williams, dmaengine,
	linux-kernel

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.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index cbc2ad49043e..dabb9223990c 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -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;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH v1 2/2] i2c: tegra: Synchronize DMA before termination
  2020-03-19 20:11 [PATCH v1 0/2] NVIDIA Tegra I2C synchronization correction Dmitry Osipenko
  2020-03-19 20:11 ` [PATCH v1 1/2] i2c: tegra: Better handle case where CPU0 is busy for a long time Dmitry Osipenko
@ 2020-03-19 20:11 ` Dmitry Osipenko
  1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Osipenko @ 2020-03-19 20:11 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, Vinod Koul, Dan Williams, dmaengine,
	linux-kernel

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.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index dabb9223990c..b69a10497cd5 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -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);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-03-19 20:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19 20:11 [PATCH v1 0/2] NVIDIA Tegra I2C synchronization correction Dmitry Osipenko
2020-03-19 20:11 ` [PATCH v1 1/2] i2c: tegra: Better handle case where CPU0 is busy for a long time Dmitry Osipenko
2020-03-19 20:11 ` [PATCH v1 2/2] i2c: tegra: Synchronize DMA before termination Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).