[U-Boot] Odd value for I2C_TIMEOUT in fsl_i2c.c

[Timur Tabi]

On Fri, Sep 4, 2009 at 4:25 AM, Wolfgang Denk<wd at denx.de> wrote:

> There are actually two parts in Timur's mail:
>
> 1) First part is the question if the timeout, which is currently set
>   to 250 us, should be raised to 1,000 us.
>
>   I cannot answer this question. I don't even understand why the
>   i2c_wait4bus() function is needed at all.

Can you explain?  I don't know enough about the I2C protocol.  Why is
i2c_wait4bus unnecessary?

> 2) The second part is if the timeout definition should be changed
>   from being based on CONFIG_SYS_HZ to a plain numeric constant.
>   Assuming I2C_TIMEOUT is intended to be a period of time (as the
>   name suggests), then "(CONFIG_SYS_HZ / 4)" is clearly wrong as
>   it's a frequency and not a time.
>
>   In this case, a comment should be added explainign what
>   I2C_TIMEOUT means.

It took me a while to track it down, but I finally found the commit
that make the change: SHA 09d318a8bb1444ec92e31cafcdba877eb9409e58,
from Kumar about a year ago:

    fsl_i2c: Use timebase timer functions instead of get_timer()

    The current implementation of get_timer() is only really useful after we
    have relocated u-boot to memory.  The i2c code is used before that as part
    of the SPD DDR setup.

    We actually have a bug when using the get_timer() code before relocation
    because the .bss hasn't been setup and thus we could be reading/writing
    a random location (probably in flash).

    Signed-off-by: Kumar Gala <galak at kernel.crashing.org>

Specifically, it does this:

 	while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) {
-		if (get_timer(timeval) > I2C_TIMEOUT) {
+		if ((get_ticks() - timeval) > usec2ticks(I2C_TIMEOUT))
 			return -1;
-		}
 	}

Kumar, any thoughts?  Is there something sneaky going on here, or did
you just misinterpret the value of I2C_TIMEOUT?

-- 
Timur Tabi
Linux kernel developer at Freescale