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

[Heiko Schocher <hs@denx.de>]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany