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@transmode.se Reply-To: joakim.tjernlund@transmode.se To: Andrew Dyer amdyer@gmail.com Cc: U-Boot list u-boot-users@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@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@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@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users