Subject: Re: clocks on mips
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
From: Garrett D'Amore <garrett_damore@tadpole.com>
List: port-mips
Date: 09/08/2006 10:26:34
Izumi Tsutsui wrote:
>
>
> BTW, current mips3_clock.c:mips3_clockintr() doesn't handle
> lost clock interrupts properly. (just adjusting the next compare)
> I wonder if it's worth to count how many interrupts were lost
> and call hardclock(9) more than once, like current
> arc/timer.c:statclockintr() and macppc/clock.c:decr_intr() do.
>
How about the following rewrite? I've not tested it yet, but I'm a
little concerned about the potential implications of calling hardclock
repeatedly to "catch up" lost interrupts. Especially as it effects ntp
and timecounters. I'd like to hear opinions on the matter.
I also added an evcnt for missed clock interrupts.
struct evcnt mips_int5_missed_evcnt =
EVCNT_INITIALIZER(EVCNT_TYPE_INTR, NULL, "mips", "missed int 5");
/*
* Handling to be done upon receipt of a MIPS 3 clock interrupt. This
* routine is to be called from the master interrupt routine
* (e.g. cpu_intr), if MIPS INT5 is pending. The caller is
* responsible for blocking and renabling the interrupt in the
* cpu_intr() routine.
*/
void
mips3_clockintr(uint32_t status, uint32_t pc)
{
uint32_t new_cnt, delta, lost = 0;
struct clockframe cf;
cf.pc = pc;
cf.sr = status;
next_cp0_clk_intr += curcpu()->ci_cycles_per_hz;
mips3_cp0_compare_write(next_cp0_clk_intr);
/* Check for lost clock interrupts */
new_cnt = mips3_cp0_count_read();
delta = next_cp0_clk_intr - new_cnt;
while (__predict_false((int32_t)delta < 0)) {
lost++;
delta += curcpu()->ci_cycles_per_hz;
}
/*
* Missed one or more clock interrupts, so let's start
* counting again from the current value. We also trigger
* hardclocks to get the clock interrupt back up to speed.
*
* XXX: What impact will this have on NTP and timecounters?
*/
if (__predict_false(lost > 0)) {
next_cp0_clk_intr = new_cnt + curcpu()->ci_cycles_per_hz;
mips3_cp0_compare_write(next_cp0_clk_intr);
for (; lost > 0; lost--) {
hardclock(cfp);
mips_int5_evcnt.ev_count++;
}
}
hardclock(&cf);
mips_int5_evcnt.ev_count++;
/* caller should renable clock interrupts */
}
>
>> 3) this probably means cleaning up mips3_clock.c somewhat -- the
>> statclock needs to move out of it, and the delay() needs to rename to
>> mips3_delay.
>>
>> 4) for systems that can just use mips3_delay as is, I would use a weak
>> symbol alias so that at link time delay() is resolved to mips3_delay.
>>
>> 5) I'd like to rename cpu_initclocks() to mips3_initclocks() and weak
>> alias it as well.
>>
>
> Yeah, these are what I asked.
>
>
>> 6) I'd like to move the resulting mips3_initclocks() and
>> mips3_clockintr() into a new file, mips3_hardclock.c or
>> mips3_clockintr.c, so that ports which for some reason can't use these
>> (ews4800mips?) don't have to carry the baggage, but can still use the
>> rest of the code in mips3_clock.c
>>
>
> Or to have some __HAVE_MIPS_NO_INT5_CLOCK or something in types.h?
> (and we should get rid of options MIPS3_ENABLE_CLOCK_INTR)
> ---
> Izumi Tsutsui
>
--
Garrett D'Amore, Principal Software Engineer
Tadpole Computer / Computing Technologies Division,
General Dynamics C4 Systems
http://www.tadpolecomputer.com/
Phone: 951 325-2134 Fax: 951 325-2191