On Tue, Jul 6, 2010 at 1:14 AM, Thomas Chou <tho...@wytron.com.tw> wrote:
> We should not set SDA after TRISTATE, as it results in contention.
>
> Signed-off-by: Thomas Chou <tho...@wytron.com.tw>
> ---
>  drivers/i2c/soft_i2c.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/soft_i2c.c b/drivers/i2c/soft_i2c.c
> index 847db76..344b7f8 100644
> --- a/drivers/i2c/soft_i2c.c
> +++ b/drivers/i2c/soft_i2c.c
> @@ -305,8 +305,8 @@ static uchar read_byte(int ack)
>        /*
>         * Read 8 bits, MSB first.
>         */
> -       I2C_TRISTATE;
>        I2C_SDA(1);
> +       I2C_TRISTATE;
>        data = 0;
>        for(j = 0; j < 8; j++) {
>                I2C_SCL(0);
> --

NAK.

I2C_TRISTATE is supposed to be persistent until I2C_ACTIVE is called,
so in the original code it should still be in effect when I2C_SDA(1)
is executed and there should be no contention.  This patch causes the
code to actively drive SDA high at the same time the addressed device
might be driving it low, causing contention until the I2C_TRISTATE
takes effect.

In some sense the code is misleadingly written, as it is not allowed
in the spec to actively drive a '1' on the bus, a chip is only
supposed to drive 'z' or '0', and the platform is supposed to provide
the pullup current.  This comes more into play if the i2c bus supports
clock stretching or arbitration among multiple masters.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to