[U-Boot-Users] [Fwd: Re: [PATCH] bug in README and soft_i2c.c]

[Joakim Tjernlund]

Wolfgang,
I think the below patch is ready for wider audience and
ask that you include it in your tree.
I and 2 others has tested it to some extent and I don't think
it will receive any more testing outside the u-boot tree.

   Jocke
-------- Forwarded Message --------
From: Joakim Tjernlund <joakim.tjernlund at transmode.se>
Reply-To: joakim.tjernlund at transmode.se
To: Andrew Dyer <amdyer at gmail.com>
Cc: U-Boot list <u-boot-users at lists.sourceforge.net>
Subject: Re: [U-Boot-Users] [PATCH] bug in README and soft_i2c.c
Date: Tue, 22 May 2007 18:23:31 +0200

On Mon, 2007-05-21 at 17:04 -0500, Andrew Dyer wrote:
> Hi, I noticed a bug in soft_i2c.c (or in the README depending on how
> you think about it).
> 
> If CONFIG_SOFT_I2C is defined and using open-collector I/O, then the
> README says that I2C_TRISTATE can be defined as null.  If this is the
> case, then doing:
> 
> imd 29 0.0 2
> 
> will cause the following chain of calls in soft_i2c.c:
> 
> send_start()
> write_byte()
> read_byte()
> read_byte()
> send_stop()
> 
> The first read_byte() calls send_ack(0).  send_ack(0) drives the data
> line low without releasing it.  The following call to read_byte()
> calls I2C_TRISTATE, but in this case it is defined as NULL per the
> README.  This leaves the data line driven low during the read of the
> 2nd byte causing data^h^h^h^hhair loss that I can ill afford :-)
> 
> I propose the following fix - modify the README to make say that
> I2C_TRISTATE should be defined even for open-collector setups, and add
> code to soft_i2c.c to explictly tristate the data at the end of
> send_ack(0)
> 
> Signed-off-by: Andrew Dyer <amdyer at gmail.com>


I think the README w.r.t I2C_TRISTATE is OK as is(don't change it). I do
think the soft i2c driver is broken in several places w.r.t
IC2_ACTIVE/I2C_TRISTATE and I2C reset sequence. The below patch is an
attempt to fix it.

Signed-off-by: Joakim Tjernlund <joakim.tjernlund at transmode.se>

diff --git a/common/soft_i2c.c b/common/soft_i2c.c
index edad51b..ed3f8f8 100644
--- a/common/soft_i2c.c
+++ b/common/soft_i2c.c
@@ -100,15 +100,18 @@ static void send_reset(void)
 #endif
 	I2C_TRISTATE;
 	for(j = 0; j < 9; j++) {
+		if(I2C_READ)
+			send_start();
 		I2C_SCL(0);
 		I2C_DELAY;
+		I2C_TRISTATE;
+		I2C_SDA(1);
 		I2C_DELAY;
 		I2C_SCL(1);
 		I2C_DELAY;
 		I2C_DELAY;
 	}
 	send_stop();
-	I2C_TRISTATE;
 }
 
 /*-----------------------------------------------------------------------
@@ -124,12 +127,13 @@ static void send_start(void)
 #endif
 
 	I2C_DELAY;
+	I2C_TRISTATE;
 	I2C_SDA(1);
-	I2C_ACTIVE;
 	I2C_DELAY;
 	I2C_SCL(1);
 	I2C_DELAY;
 	I2C_SDA(0);
+	I2C_ACTIVE;
 	I2C_DELAY;
 }
 
@@ -152,9 +156,9 @@ static void send_stop(void)
 	I2C_DELAY;
 	I2C_SCL(1);
 	I2C_DELAY;
+	I2C_TRISTATE;
 	I2C_SDA(1);
 	I2C_DELAY;
-	I2C_TRISTATE;
 }
 

@@ -172,14 +176,17 @@ static void send_ack(int ack)
 
 	I2C_SCL(0);
 	I2C_DELAY;
-	I2C_ACTIVE;
 	I2C_SDA(ack);
+	I2C_ACTIVE;
 	I2C_DELAY;
 	I2C_SCL(1);
 	I2C_DELAY;
 	I2C_DELAY;
 	I2C_SCL(0);
 	I2C_DELAY;
+	I2C_TRISTATE;
+	I2C_SDA(1);
+	I2C_DELAY;
 }
 

@@ -215,8 +222,8 @@ static int write_byte(uchar data)
 	 */
 	I2C_SCL(0);
 	I2C_DELAY;
-	I2C_SDA(1);
 	I2C_TRISTATE;
+	I2C_SDA(1);
 	I2C_DELAY;
 	I2C_SCL(1);
 	I2C_DELAY;
@@ -224,7 +231,6 @@ static int write_byte(uchar data)
 	nack = I2C_READ;
 	I2C_SCL(0);
 	I2C_DELAY;
-	I2C_ACTIVE;
 
 	return(nack);	/* not a nack is an ack */
 }
@@ -249,6 +255,7 @@ static uchar read_byte(int ack)
 	 * Read 8 bits, MSB first.
 	 */
 	I2C_TRISTATE;
+	I2C_SDA(1);
 	data = 0;
 	for(j = 0; j < 8; j++) {
 		I2C_SCL(0);


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users