diff mbox

i2c-mpc: Do not generate STOP after read.

Message ID 1253620242-18461-1-git-send-email-Joakim.Tjernlund@transmode.se (mailing list archive)
State Accepted, archived
Delegated to: Grant Likely
Headers show

Commit Message

Joakim Tjernlund Sept. 22, 2009, 11:50 a.m. UTC 
The driver always ends a read with a STOP condition which
breaks subsequent I2C reads/writes in the same transaction as
these expect to do a repeated START(ReSTART).

This will also help I2C multimaster as the bus will not be released
after the first read, but when the whole transaction ends.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---

This should also fix a problem reported by Esben Haabendal:

[PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg

This fixes MAL (arbitration lost) bug caused by illegal use of
RSTA (repeated START) after STOP condition generated after last byte
of reads. With this patch, it is possible to do an i2c_transfer() with
additional i2c_msg's following the I2C_M_RD messages.

It still needs to be resolved if it is possible to fix this issue
by removing the STOP condition after reads in a robust way.

Signed-off-by: Esben Haabendal <eha@doredevelopment.dk>

 drivers/i2c/busses/i2c-mpc.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

Comments

Wolfgang Grandegger Sept. 25, 2009, 10:01 a.m. UTC | #1 
Joakim Tjernlund wrote:
> The driver always ends a read with a STOP condition which
> breaks subsequent I2C reads/writes in the same transaction as
> these expect to do a repeated START(ReSTART).
> 
> This will also help I2C multimaster as the bus will not be released
> after the first read, but when the whole transaction ends.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Tested-by: Wolfgang Grandegger <wg@grandegger.com>

on a MPC8548 board with an up-to-date kernel. I did not realize any
problems.

Wolfgang.
Joakim Tjernlund Sept. 27, 2009, 10:26 p.m. UTC | #2 
Jean, I just noticed you pull request for i2c on LKML but I didn't see this
patch nor have I got any feedback from you. What is your view?

   Jocke

Wolfgang Grandegger <wg@grandegger.com> wrote on 25/09/2009 12:01:17:
>
> Joakim Tjernlund wrote:
> > The driver always ends a read with a STOP condition which
> > breaks subsequent I2C reads/writes in the same transaction as
> > these expect to do a repeated START(ReSTART).
> >
> > This will also help I2C multimaster as the bus will not be released
> > after the first read, but when the whole transaction ends.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Tested-by: Wolfgang Grandegger <wg@grandegger.com>
>
> on a MPC8548 board with an up-to-date kernel. I did not realize any
> problems.
>
> Wolfgang.
>
Jean Delvare Sept. 28, 2009, 7:28 a.m. UTC | #3 
On Mon, 28 Sep 2009 00:26:54 +0200, Joakim Tjernlund wrote:
> Jean, I just noticed you pull request for i2c on LKML but I didn't see this
> patch nor have I got any feedback from you. What is your view?

My view is that i2c-mpc is nor under my jurisdiction, so I did not, and
will not, pay any attention to it.
Joakim Tjernlund Sept. 28, 2009, 7:30 a.m. UTC | #4 
Jean Delvare <khali@linux-fr.org> wrote on 28/09/2009 09:28:09:
>
> On Mon, 28 Sep 2009 00:26:54 +0200, Joakim Tjernlund wrote:
> > Jean, I just noticed you pull request for i2c on LKML but I didn't see this
> > patch nor have I got any feedback from you. What is your view?
>
> My view is that i2c-mpc is nor under my jurisdiction, so I did not, and
> will not, pay any attention to it.

Ah, that explains it. Who then will look after i2c-mpc? Kumar?
Peter Korsgaard Sept. 28, 2009, 7:33 a.m. UTC | #5 
>>>>> "Joakim" == Joakim Tjernlund <joakim.tjernlund@transmode.se> writes:

Hi,

 Joakim> Ah, that explains it. Who then will look after i2c-mpc? Kumar?

Ben Dooks (embedded i2c maintainer). He's afaik coming home today, so
give him a few days to catch up on mails.
Jean Delvare Sept. 28, 2009, 7:34 a.m. UTC | #6 
On Mon, 28 Sep 2009 09:30:32 +0200, Joakim Tjernlund wrote:
> Jean Delvare <khali@linux-fr.org> wrote on 28/09/2009 09:28:09:
> >
> > On Mon, 28 Sep 2009 00:26:54 +0200, Joakim Tjernlund wrote:
> > > Jean, I just noticed you pull request for i2c on LKML but I didn't see this
> > > patch nor have I got any feedback from you. What is your view?
> >
> > My view is that i2c-mpc is nor under my jurisdiction, so I did not, and
> > will not, pay any attention to it.
> 
> Ah, that explains it. Who then will look after i2c-mpc? Kumar?

What does MAINTAINERS say?
Joakim Tjernlund Sept. 28, 2009, 7:43 a.m. UTC | #7 
Jean Delvare <khali@linux-fr.org> wrote on 28/09/2009 09:34:34:
>
> On Mon, 28 Sep 2009 09:30:32 +0200, Joakim Tjernlund wrote:
> > Jean Delvare <khali@linux-fr.org> wrote on 28/09/2009 09:28:09:
> > >
> > > On Mon, 28 Sep 2009 00:26:54 +0200, Joakim Tjernlund wrote:
> > > > Jean, I just noticed you pull request for i2c on LKML but I didn't see this
> > > > patch nor have I got any feedback from you. What is your view?
> > >
> > > My view is that i2c-mpc is nor under my jurisdiction, so I did not, and
> > > will not, pay any attention to it.
> >
> > Ah, that explains it. Who then will look after i2c-mpc? Kumar?
>
> What does MAINTAINERS say?

I did a grep for i2c-mpc and nothing turned up. Now that someone mentioned
Ben Dooks I found it.
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index cdb1858..88ae582 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -369,9 +369,6 @@  static int mpc_write(struct mpc_i2c *i2c, int target,
 	unsigned timeout = i2c->adap.timeout;
 	u32 flags = restart ? CCR_RSTA : 0;
 
-	/* Start with MEN */
-	if (!restart)
-		writeccr(i2c, CCR_MEN);
 	/* Start as master */
 	writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags);
 	/* Write target byte */
@@ -400,9 +397,6 @@  static int mpc_read(struct mpc_i2c *i2c, int target,
 	int i, result;
 	u32 flags = restart ? CCR_RSTA : 0;
 
-	/* Start with MEN */
-	if (!restart)
-		writeccr(i2c, CCR_MEN);
 	/* Switch to read - restart */
 	writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags);
 	/* Write target address byte - this time with the read flag set */
@@ -429,9 +423,9 @@  static int mpc_read(struct mpc_i2c *i2c, int target,
 		/* Generate txack on next to last byte */
 		if (i == length - 2)
 			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_TXAK);
-		/* Generate stop on last byte */
+		/* Do not generate stop on last byte */
 		if (i == length - 1)
-			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_TXAK);
+			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX);
 		data[i] = readb(i2c->base + MPC_I2C_DR);
 	}