All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Fix double address byte issue
@ 2018-05-30 10:07 Fabrizio Castro
  2018-05-30 10:07 ` [PATCH 1/3] i2c: rcar: don't issue stop when HW does it automatically Fabrizio Castro
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Fabrizio Castro @ 2018-05-30 10:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Fabrizio Castro, Wolfram Sang, linux-i2c, Chris Paterson,
	Biju Das, stable, Ben Hutchings

Hello Greg,

Wolfram recommends the backporting of this series in order to
improve the situation with a race condition.
Do you think you can take this series for 4.4 stable?

This series applies on 4.4.133, and depends on series:
"Fix R-Car I2C data byte sent twice issue"

Thanks,
Fab

Wolfram Sang (3):
  i2c: rcar: don't issue stop when HW does it automatically
  i2c: rcar: check master irqs before slave irqs
  i2c: rcar: revoke START request early

 drivers/i2c/busses/i2c-rcar.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

-- 
2.7.4

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

* [PATCH 1/3] i2c: rcar: don't issue stop when HW does it automatically
  2018-05-30 10:07 [PATCH 0/3] Fix double address byte issue Fabrizio Castro
@ 2018-05-30 10:07 ` Fabrizio Castro
  2018-05-30 10:07 ` [PATCH 2/3] i2c: rcar: check master irqs before slave irqs Fabrizio Castro
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Fabrizio Castro @ 2018-05-30 10:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Wolfram Sang, Wolfram Sang, linux-i2c, Chris Paterson, Biju Das,
	Fabrizio Castro, stable, Ben Hutchings

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

commit d89667b14f9d13b684287f6189ca209af5feee43 upstream.

The manual says (55.4.8.6) that HW does automatically send STOP after
NACK was received. My measuerments confirm that.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
[fabrizio: cherry-pick to 4.4]
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Reviewed-by: Biju Das <biju.das@bp.renesas.com>
---
 drivers/i2c/busses/i2c-rcar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 44662e2..6f23b97 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -455,8 +455,8 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 
 	/* Nack */
 	if (msr & MNR) {
-		/* go to stop phase */
-		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
+		/* HW automatically sends STOP after received NACK */
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 		rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
 		rcar_i2c_flags_set(priv, ID_NACK);
 		goto out;
-- 
2.7.4

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

* [PATCH 2/3] i2c: rcar: check master irqs before slave irqs
  2018-05-30 10:07 [PATCH 0/3] Fix double address byte issue Fabrizio Castro
  2018-05-30 10:07 ` [PATCH 1/3] i2c: rcar: don't issue stop when HW does it automatically Fabrizio Castro
