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

[Andrew Dyer]

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>

-- 
Hardware, n.:
        The parts of a computer system that can be kicked.
-------------- next part --------------
Index: common/soft_i2c.c
===================================================================
RCS file: /home/cvsroot/Projects/u-boot/common/soft_i2c.c,v
retrieving revision 1.5
diff -p -u -r1.5 soft_i2c.c
--- common/soft_i2c.c	25 Feb 2007 01:30:17 -0000	1.5
+++ common/soft_i2c.c	21 May 2007 21:49:01 -0000
@@ -183,6 +183,9 @@ static void send_ack(int ack)
 	I2C_DELAY;
 	I2C_SCL(0);
 	I2C_DELAY;
+	I2C_SDA(1);
+	I2C_TRISTATE;
+	I2C_DELAY;
 }
 
 
Index: README
===================================================================
RCS file: /home/cvsroot/Projects/u-boot/README,v
retrieving revision 1.3
diff -p -u -r1.3 README
--- README	23 Apr 2007 23:09:10 -0000	1.3
+++ README	21 May 2007 21:49:35 -0000
@@ -1279,7 +1279,7 @@ The following options need to be configu
 
 		The code necessary to make the I2C data line tri-stated
 		(inactive).  If the data line is open collector, this
-		define can be null.
+		define should be the same as I2C_SDA(1).
 
 		eg: #define I2C_TRISTATE (immr->im_cpm.cp_pbdir &= ~PB_SDA)