[U-Boot-Users] [PATCH] [resend] mips: Support to set CFG_HZ to 1000, consistent with other architectures

[Shinya Kuribayashi <shinya.kuribayashi@necel.com>]

Hi Jason,

I don't look closely yet, but find some comments below:

Jason McMullan wrote:
> ---
>  include/configs/dbau1x00.h  |    3 ++-
>  include/configs/gth2.h      |    3 ++-
>  include/configs/incaip.h    |    3 ++-
>  include/configs/pb1x00.h    |    3 ++-
>  include/configs/purple.h    |    3 ++-
>  include/configs/qemu-mips.h |    3 ++-
>  include/configs/tb0229.h    |    3 ++-

Ok.

> diff --git a/lib_mips/time.c b/lib_mips/time.c
> index cd8dc72..4807f1a 100644
> --- a/lib_mips/time.c
> +++ b/lib_mips/time.c
> @@ -23,23 +23,57 @@
>  
>  #include <common.h>
>  
> +/* CFG_MIPS_CLOCK is the number of ticks per second of the MIPS C0 Clock timer.
> + *
> + * For most implementations, this is the same as the CPU speed in HZ
> + * divided by 2. Some embedded MIPS implementations may use a /4
> + * or /1 divider, so see your CPU reference manual for specific details.
> + */
> +#ifndef CFG_MIPS_CLOCK
> +#error CFG_MIPS_CLOCK must be set in the board configuration file
> +#endif
>  
> +static struct {
> +	uint32_t lo;
> +	uint32_t hi;
> +} mips_ticks;	/* Last number of ticks seen */

See below.

> +/* Input is in CFG_HZ ticks */
>  static inline void mips_compare_set(u32 v)
>  {
> +	v *= (CFG_MIPS_CLOCK / CFG_HZ);
>  	asm volatile ("mtc0 %0, $11" : : "r" (v));
>  }

I tend to remove these mips_{count,compare}_{get,set} functions because
they can be (and should be) replaced {read,write}_32bit_ cp0_register,
IMO.

And I want to handle (CFG_MIPS_CLOCK/CFG_HZ) stuffs or something like
that at udelay() side.

> +/* Returns CFG_HZ ticks 
> + *
> + * NOTE: This must be called at least once every
> + *       few seconds to be reliable.
> + */
>  static inline u32 mips_count_get(void)
>  {
>  	u32 count;
>  
>  	asm volatile ("mfc0 %0, $9" : "=r" (count) :);
> +
> +	/* Handle 32-bit timer overflow */
> +	if (count < mips_ticks.lo) {
> +		mips_ticks.hi++;
> +	}
> +	mips_ticks.lo = count;
> +	count =(mips_ticks.lo / (CFG_MIPS_CLOCK / CFG_HZ)) +
> +	       (mips_ticks.hi * (0x100000000ULL / (CFG_MIPS_CLOCK / CFG_HZ)));
> +
>  	return count;
>  }

I disagree with having this structure. Basic strategy for MIPS COUNT/
COMPARE handling is, let them overflow (os should I say wrap-around) as
they are. All we need is the Delta, not the numbers of overflows.

> @@ -75,7 +109,7 @@ void udelay (unsigned long usec)
>  	ulong tmo;
>  	ulong start = get_timer(0);
>  
> -	tmo = usec * (CFG_HZ / 1000000);
> +	tmo = usec * CFG_HZ / 1000;
>  	while ((ulong)((mips_count_get() - start)) < tmo)
>  		/*NOP*/;
>  }

Again, what is needed is `the Delta' between CP0.COUNT(Previous) and
CP0.COUNT(Current). This can be always achieved by

    delta = COUNT(Curr) - COUNT(Prev)

regardless of COUNT value. Even if `(u32)0x00010000 - (u32)0xFFFFFFFF',
it works.

I'll look around until this weekend. Sorry for inconvinience, and thank
you for working on this.

  Shinya