[U-Boot] [PATCH] fsl_i2c: increase I2C timeout values and make them configurable

[U-Boot] [PATCH] fsl_i2c: increase I2C timeout values and make them configurable

[Timur Tabi]

The value of I2C_TIMEOUT in fsl_i2c.c has several problems.  First, it is
defined as CONFIG_HZ/4, but it is used as a count of microseconds, so it makes
no sense to derive it from a clock rate.  Second, the current value (250) is
too low for some boards, so it needs to be increased.  Third, the timeout
necessary for multiple-master arbitration is larger than the timeout for basic
read/write operations, so we shouldn't have a single constant for both timeouts.
Finally, it would be nice if we could override these values on a per-board
basis.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/i2c/fsl_i2c.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
index ce0f301..47bbf79 100644
--- a/drivers/i2c/fsl_i2c.c
+++ b/drivers/i2c/fsl_i2c.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2006 Freescale Semiconductor, Inc.
+ * Copyright 2006,2009 Freescale Semiconductor, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -26,7 +26,21 @@
 #include <asm/io.h>
 #include <asm/fsl_i2c.h>	/* HW definitions */
 
-#define I2C_TIMEOUT	(CONFIG_SYS_HZ / 4)
+/* The maximum number of microseconds we will wait until another master has
+ * released the bus.  If not defined in the board header file, then use a
+ * generic value.
+ */
+#ifndef CONFIG_I2C_MBB_TIMEOUT
+#define CONFIG_I2C_MBB_TIMEOUT	100000
+#endif
+
+/* The maximum number of microseconds we will wait for a read or write
+ * operation to complete.  If not defined in the board header file, then use a
+ * generic value.
+ */
+#ifndef CONFIG_I2C_TIMEOUT
+#define CONFIG_I2C_TIMEOUT	10000
+#endif
 
 #define I2C_READ_BIT  1
 #define I2C_WRITE_BIT 0
@@ -213,9 +227,10 @@ static __inline__ int
 i2c_wait4bus(void)
 {
 	unsigned long long timeval = get_ticks();
+	const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
 
 	while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) {
-		if ((get_ticks() - timeval) > usec2ticks(I2C_TIMEOUT))
+		if ((get_ticks() - timeval) > timeout)
 			return -1;
 	}
 
@@ -227,6 +242,7 @@ i2c_wait(int write)
 {
 	u32 csr;
 	unsigned long long timeval = get_ticks();
+	const unsigned long long timeout = usec2ticks(CONFIG_I2C_TIMEOUT);
 
 	do {
 		csr = readb(&i2c_dev[i2c_bus_num]->sr);
@@ -251,7 +267,7 @@ i2c_wait(int write)
 		}
 
 		return 0;
-	} while ((get_ticks() - timeval) < usec2ticks(I2C_TIMEOUT));
+	} while ((get_ticks() - timeval) < timeout);
 
 	debug("i2c_wait: timed out\n");
 	return -1;
-- 
1.6.0.6

[U-Boot] [PATCH] fsl_i2c: increase I2C timeout values and make them configurable

[Wolfgang Denk]

Dear Timur Tabi,

In message <1252099715-19255-1-git-send-email-timur@freescale.com> you wrote:
> The value of I2C_TIMEOUT in fsl_i2c.c has several problems.  First, it is
> defined as CONFIG_HZ/4, but it is used as a count of microseconds, so it makes
> no sense to derive it from a clock rate.  Second, the current value (250) is
> too low for some boards, so it needs to be increased.  Third, the timeout
> necessary for multiple-master arbitration is larger than the timeout for basic
> read/write operations, so we shouldn't have a single constant for both timeouts.
> Finally, it would be nice if we could override these values on a per-board
> basis.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>  drivers/i2c/fsl_i2c.c |   24 ++++++++++++++++++++----
>  1 files changed, 20 insertions(+), 4 deletions(-)

Acked-by: Wolfgang Denk <wd@denx.de>


Thanks!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"This isn't brain surgery; it's just television."   - David Letterman

[U-Boot] [PATCH] fsl_i2c: increase I2C timeout values and make them configurable

[Peter Tyser]

On Fri, 2009-09-04 at 23:59 +0200, Wolfgang Denk wrote:
> Dear Timur Tabi,
> 
> In message <1252099715-19255-1-git-send-email-timur@freescale.com> you wrote:
> > The value of I2C_TIMEOUT in fsl_i2c.c has several problems.  First, it is
> > defined as CONFIG_HZ/4, but it is used as a count of microseconds, so it makes
> > no sense to derive it from a clock rate.  Second, the current value (250) is
> > too low for some boards, so it needs to be increased.  Third, the timeout
> > necessary for multiple-master arbitration is larger than the timeout for basic
> > read/write operations, so we shouldn't have a single constant for both timeouts.
> > Finally, it would be nice if we could override these values on a per-board
> > basis.
> > 
> > Signed-off-by: Timur Tabi <timur@freescale.com>
> > ---
> >  drivers/i2c/fsl_i2c.c |   24 ++++++++++++++++++++----
> >  1 files changed, 20 insertions(+), 4 deletions(-)
> 
> Acked-by: Wolfgang Denk <wd@denx.de>

Tested-by: Peter Tyser <ptyser@xes-inc.com>
Acked-by: Peter Tyser <ptyser@xes-inc.com>

Looks good to me,
Peter

[U-Boot] [PATCH] fsl_i2c: increase I2C timeout values and make them configurable

[Heiko Schocher]

Hello Timur,

Timur Tabi wrote:
> The value of I2C_TIMEOUT in fsl_i2c.c has several problems.  First, it is
> defined as CONFIG_HZ/4, but it is used as a count of microseconds, so it makes
> no sense to derive it from a clock rate.  Second, the current value (250) is
> too low for some boards, so it needs to be increased.  Third, the timeout
> necessary for multiple-master arbitration is larger than the timeout for basic
> read/write operations, so we shouldn't have a single constant for both timeouts.
> Finally, it would be nice if we could override these values on a per-board
> basis.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>  drivers/i2c/fsl_i2c.c |   24 ++++++++++++++++++++----
>  1 files changed, 20 insertions(+), 4 deletions(-)

Applied to u-boot-i2c.git

Thanks for your work,

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