On Sun, 22 Oct 2006 23:25:40 +0400, Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx> wrote:
> > arch/mips/au1000/common/time.c | 98 ----------
>
> If the generic implementation is working well, the Alchemy code doesn't
> need its own anymore. However, my patch that fixes the mips_hpt_frequency
> calculation needs to be applied first before deleing this code. I'll try to
> look into this and test some time...
Hmm, mips_hpt_frequency would be bad than lesser resolution. Please
push your fix to Ralf again ;)
> > +static unsigned int jmr3927_hpt_read(void)
> > +{
> > + unsigned int count;
> > + unsigned long j;
> > + /* read consistent jiffies and counter */
> > + do {
> > + count = jmr3927_tmrptr->trr;
> > + j = jiffies;
> > + } while (count > jmr3927_tmrptr->trr);
> > + return j * (JMR3927_TIMER_CLK / HZ) + count;
> > +}
>
> That emulation trick looks very dubious. I'd suggest to implement a
> different clocksource driver instead, since this is, after all, is not a CPU
> counter. And this will get in the way of the clockevent implementation later.
> Also, it's stops to be continuous this way. And I don't understand why you
> need this trick at all if you have the variable mips_hpt_mask...
> And the same complaint about BCM1480 code.
This trick is due to range of TRR register. The width of the counter
field is 24bit, but the range is not 0 - 0xffffff. It wraps at some
non-all-F value. So mips_hpt_mask can not help this.
But this loop is not correct indeed. If it called without xtime_lock
and interrupt disabled, it would return wrong value. I should think
again ...
> Well, I'd vote against the generic implementation. It's not
> quite correct to call all the diverse timers here "MIPS", IMHO...
How about calling it "MIPS-hpt" or something?
---
Atsushi Nemoto
