From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heiko Schocher <hs@denx.de>
Date: Fri, 04 Sep 2009 09:16:21 +0200
Subject: [U-Boot] Odd value for I2C_TIMEOUT in fsl_i2c.c
In-Reply-To: <4A9FDF1E.4090908@freescale.com>
References: <4A9FDF1E.4090908@freescale.com>
Message-ID: <4AA0BEC5.3010505@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hello Timur,

Timur Tabi wrote:
> Currently we define I2C_TIMEOUT like this:
> 
> #define I2C_TIMEOUT	(CONFIG_SYS_HZ / 4)
> 
> I'm seeing some I2C instability on a new board I'm working on, especially with SPD.  If I change the above to 
> 
> #define I2C_TIMEOUT	(CONFIG_SYS_HZ / 2)
> 
> The problems go away (or at least, so far appear to).  Can someone tell me why we choose (CONFIG_SYS_HZ / 4) to begin with?  The way we use I2C_TIMEOUT is confusing:
> 
> 	while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) {
> 		if ((get_ticks() - timeval) > usec2ticks(I2C_TIMEOUT))
> 			return -1;
> 	}
> 
> CONFIG_HZ is 1000, so I2C_TIMEOUT is equal to 250.  However, the way it's used, 250 isn't the number of ticks per second, it's used as number of microseconds.  If CONFIG_HZ is changed to 100, does that mean that we want to call usec2ticks(25)?

This seems like a Bug to me.

> I think what we should be doing is this:
> 
> #define I2C_TIMEOUT	1000
> 
> Surely, one millisecond is not too long of a timeout?

I think this should be Ok. Can you provide a patch?
Thanks for catching this.

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany