[U-Boot] [PATCH 1/1] Changes for fixing the timer to 1 msec and

[Dirk Behme]

Hi Mani,

Manikandan Pillai wrote:
> The cmd_misc.c was changed to fix sleep.
> nand_base.c was fixed to fix the wait.

Should this be the fix for

http://lists.denx.de/pipermail/u-boot/2009-February/048107.html

?

I'm not familiar with U-Boot timer handling, so just some questions. 
Maybe the U-Boot experts like to comment, too.

> Signed-off-by: Manikandan Pillai <mani.pillai at ti.com>
> ---
>  common/cmd_misc.c                   |    1 +
>  cpu/arm_cortexa8/omap3/interrupts.c |   13 ++++---------
>  drivers/mtd/nand/nand_base.c        |    2 ++
>  include/configs/omap3_evm.h         |    2 +-
>  4 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/common/cmd_misc.c b/common/cmd_misc.c
> index 024299a..e3e64c0 100644
> --- a/common/cmd_misc.c
> +++ b/common/cmd_misc.c
> @@ -38,6 +38,7 @@ int do_sleep (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  	}
>  
>  	delay = simple_strtoul(argv[1], NULL, 10) * CONFIG_SYS_HZ;
> +	delay += get_timer(0);

As I understand above error report there is an issue with OMAP3 timer 
not returning ms. Why do we have to touch cmd_misc.c for *all* 
architectures then?

>  	while (get_timer(start) < delay) {
>  		if (ctrlc ()) {
> diff --git a/cpu/arm_cortexa8/omap3/interrupts.c b/cpu/arm_cortexa8/omap3/interrupts.c
> index 9e9817d..5bfe6fc 100644
> --- a/cpu/arm_cortexa8/omap3/interrupts.c
> +++ b/cpu/arm_cortexa8/omap3/interrupts.c
> @@ -193,7 +193,7 @@ void reset_timer(void)
>  
>  ulong get_timer(ulong base)
>  {
> -	return get_timer_masked() - base;
> +	return get_timer_masked();

Why don't we have to deal with base here any more?

>  }
>  
>  void set_timer(ulong t)
> @@ -238,14 +238,9 @@ void reset_timer_masked(void)
>  
>  ulong get_timer_masked(void)
>  {
> -	ulong now = readl(&timer_base->tcrr); /* current tick value */
> -
> -	if (now >= lastinc)	/* normal mode (non roll) */
> -		/* move stamp fordward with absoulte diff ticks */
> -		timestamp += (now - lastinc);
> -	else	/* we have rollover of incrementer */
> -		timestamp += (0xFFFFFFFF - lastinc) + now;
> -	lastinc = now;
> +	timestamp = readl(&timer_base->tcrr); /* current tick value */
> +	/* get_timer() has to return time in milliseconds */
> +	timestamp >>= 6;

Why don't we have to deal with timer overflow here any more?

As mentioned I haven't looked into the details, but why wouldn't it be 
sufficient to just change first line of get_timer_masked() to 
something like

ulong now = readl(&timer_base->tcrr) >> 6; /* current tick value */

to get ms?

>  	return timestamp;
>  }
>  
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index d33fee2..283b456 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -509,6 +509,7 @@ void nand_wait_ready(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd->priv;
>  	u32 timeo = (CONFIG_SYS_HZ * 20) / 1000;
>  
> +	timeo += get_timer(0);
>  	reset_timer();

Same as above here and below: Why do we have to touch global code for 
*all* architectures?

>  	/* wait until command is processed or timeout occures */
> @@ -854,6 +855,7 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *this)
>  	else
>  		timeo = (CONFIG_SYS_HZ * 20) / 1000;
>  
> +	timeo += get_timer(0);
>  	if ((state == FL_ERASING) && (this->options & NAND_IS_AND))
>  		this->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1);
>  	else
> diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
> index bf5614e..8d6e5fe 100644
> --- a/include/configs/omap3_evm.h
> +++ b/include/configs/omap3_evm.h
> @@ -230,7 +230,7 @@
>  
>  #define CONFIG_SYS_TIMERBASE		OMAP34XX_GPT2
>  #define CONFIG_SYS_PVT			V_PVT	/* 2^(pvt+1) */
> -#define CONFIG_SYS_HZ			((V_SCLK) / (2 << CONFIG_SYS_PVT))
> +#define CONFIG_SYS_HZ			1000

Would be nice if you could change this for all OMAP3 boards.

Best regards

Dirk