[U-Boot] [PATCH 3/3] mpc5200: make i2c faster

[Wolfgang Denk]

Dear Jon,

In message <9e4733910903220720l723de65fm59a7e6a7fed1eda7 at mail.gmail.com> you wrote:
>
> > And how much do we make I2C faster this way? Where do we save time,
> > and how much?
> 
> I measured the delay. It is between 40us and 52us on my hardware so I
> adjusted the loop to 60us. The previous 1ms delay was 20x bigger than
> necessary.  If the hardware is slower, it will  just loop and add more
> delay.

You did not answer my question in which way this actually makes I2C
faster? Where do we save time, and how much?

Which operation of I2C on the board gets acelerated, and how much?


> The mpc5xxx i2c driver has great delays while waiting for the chip status.
> Make the delays smaller and the driver faster. The measured delay is
> 55us at 100Khz. Set the delay to 15us which should work for 400Khz. 100Khz
> will loop four times and get a 60us delay.
> 
> Signed-off-by: Jon Smirl <jonsmirl at gmail.com>
> ---
>  cpu/mpc5xxx/i2c.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
> index a5478b2..dcb184c 100644
> --- a/cpu/mpc5xxx/i2c.c
> +++ b/cpu/mpc5xxx/i2c.c
> @@ -38,7 +38,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  #error CONFIG_SYS_I2C_MODULE is not properly configured
>  #endif
>
> -#define I2C_TIMEOUT	100
> +#define I2C_TIMEOUT	1000
>  #define I2C_RETRIES	3
>
>  #ifdef CONFIG_ENV_IS_IN_EEPROM
> @@ -101,7 +101,7 @@ static int wait_for_bb(void)
>  		mpc_reg_out(&regs->mcr, 0, 0);
>  		mpc_reg_out(&regs->mcr, I2C_EN, 0);
>  #endif
> -		udelay(1000);
> +		udelay(15);
>  		status = mpc_reg_in(&regs->msr);
>  	}

Hm.. You increase the I2C_TIMEOUT by a factor of 10, but  reduce  the
per-loop  delay by a factor of > 66; instead of the old total timeout
of 100 * 1000 us = 100 milliseconds we now have a timeout of  1000  *
15 us = 15 milliseconds. That is much, much less.

Is this change intentional? If yes, are you sure it is  correct  with
all existing boards and I2C devices out there?

> @@ -116,7 +116,7 @@ static int wait_for_pin(int *status)
>  	*status = mpc_reg_in(&regs->msr);
>
>  	while (timeout-- && !(*status & I2C_IF)) {
> -		udelay(1000);
> +		udelay(15);
>  		*status = mpc_reg_in(&regs->msr);

Ditto here.

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
The best things in life are for a fee.