| Message ID | 1435163776-29778-1-git-send-email-ivan.khoronzhuk@linaro.org |
|---|---|
| State | Accepted |
| Commit | 926c7bf578329ec936ac1933db232768d781e2f9 |
| Headers | show |
On 24 June 2015 at 18:36, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > The set_next_free() checks if timer queue is not set, that is > ODP_QUEUE_INVALID. In case of linux-generic the invalid queue handle > is 0. By coincidence, after allocation, the timer queue handle is > also 0, by this way the ODP_ASSERT is masked. > In case ODP_QUEUE_INVALID is another from 0 the test generates error. > It's not correct, the code shouldn't have such kind dependency from > macro definition, and it prevents to re-use this code for another > platforms with different ODP_QUEUE_INVALID. So, fix it. > Same patch, updated commit message. I approve of this again. -- Ola > > Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > > > > platform/linux-generic/odp_timer.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/platform/linux-generic/odp_timer.c > b/platform/linux-generic/odp_timer.c > index e5391dc..bd1b778 100644 > --- a/platform/linux-generic/odp_timer.c > +++ b/platform/linux-generic/odp_timer.c > @@ -238,6 +238,7 @@ static odp_timer_pool *odp_timer_pool_new( > /* Initialize all odp_timer entries */ > uint32_t i; > for (i = 0; i < tp->param.num_timers; i++) { > + tp->timers[i].queue = ODP_QUEUE_INVALID; > set_next_free(&tp->timers[i], i + 1); > tp->timers[i].user_ptr = NULL; > odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, TMO_UNUSED); > -- > 1.9.1 > >
Please resend this patch in text format. Not in html. Use git format-patch command. Thank you, Maxim. On 06/24/15 19:40, Ola Liljedahl wrote: > On 24 June 2015 at 18:36, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> wrote: > > The set_next_free() checks if timer queue is not set, that is > ODP_QUEUE_INVALID. In case of linux-generic the invalid queue handle > is 0. By coincidence, after allocation, the timer queue handle is > also 0, by this way the ODP_ASSERT is masked. > In case ODP_QUEUE_INVALID is another from 0 the test generates error. > It's not correct, the code shouldn't have such kind dependency from > macro definition, and it prevents to re-use this code for another > platforms with different ODP_QUEUE_INVALID. So, fix it. > > Same patch, updated commit message. I approve of this again. > -- Ola > > > Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> > --- > > > > platform/linux-generic/odp_timer.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/platform/linux-generic/odp_timer.c > b/platform/linux-generic/odp_timer.c > index e5391dc..bd1b778 100644 > --- a/platform/linux-generic/odp_timer.c > +++ b/platform/linux-generic/odp_timer.c > @@ -238,6 +238,7 @@ static odp_timer_pool *odp_timer_pool_new( > /* Initialize all odp_timer entries */ > uint32_t i; > for (i = 0; i < tp->param.num_timers; i++) { > + tp->timers[i].queue = ODP_QUEUE_INVALID; > set_next_free(&tp->timers[i], i + 1); > tp->timers[i].user_ptr = NULL; > odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, TMO_UNUSED); > -- > 1.9.1 > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
Applied. Looks like something wrong was with my set up. First time I saw &ndsp and <br> in this email. Thanks Anders to double checking that patch can be applied! Maxim. On 06/24/15 19:40, Ola Liljedahl wrote: > On 24 June 2015 at 18:36, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> wrote: > > The set_next_free() checks if timer queue is not set, that is > ODP_QUEUE_INVALID. In case of linux-generic the invalid queue handle > is 0. By coincidence, after allocation, the timer queue handle is > also 0, by this way the ODP_ASSERT is masked. > In case ODP_QUEUE_INVALID is another from 0 the test generates error. > It's not correct, the code shouldn't have such kind dependency from > macro definition, and it prevents to re-use this code for another > platforms with different ODP_QUEUE_INVALID. So, fix it. > > Same patch, updated commit message. I approve of this again. > -- Ola > > > Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> > --- > > > > platform/linux-generic/odp_timer.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/platform/linux-generic/odp_timer.c > b/platform/linux-generic/odp_timer.c > index e5391dc..bd1b778 100644 > --- a/platform/linux-generic/odp_timer.c > +++ b/platform/linux-generic/odp_timer.c > @@ -238,6 +238,7 @@ static odp_timer_pool *odp_timer_pool_new( > /* Initialize all odp_timer entries */ > uint32_t i; > for (i = 0; i < tp->param.num_timers; i++) { > + tp->timers[i].queue = ODP_QUEUE_INVALID; > set_next_free(&tp->timers[i], i + 1); > tp->timers[i].user_ptr = NULL; > odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, TMO_UNUSED); > -- > 1.9.1 > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c index e5391dc..bd1b778 100644 --- a/platform/linux-generic/odp_timer.c +++ b/platform/linux-generic/odp_timer.c @@ -238,6 +238,7 @@ static odp_timer_pool *odp_timer_pool_new( /* Initialize all odp_timer entries */ uint32_t i; for (i = 0; i < tp->param.num_timers; i++) { + tp->timers[i].queue = ODP_QUEUE_INVALID; set_next_free(&tp->timers[i], i + 1); tp->timers[i].user_ptr = NULL; odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, TMO_UNUSED);