All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: linux-i2c@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>
Subject: [PATCH 1/2] i2c: rcar: avoid race condition with SMIs
Date: Fri, 20 May 2022 12:33:24 +0200	[thread overview]
Message-ID: <20220520103325.81110-2-wsa+renesas@sang-engineering.com> (raw)
In-Reply-To: <20220520103325.81110-1-wsa+renesas@sang-engineering.com>

A customer experienced a race condition with 'repeated starts' when a
System Management Interrupt took over for 30us and more. The problem was
that during the SMI a new MAT interrupt came in because we set up the
'repeated start' condition. But the old one was not acknowledged yet.
So, when it was acknowledged after the SMI, the new MAT interrupt was
lost, confusing the state machine of the driver.

The fix consists of two parts. First, we do not clear the status
register for 'repeated starts' when preparing the next message anymore.
The interrupt handlers for sending and receiving data is now solely
responsible for that and it makes the code easier to follow, in fact.
Secondly, clearing the status register is now split up to handle MAT
interrupts independently. This avoids the race condition because the old
MAT interrupt will be now cleared before we initiate the "repeated
start" condition.

Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index ca61dbe218bf..143cb4186daa 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -97,9 +97,6 @@
 #define RCAR_IRQ_RECV	(MNR | MAL | MST | MAT | MDR)
 #define RCAR_IRQ_STOP	(MST)
 
-#define RCAR_IRQ_ACK_SEND	(~(MAT | MDE) & 0x7F)
-#define RCAR_IRQ_ACK_RECV	(~(MAT | MDR) & 0x7F)
-
 #define ID_LAST_MSG	(1 << 0)
 #define ID_FIRST_MSG	(1 << 1)
 #define ID_DONE		(1 << 2)
@@ -161,6 +158,11 @@ static u32 rcar_i2c_read(struct rcar_i2c_priv *priv, int reg)
 	return readl(priv->io + reg);
 }
 
+static void rcar_i2c_clear_irq(struct rcar_i2c_priv *priv, u32 val)
+{
+	writel(~val & 0x7f, priv->io + ICMSR);
+}
+
 static int rcar_i2c_get_scl(struct i2c_adapter *adap)
 {
 	struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
@@ -356,7 +358,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 			priv->flags &= ~ID_P_REP_AFTER_RD;
 		else
 			rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-		rcar_i2c_write(priv, ICMSR, 0);
+		/* ICMSR is cleared in interrupt handlers */
 	}
 }
 
@@ -476,11 +478,15 @@ static bool rcar_i2c_dma(struct rcar_i2c_priv *priv)
 static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 {
 	struct i2c_msg *msg = priv->msg;
+	u32 irqs_to_clear = MDE;
 
 	/* FIXME: sometimes, unknown interrupt happened. Do nothing */
 	if (!(msr & MDE))
 		return;
 
+	if (msr & MAT)
+		irqs_to_clear |= MAT;
+
 	/* Check if DMA can be enabled and take over */
 	if (priv->pos == 1 && rcar_i2c_dma(priv))
 		return;
@@ -504,32 +510,32 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 		 * [ICRXTX] -> [SHIFT] -> [I2C bus]
 		 */
 
-		if (priv->flags & ID_LAST_MSG) {
+		if (priv->flags & ID_LAST_MSG)
 			/*
 			 * If current msg is the _LAST_ msg,
 			 * prepare stop condition here.
 			 * ID_DONE will be set on STOP irq.
 			 */
 			rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
-		} else {
+		else
 			rcar_i2c_next_msg(priv);
-			return;
-		}
 	}
 
-	rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_SEND);
+	rcar_i2c_clear_irq(priv, irqs_to_clear);
 }
 
 static void rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 {
 	struct i2c_msg *msg = priv->msg;
 	bool recv_len_init = priv->pos == 0 && msg->flags & I2C_M_RECV_LEN;
+	u32 irqs_to_clear = MDR;
 
 	/* FIXME: sometimes, unknown interrupt happened. Do nothing */
 	if (!(msr & MDR))
 		return;
 
 	if (msr & MAT) {
+		irqs_to_clear |= MAT;
 		/*
 		 * Address transfer phase finished, but no data at this point.
 		 * Try to use DMA to receive data.
@@ -570,8 +576,8 @@ static void rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 
 	if (priv->pos == msg->len && !(priv->flags & ID_LAST_MSG))
 		rcar_i2c_next_msg(priv);
-	else
-		rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_RECV);
+
+	rcar_i2c_clear_irq(priv, irqs_to_clear);
 }
 
 static bool rcar_i2c_slave_irq(struct rcar_i2c_priv *priv)
-- 
2.35.1


  reply	other threads:[~2022-05-20 10:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-20 10:33 [PATCH 0/2] i2c: rcar: increase robustness against long SMIs Wolfram Sang
2022-05-20 10:33 ` Wolfram Sang [this message]
2022-05-21  6:37   ` [PATCH 1/2] i2c: rcar: avoid race condition with SMIs Wolfram Sang
2022-05-20 10:33 ` [PATCH 2/2] i2c: rcar: refactor handling of first message Wolfram Sang
2022-05-21  6:37   ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220520103325.81110-2-wsa+renesas@sang-engineering.com \
    --to=wsa+renesas@sang-engineering.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.