[U-Boot] [PATCH] i2c: fix SDA contention in read_byte()

[Andrew Dyer]

On Tue, Jul 6, 2010 at 1:14 AM, Thomas Chou <thomas at wytron.com.tw> wrote:
> We should not set SDA after TRISTATE, as it results in contention.
>
> Signed-off-by: Thomas Chou <thomas at 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.