On Wed, May 20, 2015 at 12:25:12AM +0200, Ola Liljedahl wrote:
> On 19 May 2015 at 15:34, Jacob, Jerin <jerin.ja...@caviumnetworks.com> wrote:
> > Ola,
> >
> > Is there any specific reason for following check in timer validation test ?
> pa
> >
> > diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c
> > index 554b353..724026e 100644
> > --- a/test/validation/odp_timer.c
> > +++ b/test/validation/odp_timer.c
> > @@ -260,7 +260,7 @@ static void handle_tmo(odp_event_t ev, bool stale,
> > uint64_t prev_tick)
> >
> > if (ttp != NULL) {
> > /* Internal error */
> > - CU_ASSERT_FATAL(ttp->ev == ODP_EVENT_INVALID);
> > +----------> CU_ASSERT_FATAL(ttp->ev == ODP_EVENT_INVALID);
> > ttp->ev = ev;
> > }
> > }
> >
> > AFAIU, I should be CU_ASSERT_FATAL(ttp->ev != ODP_EVENT_INVALID) as
> > tt[i].ev = odp_timeout_to_event(odp_timeout_alloc(tbp)) specified while
> > preparing all timers.
> Yes the timers are still inactive and the timeout event is stored in
> the 'ev' member.
>
> handle_timeout() is called for received timeouts (timer has expired).
> In that case, the corresponding 'ev' member should not contain any
> timeout event.
>
> >
> > Am I missing something in the timer specification ?
> Or the timer specification is missing something?
>
> odp_timer_set_abs(tt[i].tim, tck, &tt[i].ev); (line 309) is supposed
> to grab the timeout event (on success) and clear the variable (write
> ODP_TIMEOUT_INVALID), that's why the timeout is passed by reference
> ("&tt[i].ev").
>
> Possibly this is not specified clearly enough in timer.h:
> * @param[in,out] tmo_ev Reference to an event variable that points to
> * timeout event or NULL to reuse the existing timeout event. Any existing
> * timeout event that is replaced by a successful set operation will be
> * returned here.
>
> The new timeout event is read from *tmo_ev. The old timeout event (if
> timer was active) or ODP_TIMEOUT_INVALID (if timer was inactive) is
> stored in *tmo_ev. I hope this is at least clear in the reference
> implementation.
We are on same page, except the last notes
IMO, linux generic timer implementation details leaked into creating the test
case.
AFAIU, *tmo_ev should have the event that used for _arming_ the timer so
that application can do some look up after receiving event through queue or
something similar..
What is the point of providing "ODP_TIMEOUT_INVALID" to application back, What
the
use of it for the application.
IMO, two way we can come to a conclusion for this issue.
1) Remove CU_ASSERT_FATAL(ttp->ev == ODP_EVENT_INVALID) in handle_tmo function
in unit testcase
2) Or some reason, If application wants ODP_TIMEOUT_INVALID(for inactive case)
in *tmo_ev
lets update the specification so that it will clear for odp implementer
any thought from application perspective ?
>
> >
> > Thanks,
> > Jerin.
> >
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
