[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Bug with clock nanoseconds



Here are the functions for ERC32 and Leon2 - ERC32 and Leon2 already have
the is_interrupt_pending functions :D :



ERC32:

uint32_t bsp_clock_nanoseconds_since_last_tick(void) {
    uint32_t clicks;

    clicks = ERC32_MEC.Real_Time_Clock_Counter;

    /* Down counter */
    clicks = (uint32_t) (BSP_Configuration.microseconds_per_tick - clicks) *
1000;

    /* if an interrupt occurred while interrupts were disabled and the
nanoseconds is too little */
    /* means that a clock interrupt occurred BEFORE the nanoseconds were
read */
    if (ERC32_Is_interrupt_pending(ERC32_INTERRUPT_REAL_TIME_CLOCK) &&
clicks < BSP_Configuration.microseconds_per_tick / 2)
        clicks += BSP_Configuration.microseconds_per_tick * 1000;

    return clicks;
}




LEON2:

uint32_t bsp_clock_nanoseconds_since_last_tick(void) {
    uint32_t clicks;

    clicks = LEON_REG.Timer_Counter_1;

    /* Down counter */
    clicks = (uint32_t) (BSP_Configuration.microseconds_per_tick - clicks) *
1000;

    /* if an interrupt occurred while interrupts were disabled and the
nanoseconds is too little */
    /* means that a clock interrupt occurred BEFORE the nanoseconds were
read */
    if (LEON_Is_interrupt_pending(LEON_INTERRUPT_TIMER1) && clicks <
BSP_Configuration.microseconds_per_tick / 2)
        clicks += BSP_Configuration.microseconds_per_tick * 1000;

    return clicks;
}


I've tested and the result seams ok :)
Kind regards
Manuel Coutinho



> -----Original Message-----
> From: rtems-users-bounces at rtems.org [mailto:rtems-users-bounces at rtems.org]
> On Behalf Of Manuel Coutinho
> Sent: Tuesday, March 31, 2009 11:59 AM
> To: 'Joel Sherrill'; 'Aitor Viana'
> Cc: rtems-users at rtems.com
> Subject: RE: Bug with clock nanoseconds
> 
> Yes, I agree :)
> 
> > -----Original Message-----
> > From: Joel Sherrill [mailto:joel.sherrill at oarcorp.com]
> > Sent: Tuesday, March 31, 2009 11:40 AM
> > To: Aitor Viana
> > Cc: Manuel Coutinho; rtems-users at rtems.com
> > Subject: Re: Bug with clock nanoseconds
> >
> > Aitor Viana wrote:
> > > Yep,
> > >
> > > crystal clear. BTW, isClockInterruptPending() is not present in the
> > > RTEMS distribution, it should be implemented for all BSPs I guess.
> > >
> > I repeat.  It is the responsibility of the clock tick device driver
> > to reliably and correctly report the number of nanoseconds since
> > the last time rtems_clock_tick was called.  If that was slightly
> > greater than 1 tick ago, then it needs to return a number of nanoseconds
> > slightly greater than 1 tick.
> >
> > Any code modifications outside the device driver are not correct and
> > will not address all cases of using this extension.
> >
> > This might be a common issue with get nanoseconds handlers but it will
> > NOT be addressed in this way.  Check the is pending bit or that the
> > counter overflowed in the get nanoseconds handler and return the correct
> > number.
> >
> > --joel
> > > Best regards,
> > >
> > > Aitor
> > >
> > >
> > > On Tue, Mar 31, 2009 at 10:30 AM, Manuel Coutinho
> > > <manuel.coutinho at edisoft.pt <mailto:manuel.coutinho at edisoft.pt>>
> wrote:
> > >
> > >     Hi. Thanks for testing in leon2.was just about to do that :)!
> > >
> > >
> > >
> > >     Basically, the test aims to see that the clock is always
> > >     increasing. But the result is that, sometimes, it decreases.
> > >
> > >     This happens when a clock tick occurs.
> > >
> > >
> > >
> > >     The RTEMS algorithm to read the time consists of:
> > >
> > >
> > >
> > >     1. Disabling interrupts
> > >
> > >     2. Read how many clock interrupts occurred
> > >
> > >     3. Read nanoseconds elapsed
> > >
> > >     4. Enable interrupts
> > >
> > >
> > >
> > >     The problem occurs if an interrupt occurs between the 1 and 3
> > >     steps: -> the number of clock ticks is NOT updated (because
> > >     interrupts are disabled) and the nanoseconds field (which is read
> > >     directly from the CPU registers) is restarted.
> > >
> > >
> > >
> > >     For example, the clock is 2 clock ticks and 99999 us (a clock tick
> > >     is just about to occur). Suppose you disable interrupts, read 2
> > >     clock ticks, then the CPU clock counter overflows and the
> > >     nanoseconds is reset to 0 and you read 0 nanoseconds.
> > >
> > >     You end up reading 2 clock ticks and 0 nanoseconds when in fact
> > >     you should read 3 clock ticks and 0 nanoseconds - you cannot stop
> > >     the clock counter (nor do you wish to!).
> > >
> > >
> > >
> > >     Hope this explains the problem a little bit better :)
> > >
> > >
> > >
> > >     By the way, by glancing at the code of the remaining BSPs,
> > >     specifically at the Clock_driver_nanoseconds_since_last_tick
> > >     macro, I don't believe this is a problem just with SPARC. I don't
> > >     see this problem address in any board. But, of course, I might be
> > >     wrong since it was just a very brief look.
> > >
> > >
> > >
> > >     Kind regards
> > >
> > >     Manuel Coutinho
> > >
> > >
> > >
> > >     ------------------------------------------------------------------
> --
> > ----
> > >
> > >     *From:* aitorvs at googlemail.com <mailto:aitorvs at googlemail.com>
> > >     [mailto:aitorvs at googlemail.com <mailto:aitorvs at googlemail.com>]
> > >     *On Behalf Of *Aitor Viana
> > >     *Sent:* Tuesday, March 31, 2009 8:31 AM
> > >     *To:* Joel Sherrill
> > >     *Cc:* Manuel Coutinho; rtems-users at rtems.com
> > >     <mailto:rtems-users at rtems.com>
> > >     *Subject:* Re: Bug with clock nanoseconds
> > >
> > >
> > >
> > >     Just tried in tsim-leon and happens the same.
> > >
> > >
> > >
> > >     I didn't completely understand what's the problem Manuel could you
> > >     explain to me again? is early in the morning :D
> > >
> > >
> > >
> > >     Best regards,
> > >
> > >
> > >
> > >     Aitor
> > >
> > >
> > >
> > >     On Mon, Mar 30, 2009 at 6:49 PM, Joel Sherrill
> > >     <joel.sherrill at oarcorp.com <mailto:joel.sherrill at oarcorp.com>>
> > wrote:
> > >
> > >     Manuel Coutinho wrote:
> > >     >
> > >     > Hi
> > >     >
> > >     >
> > >     >
> > >     > We have discovered a bug of RTEMS relating to the clock. Even
> > though
> > >     > we only tested it for the ERC32 board (for now), we believe it
> is
> > >     > common throughout all the targets.
> > >     >
> > >
> > >     No you have discovered a bug in the ERC32's get nanoseconds since
> > last
> > >     tick handler.
> > >     Apparently it doesn't recognize that is has been longer than a
> tick
> > >     since the last clock
> > >     tick.  It should determine that is has overflowed and return 1
> clock
> > >     tick + partial clock
> > >     tick.
> > >
> > >     It is returning the wrong answer.
> > >
> > >
> > >     >
> > >     >
> > >     > The test consists in checking if the time given by
> rtems_clock_get
> > is
> > >     > always increasing. The code is as follows:
> > >     >
> > >     >
> > >     >
> > >
> > >     > .....
> > >
> > >
> > >     ....
> > >
> > >     >
> > >     > As you can see, the problem is raised when a clock tick occurs:
> > the
> > >     > time read in "new" is older than "old". This cannot happen since
> > >     "new"
> > >     > is updated AFTER "old"! This problem occurs if a clock interrupt
> > is
> > >     > triggered after the number of clock ticks have been read but
> > BEFORE
> > >     > the nanosecond field has been read. The resulting number of
> clock
> > >     > ticks is "small" (should be plus one).
> > >     >
> > >     >
> > >     >
> > >     > We can use the interrupt pending CPU registers info to determine
> > if,
> > >     > while we are reading the number of clock ticks that occurred and
> > the
> > >     > nanoseconds, a clock interrupt could be pending.
> > >     >
> > >     >
> > >     >
> > >     > We do not have enough knowledge to say that this solution can be
> > used
> > >     > for all boards (determining if an interrupt is pending). In the
> > SPARC
> > >     > architecture, at least, it is possible. If it is possible in all
> > >     > architectures, then a solution to RTEMS would be to change the
> > >     code of
> > >     > the _TOD_Get_uptime function to:
> > >     >
> > >     >
> > >     >
> > >     >
> > >     >
> > >     > void _TOD_Get_uptime(struct timespec *uptime) {
> > >     >
> > >     >     ISR_Level level;
> > >     >
> > >     >     struct timespec offset;
> > >     >
> > >     >     volatile uint32_t pending;
> > >     >
> > >     >
> > >     >
> > >     >     /* assume uptime checked by caller */
> > >     >
> > >     >
> > >     >
> > >     >     offset.tv_sec = 0;
> > >     >
> > >     >     offset.tv_nsec = 0;
> > >     >
> > >     >
> > >     >
> > >     >     _ISR_Disable(level);
> > >     >
> > >     >     *uptime = _TOD_Uptime;
> > >     >
> > >     >     if (_Watchdog_Nanoseconds_since_tick_handler)
> > >     >
> > >     >         offset.tv_nsec =
> > >     (*_Watchdog_Nanoseconds_since_tick_handler)();
> > >     >
> > >     >
> > >     >
> > >     >     /* code added: */
> > >     >
> > >     >     pending = isClockInterruptPending();
> > >     >
> > >     >
> > >     >
> > >     >     _ISR_Enable(level);
> > >     >
> > >     >
> > >     >
> > >     >     /* code added */
> > >     >
> > >     >     /* if an interrupt occurred while interrupts were disabled
> > >     and the
> > >     > nanoseconds is too little */
> > >     >
> > >     >     /* means that a clock interrupt occurred BEFORE the
> > nanoseconds
> > >     > were read */
> > >     >
> > >     >     if (pending && offset.tv_nsec <
> > >     > rtems_configuration_get_microseconds_per_tick() / 2) {
> > >     >
> > >     >         struct timespec clockTick = {0,
> > >     > rtems_configuration_get_microseconds_per_tick()*1000};
> > >     >
> > >     >         _Timespec_Add_to(&offset, &clockTick); /* so add one
> clock
> > >     > tick to the offset */
> > >     >
> > >     >     }
> > >     >
> > >     >
> > >     >
> > >     >     /* else, the clock tick occurred AFTER the nanoseconds were
> > read,
> > >     > so no problem */
> > >     >
> > >     >
> > >     >
> > >     >     _Timespec_Add_to(uptime, &offset);
> > >     >
> > >     > }
> > >     >
> > >     >
> > >     >
> > >     >
> > >     >
> > >     > At least, with these modifications, the test passes :)!
> > >     >
> > >     >
> > >     >
> > >     > Kind regards
> > >     >
> > >     > Manuel Coutinho
> > >     >
> > >
> > >     --
> > >     Joel Sherrill, Ph.D.             Director of Research &
> Development
> > >     joel.sherrill at OARcorp.com        On-Line Applications Research
> > >     Ask me about RTEMS: a free RTOS  Huntsville AL 35805
> > >       Support Available             (256) 722-9985
> > >
> > >
> > >     _______________________________________________
> > >     rtems-users mailing list
> > >     rtems-users at rtems.com <mailto:rtems-users at rtems.com>
> > >     http://rtems.rtems.org/mailman/listinfo/rtems-users
> > >
> > >
> > >
> > >
> 
> 
> _______________________________________________
> rtems-users mailing list
> rtems-users at rtems.com
> http://rtems.rtems.org/mailman/listinfo/rtems-users