On 8 January 2015 at 15:39, Savolainen, Petri (NSN - FI/Espoo) <petri.savolai...@nsn.com> wrote: > >> >> +/** >> >> + * Create a timer pool >> >> + * >> >> + * @param name Name of the timer pool. The string will be >> copied. >> >> + * @param buf_pool Buffer pool for allocating timeouts > > Maybe a comment also here for the purpose of this pool. Or just remove it, if > there's no clear usage for it. It can be added later again if needed. I removed the buf_pool parameter. An implementation will have to allocate all resources that it may need.
> > >> >> + * @param params Timer pool parameters. The content will be >> copied. >> >> + * >> >> + * @return Timer pool handle if successful, otherwise >> >> ODP_TIMER_POOL_INVALID >> >> + * and errno set >> > >> > This is the first API to use errno. Errno needs to be changed to >> odp_errno() (with odp_errno_clear(), odp_errno_print(), odp_errno_str()). >> That can be done after merge. >> OK but these functions seem not to exist yet? > > Yes, those need to be added soon - so that ODP does not share errno with C > libraries. > > >> >> + >> >> +/** >> >> + * Set a timer (absolute time) with a user-provided timeout buffer >> >> + * >> >> + * Set (arm) the timer to expire at specific time. The timeout >> >> + * buffer will be enqueued when the timer expires. >> >> + * >> >> + * Note: any invalid parameters will be treated as programming errors >> and >> >> will >> >> + * cause the application to abort. >> >> + * >> >> + * @param tim Timer >> >> + * @param abs_tck Expiration time in absolute timer ticks >> >> + * @param tmo_buf Reference to a buffer variable that points to >> timeout >> >> buffer >> >> + * or NULL to reuse the existing timeout buffer >> > >> > Tmo_buf parameter usage needs clarification: >> > - Can this be always NULL, meaning that timer should be set with a tmo >> buffer allocated from the pool provided in timer_pool_create call? >> No not in the current design. The idea could be investigated. > > OK. Maybe then remove the pool param from pool_create. > >> >> > - Is it also an output param? Timer_reset() writes it with old buffer >> handle value. >> Yes. Should be fixed. >> >> > >> > >> >> + * >> >> + * @retval ODP_TIMER_SET_SUCCESS Operation succeeded >> >> + * @retval ODP_TIMER_SET_TOOEARLY Operation failed because expiration >> >> tick too >> >> + * early >> >> + * @retval ODP_TIMER_SET_TOOLATE Operation failed because expiration >> tick >> >> too >> >> + * late >> >> + * @retval ODP_TIMER_SET_NOBUF Operation failed because timeout buffer >> >> not >> >> + * specified in call and not present in timer >> > >> > When this actually happens? When trying to reset an expired timer ? >> Yes. Timer has already expired. The application can decide what to do. >> Either allocate a new timeout buffer and retry the set operation or it >> can ignore this situation because it knows the timeout buffer will be >> received shortly. The application decides. > > OK. The return codes are a bit long. _SET_ could be dropped without > readability issues (e.g. ODP_TIMER_SUCCESS). OK > >> >> +/** >> >> + * Check for fresh timeout >> >> + * If the corresponding timer has been reset or cancelled since this >> >> timeout >> >> + * was enqueued, the timeout is stale (not fresh). >> >> * >> >> - * @return 0 if successful >> >> + * @param tmo Timeout handle >> >> + * @retval 1 Timeout is fresh >> >> + * @retval 0 Timeout is stale >> >> */ >> >> -int odp_timer_cancel_tmo(odp_timer_t timer, odp_timer_tmo_t tmo); >> >> +int /*odp_bool_t*/odp_timeout_fresh(odp_timeout_t tmo); >> > >> > Remove /*odp_bool_t*/ >> OK. Or use odp_bool_t instead of int? true -> fresh, false -> stale. >> int as a return type if normally used with 0 for success and <>0 for >> failure. > > So far odp_bool_t has not been used in return values. This falls into > _is_xxx() category and thus would return !0 = true and 0 = false. Perhaps the function should be called odp_timeout_is_fresh() then? > > > -Petri > > _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
