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

[Reinhard Meyer]

Thomas Chou wrote:
> Reinhard Meyer wrote:
>   
>> Whenever possible by the hardware, I make I2C_SDA/SCL(1) do a tri-state and
>> I2C_TRISTATE and I2C_ACTIVE are empty.
>>     
>
> Dear Mike,
>
> I traced the i2c-gpio.c of linux and realized that there are potential 
> bus contention with the current soft_i2c.c if the ports are not 
> open-drained.
>
> Reinhard suggested a solution, which was similar to what linux driver 
> does. So I would withdraw my SDA patch.
>
> For our i2c gpio framework, I added these changes and tested on my 
> boards. Please check if it works on yours.
>
> # ifndef I2C_ACTIVE
> #  define I2C_ACTIVE do {} while (0)
> # endif
>
> # ifndef I2C_TRISTATE
> #  define I2C_TRISTATE do {} while (0)
> # endif
>
> # ifndef I2C_SDA
> #  define I2C_SDA(bit) \
> 	if (bit) {						\
> 		gpio_direction_input(CONFIG_SOFT_I2C_GPIO_SDA);	\
> 	} else {						\
> 		gpio_direction_output(CONFIG_SOFT_I2C_GPIO_SDA, 0);\
> 	}
> # endif
>
> I didn't tristate SCL(1) because it cannot be tristated on some nios2 
> boards. As soft_i2c of u-boot didn't support clock stretching, it 
> shouldn't matter.
>   
Its even simpler, provided the  hardware can do open collector. Here was 
my solution for AVR32AP7000:

int board_early_init_f(void)
{
    ...
#ifdef CONFIG_CMD_I2C
    /* set SCL and SDA to open drain gpio */
    portmux_select_gpio(PORTMUX_PORT_A,(1<<SDA_PIN),
        PORTMUX_DIR_OUTPUT|PORTMUX_INIT_LOW|PORTMUX_OPEN_DRAIN);
    portmux_select_gpio(PORTMUX_PORT_A,(1<<SCL_PIN),
        PORTMUX_DIR_OUTPUT|PORTMUX_INIT_LOW|PORTMUX_OPEN_DRAIN);
    /* initialize i2c. note: params ignored in SOFT I2C */
    i2c_init(0, 0);
#endif
    ...
}


/* I2C access functions */
#ifdef CONFIG_CMD_I2C

int iic_read(void)
{
    return pio_get_input_value(GPIO_PIN_PA(SDA_PIN));
}

void iic_sda(int bit)
{
    pio_set_output_value(GPIO_PIN_PA(SDA_PIN),bit);
}

void iic_scl(int bit)
{
    pio_set_output_value(GPIO_PIN_PA(SCL_PIN),bit);
}

#endif /* CONFIG_CMD_I2C */


I realize now, that in the init I should set the initial port value to 
high, but AP7000 is history for me.
I did make the access functions real functions and let the macros call them:

#define SDA_PIN                6
#define SCL_PIN                7
#define I2C_SOFT_DECLARATIONS        int iic_read(void);\
                    void iic_sda(int);\
                    void iic_scl(int);
#define I2C_ACTIVE
#define I2C_TRISTATE
#define I2C_READ            iic_read()
#define I2C_SDA(bit)            iic_sda(bit)
#define I2C_SCL(bit)            iic_scl(bit)
#define I2C_DELAY            udelay(3)

THAT, of course is a personal preference. As simple as the functions 
are, they could be inline or the macro itself ;)

Reinhard