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

[Joakim Tjernlund]

On Tue, 2007-05-22 at 18:23 +0200, Joakim Tjernlund wrote:
> 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, but I havn't tested it.

I have tested the non open-collector on my HW and that works fine.
If someone can test the open-collector case that would be good so
this can go into u-boot.

 Jocke

> 
> 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
>