@ 2018-05-30 10:07 ` Fabrizio Castro
  2018-05-30 10:07 ` [PATCH 3/3] i2c: rcar: revoke START request early Fabrizio Castro
  2018-06-02 13:17 ` [PATCH 0/3] Fix double address byte issue Greg Kroah-Hartman
  3 siblings, 0 replies; 5+ messages in thread
From: Fabrizio Castro @ 2018-05-30 10:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Wolfram Sang, Wolfram Sang, linux-i2c, Chris Paterson, Biju Das,
	Fabrizio Castro, stable, Ben Hutchings

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

commit c3be0af15959e11fa535d5332ab3d7cf34abd09b upstream.

Due to the HW design, master IRQs are timing critical, so give them
precedence over slave IRQ.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
[fabrizio: cherry-pick to 4.4]
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Reviewed-by: Biju Das <biju.das@bp.renesas.com>
---
 drivers/i2c/busses/i2c-rcar.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 6f23b97..ee3e02a 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -432,19 +432,17 @@ static bool rcar_i2c_slave_irq(struct rcar_i2c_priv *priv)
 static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 {
 	struct rcar_i2c_priv *priv = ptr;
-	irqreturn_t result = IRQ_HANDLED;
 	u32 msr;
 
-	if (rcar_i2c_slave_irq(priv))
-		goto exit;
-
 	msr = rcar_i2c_read(priv, ICMSR);
 
 	/* Only handle interrupts that are currently enabled */
 	msr &= rcar_i2c_read(priv, ICMIER);
 	if (!msr) {
-		result = IRQ_NONE;
-		goto exit;
+		if (rcar_i2c_slave_irq(priv))
+			return IRQ_HANDLED;
+
+		return IRQ_NONE;
 	}
 
 	/* Arbitration lost */
@@ -481,8 +479,7 @@ out:
 		wake_up(&priv->wait);
 	}
 
-exit:
-	return result;
+	return IRQ_HANDLED;
 }
 
 static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
-- 
2.7.4

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

* [PATCH 3/3] i2c: rcar: revoke START request early
  2018-05-30 10:07 [PATCH 0/3] Fix double address byte issue Fabrizio Castro
  2018-05-30 10:07 ` [PATCH 1/3] i2c: rcar: don't issue stop when HW does it automatically Fabrizio Castro
  2018-05-30 10:07 ` [PATCH 2/3] i2c: rcar: check master irqs before slave irqs Fabrizio Castro
@ 2018-05-30 10:07 ` Fabrizio Castro
  2018-06-02 13:17 ` [PATCH 0/3] Fix double address byte issue Greg Kroah-Hartman
  3 siblings, 0 replies; 5+ messages in thread
From: Fabrizio Castro @ 2018-05-30 10:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Wolfram Sang, Wolfram Sang, linux-i2c, Chris Paterson, Biju Das,
	Fabrizio Castro, stable, Ben Hutchings

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

commit 52df445f29b79006d8b2dcd129152987c0d3bd64 upstream.

If we don't clear START generation as soon as possible, it may cause
another message to be generated, e.g. when receiving NACK in address
phase. To keep the race window as small as possible, we clear it right
at the beginning of the interrupt. We don't need any checks since we
always want to stop START and STOP generation on the next occasion after
we started it.

This patch improves the situation but sadly does not completely fix it.
It is still to be researched if we can do better given this HW design.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
[fabrizio: cherry-pick to 4.4]
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Reviewed-by: Biju Das <biju.das@bp.renesas.com>
---
 drivers/i2c/busses/i2c-rcar.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index ee3e02a..6f89484 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -83,6 +83,7 @@
 
 #define RCAR_BUS_PHASE_START	(MDBS | MIE | ESG)
 #define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
+#define RCAR_BUS_MASK_DATA	(~(ESG | FSB) & 0xFF)
 #define RCAR_BUS_PHASE_STOP	(MDBS | MIE | FSB)
 
 #define RCAR_IRQ_SEND	(MNR | MAL | MST | MAT | MDE)
@@ -289,13 +290,6 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 	if (!(msr & MDE))
 		return;
 
-	/*
-	 * If address transfer phase finished,
-	 * goto data phase.
-	 */
-	if (msr & MAT)
-		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
-
 	if (priv->pos < msg->len) {
 		/*
 		 * Prepare next data to ICRXTX register.
@@ -345,11 +339,7 @@ static void rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 		return;
 
 	if (msr & MAT) {
-		/*
-		 * Address transfer phase finished,
-		 * but, there is no data at this point.
-		 * Do nothing.
-		 */
+		/* Address transfer phase finished, but no data at this point. */
 	} else if (priv->pos < msg->len) {
 		/*
 		 * get received data
@@ -365,8 +355,6 @@ static void rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 	 */
 	if (priv->pos + 1 >= msg->len)
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
-	else
-		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 
 	if (priv->pos == msg->len && !(priv->flags & ID_LAST_MSG))
 		rcar_i2c_next_msg(priv);
@@ -432,7 +420,11 @@ static bool rcar_i2c_slave_irq(struct rcar_i2c_priv *priv)
 static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 {
 	struct rcar_i2c_priv *priv = ptr;
-	u32 msr;
+	u32 msr, val;
+
+	/* Clear START or STOP as soon as we can */
+	val = rcar_i2c_read(priv, ICMCR);
+	rcar_i2c_write(priv, ICMCR, val & RCAR_BUS_MASK_DATA);
 
 	msr = rcar_i2c_read(priv, ICMSR);
 
@@ -454,7 +446,6 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 	/* Nack */
 	if (msr & MNR) {
 		/* HW automatically sends STOP after received NACK */
-		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 		rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
 		rcar_i2c_flags_set(priv, ID_NACK);
 		goto out;
-- 
2.7.4

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

* Re: [PATCH 0/3] Fix double address byte issue
  2018-05-30 10:07 [PATCH 0/3] Fix double address byte issue Fabrizio Castro
                   ` (2 preceding siblings ...)
  2018-05-30 10:07 ` [PATCH 3/3] i2c: rcar: revoke START request early Fabrizio Castro
@ 2018-06-02 13:17 ` Greg Kroah-Hartman
  3 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2018-06-02 13:17 UTC (permalink / raw)
  To: Fabrizio Castro
  Cc: Wolfram Sang, linux-i2c, Chris Paterson, Biju Das, stable, Ben Hutchings

On Wed, May 30, 2018 at 11:07:10AM +0100, Fabrizio Castro wrote:
> Hello Greg,
> 
> Wolfram recommends the backporting of this series in order to
> improve the situation with a race condition.
> Do you think you can take this series for 4.4 stable?
> 
> This series applies on 4.4.133, and depends on series:
> "Fix R-Car I2C data byte sent twice issue"

All now queued up, thanks.

greg k-h

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

end of thread, other threads:[~2018-06-02 13:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-30 10:07 [PATCH 0/3] Fix double address byte issue Fabrizio Castro
2018-05-30 10:07 ` [PATCH 1/3] i2c: rcar: don't issue stop when HW does it automatically Fabrizio Castro
2018-05-30 10:07 ` [PATCH 2/3] i2c: rcar: check master irqs before slave irqs Fabrizio Castro
2018-05-30 10:07 ` [PATCH 3/3] i2c: rcar: revoke START request early Fabrizio Castro
2018-06-02 13:17 ` [PATCH 0/3] Fix double address byte issue Greg Kroah-Hartman

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.