[U-Boot] [PATCH V3 05/11] I2C: mxc_i2c: address failure with mx35 processor

Heiko Schocher
2011-01-21T06:36:27Z

Hello Stefano,

just a question ...

Stefano Babic wrote:
> There is sporadic failures when more as one I2C slave
> is on the bus and the processor tries to communicate
> with more as one slave.
> The problem was seen on a mx35pdk (two I2C slaves,
> PMIC controller and CAN/RTC chip).
>
> The current driver uses the IIF bit in the status register
> to check if the bus is busy or not. According to the manual,
> this is not correct, because the IIB bit should be checked.
> Not only, to check if a transfer is finished must be checked
> the ICF bit, and this is not tested at all.
>
> This patch comes from analyse with a corresponding driver
> provided by Freescale as part of the LTIB tool. Comparing
> the two drivers, it appears that the current u-boot driver checks
> the wrong bits, and depending on race condition, the transfer
> can be successful or not.
>
> The patch gets rid also of own debug function (DPRINTF),
> replaced with the general debug().
>
> Tested on Freescale mx35pdk.
>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Heiko Schocher <hs@denx.de>
> ---
> Changes:
>
> Wolfgang Denk:
> - change commit message explaining the problem
> and the changes
> - describe in commit message the drop of DPRINTF
>
> drivers/i2c/mxc_i2c.c | 86 ++++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 68 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index fd6db18..c5ec486 100755
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
[...]
> @@ -116,31 +113,61 @@ void i2c_init(int speed, int unused)
> i2c_reset();
> }
>
> +static int wait_idle(void)
> +{
> + int timeout = I2C_MAX_TIMEOUT;
> +
> + while ((readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout) {
> + writew(0, I2C_BASE + I2SR);
> + udelay(1);
> + }
> + return timeout ? timeout : (!(readw(I2C_BASE + I2SR) & I2SR_IBB));
> +}
> +
> static int wait_busy(void)
> {
> - int timeout = 10000;
> + int timeout = I2C_MAX_TIMEOUT;
>
> - while (!(readw(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
> + while (!(readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout)
> udelay(1);
> writew(0, I2C_BASE + I2SR); /* clear interrupt */
>
> return timeout;
> }
>
> +static int wait_complete(void)
> +{
> + int timeout = I2C_MAX_TIMEOUT;
> +
> + while ((!(readw(I2C_BASE + I2SR) & I2SR_ICF)) && (--timeout)) {
> + writew(0, I2C_BASE + I2SR);
> + udelay(1);
> + }
> + udelay(200);

Why is this delay necessary? Why exactly 200? Is this documented
somewhere in the doc?

bye,
Heiko