Re: clocks on mips

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