Patchwork [U-Boot,1/7] powerpc/i2c: introduce CONFIG_I2C_TWR for setting tWR value

login
register
mail settings
Submitter York Sun
Date May 26, 2011, 11:25 p.m.
Message ID <1306452353-11611-1-git-send-email-yorksun@freescale.com>
Download mbox | patch
Permalink /patch/97636/
State Not Applicable
Delegated to: Kumar Gala
Headers show

Comments

York Sun - May 26, 2011, 11:25 p.m.
From: york <yorksun@freescale.com>

EEPROM requires tWR for write cycle time. Since there is no other way to
poll if the internal programming ends, wait for 5ms which is the max timing
for AT24C01/02/04/08/16 by default. It can be overridden by defining
CONFIG_I2C_TWR in configuration file if the slowest device has different write
cycle time.

Signed-off-by: York Sun <yorksun@freescale.com>
---
 README                |    7 +++++++
 drivers/i2c/fsl_i2c.c |    9 +++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)
Tabi Timur-B04825 - May 27, 2011, 12:20 a.m.
York Sun wrote:
> From: york <yorksun@freescale.com>

You should fix this so that it includes your full name.
York Sun - May 27, 2011, 12:25 a.m.
On Thu, 2011-05-26 at 17:20 -0700, Tabi Timur-B04825 wrote:
> York Sun wrote:
> > From: york <yorksun@freescale.com>
> 
> You should fix this so that it includes your full name.
> 
> -- 

Sure. I can fix it along with other feedback.

York
Heiko Schocher - May 27, 2011, 6:04 a.m.
Hello York,

York Sun wrote:
> From: york <yorksun@freescale.com>
> 
> EEPROM requires tWR for write cycle time. Since there is no other way to
> poll if the internal programming ends, wait for 5ms which is the max timing
> for AT24C01/02/04/08/16 by default. It can be overridden by defining
> CONFIG_I2C_TWR in configuration file if the slowest device has different write
> cycle time.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>
> ---
>  README                |    7 +++++++
>  drivers/i2c/fsl_i2c.c |    9 +++++++++
>  2 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/README b/README
> index 8bb9c8d..83a316c 100644
> --- a/README
> +++ b/README
> @@ -1717,6 +1717,13 @@ The following options need to be configured:
>  		devices can use either method, but some require one or
>  		the other.
>  
> +		CONFIG_I2C_TWR

Maybe CONFIG_SYS_I2C_EEPROM_TWR is better?

> +
> +		defining this will override the default write cycle time

the default eeprom write cycle time ...

> +		5ms which is the default for AT24C01/02/4/08/16. If the
> +		slowest I2C device has different write cycle time, this
> +		option should be defined. The unit is microsecond.
> +
>  - SPI Support:	CONFIG_SPI
>  
>  		Enables SPI driver (so far only tested with
> diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
> index cb13dee..d192b2a 100644
> --- a/drivers/i2c/fsl_i2c.c
> +++ b/drivers/i2c/fsl_i2c.c
> @@ -42,6 +42,14 @@
>  #define CONFIG_I2C_TIMEOUT	10000
>  #endif
>  
> +/*
> + * tWR is the write cycle time for EEPROM device
> + * in microseconds
> + */
> +#ifndef CONFIG_I2C_TWR
> +#define CONFIG_I2C_TWR 5000
> +#endif
> +
>  #define I2C_READ_BIT  1
>  #define I2C_WRITE_BIT 0
>  
> @@ -416,6 +424,7 @@ i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
>  	if (i2c_wait4bus()) /* Wait until STOP */
>  		debug("i2c_write: wait4bus timed out\n");
>  
> +	udelay(CONFIG_I2C_TWR);
>  	if (i == length)
>  	    return 0;
>  

Hmm.. you add this timeout in the i2c driver, which will result in
adding this default 5 ms delay for *all* i2c writes, not only for
eeprom devices ... why you didn;t add this timeout in cmd_eeprom,
where it seems to me is the better place for it? Looking in this
common/cmd_eeprom.c there is a CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS
define ... is this not adequate?

bye,
Heiko
York Sun - May 27, 2011, 6:25 a.m.
On Fri, 2011-05-27 at 08:04 +0200, Heiko Schocher wrote:

> Hmm.. you add this timeout in the i2c driver, which will result in
> adding this default 5 ms delay for *all* i2c writes, not only for
> eeprom devices ... why you didn;t add this timeout in cmd_eeprom,
> where it seems to me is the better place for it? Looking in this
> common/cmd_eeprom.c there is a CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS
> define ... is this not adequate?
> 


You are right. I overlooked CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS and
should use this instead.

Drop this patch, please.

York

Patch

diff --git a/README b/README
index 8bb9c8d..83a316c 100644
--- a/README
+++ b/README
@@ -1717,6 +1717,13 @@  The following options need to be configured:
 		devices can use either method, but some require one or
 		the other.
 
+		CONFIG_I2C_TWR
+
+		defining this will override the default write cycle time
+		5ms which is the default for AT24C01/02/4/08/16. If the
+		slowest I2C device has different write cycle time, this
+		option should be defined. The unit is microsecond.
+
 - SPI Support:	CONFIG_SPI
 
 		Enables SPI driver (so far only tested with
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
index cb13dee..d192b2a 100644
--- a/drivers/i2c/fsl_i2c.c
+++ b/drivers/i2c/fsl_i2c.c
@@ -42,6 +42,14 @@ 
 #define CONFIG_I2C_TIMEOUT	10000
 #endif
 
+/*
+ * tWR is the write cycle time for EEPROM device
+ * in microseconds
+ */
+#ifndef CONFIG_I2C_TWR
+#define CONFIG_I2C_TWR 5000
+#endif
+
 #define I2C_READ_BIT  1
 #define I2C_WRITE_BIT 0
 
@@ -416,6 +424,7 @@  i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
 	if (i2c_wait4bus()) /* Wait until STOP */
 		debug("i2c_write: wait4bus timed out\n");
 
+	udelay(CONFIG_I2C_TWR);
 	if (i == length)
 	    return 0;