If I understand your patch correctly, a timeout event is really a buffer with the timeout-specific header stored just after the buffer header (as before). You don't change the definition of odp_timeout_hdr_t and the implementation of odp_timeout_hdr(odp_buffer_t buf) has just moved from odp_timer_internal.h to odp_timer.c where is renamed to timeout_hdr_from_buf(). There is also a new function timeout_hdr(odp_timeout_t tmo) which helps with the conversion from timeout to timeout header.
So you essential change seems to make the timeout event handle the same as a buffer event handle and not necessarily a pointer (as in the original code). I don't see why this is an important change (from a portability or structural point of view), can you please explain this? -- Ola On 31 May 2015 at 08:50, Taras Kondratiuk <[email protected]> wrote: > On 05/29/2015 07:26 AM, Ola Liljedahl wrote: > > On 13 May 2015 at 17:31, Ola Liljedahl <[email protected] > > <mailto:[email protected]>> wrote: > > > > On 13 May 2015 at 12:19, Maxim Uvarov <[email protected] > > <mailto:[email protected]>> wrote: > > > > Patch looks good. Validation test passed. Need one more review. > > > > Thanks. I did this primarily for KS2 being able to use the > > linux-generic timer implementation without any changes. I am waiting > > for Taras to verify this. > > > > Well it seems now that Taras is not going to verify that this patch > > solves his problems on KS2. I like the patch anyway and would like to > > see it merged. Any reviewers? > > Sorry for a long delay. > I had a slightly different approach in mind. > > ----8<----------------------------------------------------------------- > From: Taras Kondratiuk <[email protected]> > Date: Sat, 30 May 2015 23:26:19 -0700 > Subject: [PATCH] linux-generic: timer: convert odp_timeout_t to event > > Signed-off-by: Taras Kondratiuk <[email protected]> > --- > .../linux-generic/include/odp/plat/timer_types.h | 8 ++++-- > .../linux-generic/include/odp_timer_internal.h | 8 ------ > platform/linux-generic/odp_timer.c | 31 > ++++++++++++---------- > 3 files changed, 23 insertions(+), 24 deletions(-) > > diff --git a/platform/linux-generic/include/odp/plat/timer_types.h > b/platform/linux-generic/include/odp/plat/timer_types.h > index 006683e..d987096 100644 > --- a/platform/linux-generic/include/odp/plat/timer_types.h > +++ b/platform/linux-generic/include/odp/plat/timer_types.h > @@ -18,6 +18,10 @@ > extern "C" { > #endif > > +#include <odp/std_types.h> > +#include <odp/plat/strong_types.h> > +#include <odp/plat/event_types.h> > + > /** @addtogroup odp_timer > * @{ > **/ > @@ -32,9 +36,9 @@ typedef uint32_t odp_timer_t; > > #define ODP_TIMER_INVALID ((uint32_t)~0U) > > -typedef void *odp_timeout_t; > +typedef odp_handle_t odp_timeout_t; > > -#define ODP_TIMEOUT_INVALID NULL > +#define ODP_TIMEOUT_INVALID (odp_timeout_t)ODP_EVENT_INVALID > > /** > * @} > diff --git a/platform/linux-generic/include/odp_timer_internal.h > b/platform/linux-generic/include/odp_timer_internal.h > index 90af62c..db379ec 100644 > --- a/platform/linux-generic/include/odp_timer_internal.h > +++ b/platform/linux-generic/include/odp_timer_internal.h > @@ -40,12 +40,4 @@ typedef struct odp_timeout_hdr_stride { > } odp_timeout_hdr_stride; > > > -/** > - * Return the timeout header > - */ > -static inline odp_timeout_hdr_t *odp_timeout_hdr(odp_buffer_t buf) > -{ > - return (odp_timeout_hdr_t *)odp_buf_to_hdr(buf); > -} > - > #endif > diff --git a/platform/linux-generic/odp_timer.c > b/platform/linux-generic/odp_timer.c > index b7cb04f..39e76e1 100644 > --- a/platform/linux-generic/odp_timer.c > +++ b/platform/linux-generic/odp_timer.c > @@ -80,7 +80,13 @@ static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple > locks per cache line! */ > > static odp_timeout_hdr_t *timeout_hdr_from_buf(odp_buffer_t buf) > { > - return (odp_timeout_hdr_t *)odp_buf_to_hdr(buf); > + return (odp_timeout_hdr_t *)(void *)odp_buf_to_hdr(buf); > +} > + > +static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t tmo) > +{ > + odp_buffer_t buf = > odp_buffer_from_event(odp_timeout_to_event(tmo)); > + return timeout_hdr_from_buf(buf); > } > > > > /****************************************************************************** > @@ -422,7 +428,8 @@ static bool timer_reset(uint32_t idx, > } else { > /* We have a new timeout buffer which replaces any old one > */ > /* Fill in some (constant) header fields for timeout > events */ > - if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) { > + if (odp_event_type(odp_buffer_to_event(*tmo_buf)) == > + ODP_EVENT_TIMEOUT) { > /* Convert from buffer to timeout hdr */ > odp_timeout_hdr_t *tmo_hdr = > timeout_hdr_from_buf(*tmo_buf); > @@ -567,7 +574,8 @@ static unsigned timer_expire(odp_timer_pool *tp, > uint32_t idx, uint64_t tick) > #endif > if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) { > /* Fill in expiration tick for timeout events */ > - if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) { > + if (odp_event_type(odp_buffer_to_event(tmo_buf)) == > + ODP_EVENT_TIMEOUT) { > /* Convert from buffer to timeout hdr */ > odp_timeout_hdr_t *tmo_hdr = > timeout_hdr_from_buf(tmo_buf); > @@ -811,19 +819,17 @@ odp_timeout_t odp_timeout_from_event(odp_event_t ev) > /* This check not mandated by the API specification */ > if (odp_event_type(ev) != ODP_EVENT_TIMEOUT) > ODP_ABORT("Event not a timeout"); > - return > (odp_timeout_t)timeout_hdr_from_buf(odp_buffer_from_event(ev)); > + return (odp_timeout_t)ev; > } > > odp_event_t odp_timeout_to_event(odp_timeout_t tmo) > { > - odp_timeout_hdr_t *tmo_hdr = (odp_timeout_hdr_t *)tmo; > - odp_buffer_t buf = odp_hdr_to_buf(&tmo_hdr->buf_hdr); > - return odp_buffer_to_event(buf); > + return (odp_event_t)tmo; > } > > int odp_timeout_fresh(odp_timeout_t tmo) > { > - const odp_timeout_hdr_t *hdr = (odp_timeout_hdr_t *)tmo; > + const odp_timeout_hdr_t *hdr = timeout_hdr(tmo); > odp_timer_t hdl = hdr->timer; > odp_timer_pool *tp = handle_to_tp(hdl); > uint32_t idx = handle_to_idx(hdl, tp); > @@ -836,20 +842,17 @@ int odp_timeout_fresh(odp_timeout_t tmo) > > odp_timer_t odp_timeout_timer(odp_timeout_t tmo) > { > - const odp_timeout_hdr_t *hdr = (odp_timeout_hdr_t *)tmo; > - return hdr->timer; > + return timeout_hdr(tmo)->timer; > } > > uint64_t odp_timeout_tick(odp_timeout_t tmo) > { > - const odp_timeout_hdr_t *hdr = (odp_timeout_hdr_t *)tmo; > - return hdr->expiration; > + return timeout_hdr(tmo)->expiration; > } > > void *odp_timeout_user_ptr(odp_timeout_t tmo) > { > - const odp_timeout_hdr_t *hdr = (odp_timeout_hdr_t *)tmo; > - return hdr->user_ptr; > + return timeout_hdr(tmo)->user_ptr; > } > > odp_timeout_t odp_timeout_alloc(odp_pool_t pool) > -- > 1.9.1 > >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
