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 <sba...@denx.de>
> CC: Heiko Schocher <h...@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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to